Visorgood commented on PR #6110:
URL:
https://github.com/apache/datafusion-comet/pull/6110#issuecomment-6106789109
Rebased onto main and addressed both points.
The `CometExpressionSuite` conflict took main's side of both hunks, as you
suggested – the file is now identical to main and has dropped out of this PR. I
also found a fourth stale comment you hadn't listed: the one above `range
partitioning on a nested floating-point key` claimed `CometSortOrder` declines
the key "for nested floating point under strict mode". After #6476 that is no
longer the rule – `canHoldNestedNull` has to hold first. The test passes only
because the parquet struct's fields are nullable, and the comment now says so.
On cost: four shapes, two runs each, 8M rows on an M3 Pro. The baseline arm
switches Comet shuffle off, which reproduces the pre-#5971 plan inside one
build; comparing against branch-1.1 directly would also carry every unrelated
commit between us.
run 1
run 2
range, Comet scan +5.7% +2.9%
range, row-based Spark scan -1.6% -1.5%
hash, Comet scan -11.7% -11.1%
hash, row-based Spark scan -4.2% -6.9%
One shape is consistently slower, three are consistently faster. The
direction reproduces, the magnitude does not – run-to-run drift is wider than
the within-run stdev, so I'd call it 1.5-6% rather than pin a number. That is
close to the ~2% you saw on the aggregate case.
Two things worth flagging:
The slow shape is the one with a *native* child, not the one with Spark on
both sides. Keeping the probe when the child is not a Comet plan would preserve
the regression and give up the wins.
And Spark-on-both-sides is narrower than it looks:
`columnarShuffleFailureReasons` takes a child only if it is row-based or a
Comet operator, so a Spark columnar scan is declined outright. That case only
arises with a row-based child, where this change is faster.
Added the harness as `CometUnserializableShuffleKeyBenchmark` so this can be
re-run. Happy to put the probe back for a narrower case if you'd rather, but on
these numbers I'd leave it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
