andygrove commented on PR #6716: URL: https://github.com/apache/datafusion-comet/pull/6716#issuecomment-6124658524
Thanks @grorge123, that's a real gap, and it wasn't only the shuffle. The native shuffle writer serializes its partitioning keys when the task runs, and `CometTakeOrderedAndProjectExec` does the same for its sort keys and projection, so neither ran inside `withSharedSubexpressions`, and `canShortCircuitNulls` read the missing set as empty. Your query reproduced as described, and so did `SELECT array_contains(ai, CAST(s AS INT)), array_position(ai, CAST(s AS INT)) FROM (SELECT * FROM t ORDER BY i LIMIT 10)`, where Spark's projection after the sort shares the cast. I took both of your suggestions in 3090d859a. `canShortCircuitNulls` now skips no argument outside `withSharedSubexpressions`, which keeps main's eager evaluation for anything serialized after the operator's conversion. The shuffle writer then computes the shared set from the hash or range partitioning keys, which Spark evaluates together in one projection, and `CometTakeOrderedAndProjectExec` from its own expressions, so a cast that only one key holds is still skipped there. `array_null_short_circuit.sql` now has your query and the TakeOrderedAndProject one, which raise as in Spark, plus single-key `DISTRIBUTE BY`, `ORDER BY` and TakeOrderedAndProject queries that return rows. Both shared queries fail at d01ae0a8e, and dropping the shuffle writer's and TakeOrderedAndProject's sets fails the three single-key ones. A `QueryPlanSerdeSuite` test pins the default outside a conversion. -- 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]
