[FLINK-15577][table-planner] Fix similar aggregations with different windows being considered the same #18
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.
What is the purpose of the change
The RelNode's digest is used by the Calcite HepPlanner to avoid adding
duplicate vertices to the graph. If an equivalent vertex was already
present in the graph, then that vertex is used in place of the newly
generated one.
This means that the digest needs to contain all the information
necessary to identifying a vertex and distinguishing it from similar (but not equivalent) vertices.
In the case of the
WindowAggregationnodes, the window specs arecurrently not in the digest, meaning that two aggregations with the same
signatures and expressions but different windows are considered
equivalent by the planner, which is not correct and will lead to an
invalid Physical Plan.
This commit fixes this issue and adds a test ensuring that the window
specs are in the digest, as well as similar aggregations on two
different windows will not be considered equivalent.
Brief change log
Added window specs to the following RelNodes:
Added unit tests to the legacy planner and the blink planner to prevent future regressions.
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation