perf: Improve query planning to more reliably fall back to columnar shuffle when native shuffle is not supported #1209
Conversation
This file contains 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
andygrove
force-pushed
the
choose-better-shuffle
branch
from
2540893 to
a1d5dc9
Compare
andygrove
changed the title
viirya
reviewed
Jan 1, 2025
Comment on lines
-813
to
-814
| case s: ShuffleExchangeExec | ||
| if isCometShuffleEnabled(conf) && isCometJVMShuffleMode(conf) && |
There was a problem hiding this comment.
Hmm, if it is auto mode, why this is not triggered?
There was a problem hiding this comment.
This part is triggered, but in the following code, transform1(s) returns None so we return the original shuffle.
val newOp = transform1(s)
newOp match {
case Some(nativeOp) =>
// Switch to use Decimal128 regardless of precision, since Arrow native execution
// doesn't support Decimal32 and Decimal64 yet.
conf.setConfString(CometConf.COMET_USE_DECIMAL_128.key, "true")
val cometOp = CometShuffleExchangeExec(s, shuffleType = CometNativeShuffle)
CometSinkPlaceHolder(nativeOp, s, cometOp)
case None =>
s
}
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 #1208
Rationale for this change
For TPC-H q16, Comet was falling back to Spark shuffle when native shuffle was not supported, rather than falling back to Comet Columnar Shuffle.
Here are timings for 5 runs of q16:
Main Branch
This PR
What changes are included in this PR?
Improve query planning logic to try native first, then columnar, then Spark.
The original code had separate match arms for native shuffle and columnar shuffle. If the native shuffle arm was chosen and then it was determined that the input plan was not native, we would fall back to Spark.
The new code has a single match arm that tries native first, then columnar, before falling back to Spark.
How are these changes tested?