[FLINK-39715] [table-planner] Fix IndexOutOfBoundsException in FlinkExpandConversionRule for global aggregate after sort#28218
Open
arvindKandpal-ksolves wants to merge 1 commit into
Conversation
Collaborator
…pandConversionRule for global aggregate after sort
0b38db1 to
ae31d70
Compare
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 free
to 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.
What is the purpose of the change
This pull request fixes a query planner crash (
IndexOutOfBoundsException) that occurs in batch mode when a global aggregate function (e.g.,MAX,MIN,COUNT) is executed on top of a table that was previously sorted usingORDER BY. ( FLINK-39715 )The root cause was that
FlinkExpandConversionRule.satisfyCollationwas forcefully creating aBatchPhysicalSortwith the original sorting traits (which refer to input field indices), without validating if those indices are still within the bounds of the new node's row type (where field count shrinks to 1 due to global aggregation). This PR introduces an explicit bound check for required collation field indices before attempting to satisfy the collation trait.Brief change log
FlinkExpandConversionRule.satisfyCollationto validate that all field indices inrequiredCollationare strictly less than the current node'sgetRowType.getFieldCount. Returnsnullif the validation fails.FlinkExpandConversionRule.satisfyTraitsBySelfto gracefully return and skip conversion ifsatisfyCollationreturnsnull.ExpandConversionRuleFixTest.javaunderorg.apache.flink.table.planner.plan.rules.physicalto verify the fix and protect against future regressions.Verifying this change
This change added tests and can be verified as follows:
ExpandConversionRuleFixTest#testOrderByWithGlobalAggregatewhich reproduces the exact batch mode pipeline (ORDER BYfollowed by global aggregate) and asserts that the planner optimizes the execution plan successfully without throwing an exception.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation
Was generative AI tooling used to co-author this PR?