Repository navigation
fix: sort a Spark window's doubted input by the whole order it reports - #7555
Open
dwsmith1983 wants to merge 1 commit into
Open
dwsmith1983 wants to merge 1 commit into
dwsmith1983 wants to merge 1 commit into
Conversation
sortDoubtedWindowInputs sorted the input of a window that stays in Spark by the window's own keys only. Spark had already planned the operators above around the longer order the input reported, so a second window ordered by more keys, or a sortWithinPartitions it had removed, saw rows out of order. The sort now uses the whole reported order when it covers the window's keys. Closes apache#7553.
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.
Which issue does this PR close?
Closes #7553.
#6785 should merge after this one: it restores the whole reported order the same way for windows that run natively, and its Spark fallback relies on this rule.
Rationale for this change
When a window stays in Spark over an input whose order Comet may report falsely,
sortDoubtedWindowInputssorts that input again, but only by the window's own keys. Spark had already planned the operators above the window around the longer order the input reported, so a second window ordered by more keys, or asortWithinPartitionsSpark removed as redundant, sees rows out of order. That happens when the input really is out of order past the window's keys (a native nested loop join with more than one build row), and also when it is in order but the inserted sort spills and the merge reorders ties.What changes are included in this PR?
How are these changes tested?
Two tests in
CometInMemoryCacheSuiterun a Spark window ordered byvunder one ordered by(v, w)over a sorted cache joined through a native nested loop join, and check the rows against Spark, the order of asortWithinPartitions("v", "w")above the lower window, and that the only sort in the plan is the one this rule adds, by(v, w). One uses two build rows, the other one build row with a forced spill. Both fail without the change.