Uh oh!
There was an error while loading. Please reload this page.
Finish window function query planning stage. - #15151
Merged
Merged
Conversation
Sh-Zh-7
marked this pull request as ready for review
May 7, 2025 01:59
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR finalizes the window function query planning stage by introducing comprehensive support for window function analysis, planning, and execution.
- Added new analysis methods and extraction utilities for window functions and expressions.
- Extended the planner, execution operators, and window frame implementations to support window function operations.
Reviewed Changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| StatementAnalyzer.java | Added imports and new methods to analyze window definitions and resolve window functions. |
| ExpressionTreeUtils.java | Introduced static methods to extract window functions and nested window expressions. |
| ExpressionAnalyzer.java | Updated logic to process window function calls and integrated window analysis into expression processing. |
| ExpressionAnalysis.java | Extended analysis result to include window functions. |
| Analysis.java | Added maps and methods to record window definitions, window functions, and order-by window functions. |
| PlanVisitor.java, PlanNodeType.java, PlanGraphPrinter.java | Added new visit methods and node types to support window function planning and visualization. |
| TableOperatorGenerator.java | Integrated the new window function operators into the query execution plan. |
| Various window frame and function files | Updated window frame implementations and window function implementations (NthValue, Lead, Lag, NTile, etc.) to support new window query planning stage. |
| FilterAndProjectOperator.java | Triggered evaluation in column transformers as part of the projection process. |
Comments suppressed due to low confidence (1)
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/process/window/partition/frame/RangeFrame.java:78
- [nitpick] Consider adding parentheses to clarify the operator precedence in this condition. For example: if ((frameInfo.getStartType() == UNBOUNDED_PRECEDING && frameInfo.getEndType() == UNBOUNDED_FOLLOWING) || noOrderBy) {
if (frameInfo.getStartType() == UNBOUNDED_PRECEDING && frameInfo.getEndType() == UNBOUNDED_FOLLOWING || noOrderBy) {
JackieTien97
requested changes
May 9, 2025
| if (!hasNonMappableUDF) { | ||
| // get result of calculated common sub expressions | ||
| for (ColumnTransformer columnTransformer : commonTransformerList) { | ||
| columnTransformer.tryEvaluate(); |
Contributor
There was a problem hiding this comment.
Why add this new line? Are there any bugs before? If so, you may need to add IT about it.
ContributorAuthor
There was a problem hiding this comment.
if there is no .tryEvaluate, getColumn may return null.
| builder.writeBoolean(partition.getBoolean(defaultValChannel, index)); | ||
| return; | ||
| case TEXT: | ||
| case STRING: |
| builder.writeBoolean(partition.getBoolean(defaultValChannel, index)); | ||
| return; | ||
| case TEXT: | ||
| case STRING: |
| } | ||
| break; | ||
| case INT64: | ||
| case TIMESTAMP: |
| } | ||
| break; | ||
| case INT64: | ||
| case TIMESTAMP: |
| // ignore | ||
| } | ||
| // now we only support scalar or agg functions |
| if (isAggregation) { | ||
| functionKind = FunctionKind.AGGREGATE; | ||
| } else { | ||
| boolean isWindow = metadata.isWindowFunction(session, functionName, accessControl); |
Contributor
There was a problem hiding this comment.
what if agg function used in window clause? It's FunctionKind.AGGREGATE or FunctionKind.WINDOW
ContributorAuthor
There was a problem hiding this comment.
If this function is aggregation function, we will still warp it into a window function instance.
| import static java.util.Objects.requireNonNull; | ||
| public class DataOrganizationSpecification { |
Contributor
There was a problem hiding this comment.
deduplicated with org.apache.iotdb.db.queryengine.plan.relational.planner.DataOrganizationSpecification
| public class QueryPlanner { | ||
| private final Analysis analysis; | ||
| private final SymbolAllocator symbolAllocator; | ||
| private final QueryId idAllocator; |
Contributor
There was a problem hiding this comment.
why not using queryIdAllocator?
| .map(argument -> coercions.get(argument).toSymbolReference()) | ||
| .collect(toImmutableList()), | ||
| frame, | ||
| // TODO: remove ignore null |
Contributor
There was a problem hiding this comment.
we should never have todo, fix it
ContributorAuthor
There was a problem hiding this comment.
Looks like we already support IGNORE NULLS keyword. So this TODO is meanningless.
JackieTien97
approved these changes
May 26, 2025
5 tasks
Sh-Zh-7
commented
May 29, 2025
ContributorAuthor
PTAL~ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As per title.