dwsmith1983 commented on PR #6835: URL: https://github.com/apache/datafusion-comet/pull/6835#issuecomment-6124165241
Thanks for the right outer cases and the switch to sameRows, they read well. A few notes on f74ae8f0c: - A note for anyone picking this up for review: it still has field 11 and the NOT IN case, so it needs the rebase onto #6810 before it merges. I'll post here once #6810 is up. - The aggregate test (CometInMemoryCacheSuite.scala:3378) counts one Comet hash join of either class. Pinning (class, buildSide) through the joins helper, as the first test does, would show the right outer run is BuildLeft. It also catches a false order only while both aggregates sit in the join's native plan: a partial aggregate that splits a key gets merged again by a final hash aggregate after a shuffle. Asserting there's no exchange between the join and the final aggregate would keep it meaningful if the partitioning changes. - The comment in CometHashJoin (operators.scala around 2874) says every other join type keeps the probe order. The null-aware anti join built on the right is the exception #6810 sorts for. - For the NOT IN test: with AUTO_BROADCASTJOIN_THRESHOLD=-1 Spark plans NOT IN as a nested loop join, so it needs the default threshold or a broadcast hint. - The new fallback applies whenever the order can't be serialized, even when nothing above the join depends on it, so a join that ran natively before now goes back to Spark. Is that intended? - hash_join_declares_streamed_order_on_probe_input could assert the planned join type is Right, since its comment says both shapes run that way. -- 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]
