dwsmith1983 opened a new pull request, #5866:
URL: https://github.com/apache/datafusion-comet/pull/5866

   ## Which issue does this PR close?
   
   Closes #5605.
   
   ## Rationale for this change
   
   Native shuffle accepted a struct column with duplicate field names, and the 
task then died at the JVM Arrow FFI import because Java Arrow keys a struct 
vector's children by name and collapses the two into one. The issue's second 
reproduction, a local table scan of the same struct, showed the shuffle 
predicate is not the only gap: the scan builds the struct through Java Arrow as 
well, so even with the shuffle on Spark the plan fails in columnar-to-row when 
it reads the second child of a one-child vector. Any Comet operator that turns 
Spark rows into Arrow on the JVM has the same limitation.
   
   ## What changes are included in this PR?
   
   - The shared type gate in `DataTypeSupport` declines a struct with duplicate 
field names at any depth, with the fallback reason `struct with duplicate field 
names`. `CometLocalTableScanExec` and `CometSparkToColumnarExec` use it, so 
neither builds the struct any more and both reproductions in the issue fall 
back to Spark with a correct answer.
   - The native shuffle type predicate gains the distinct-name clause the 
columnar predicate already has. With the expression serde, the local table scan 
and row-to-columnar all declining the shape first, no plan reaches this clause 
today; it is kept so the two shuffle predicates agree, which #5021 tracks 
consolidating.
   
   ## How are these changes tested?
   
   Four tests in `CometNativeShuffleSuite`:
   - the local table scan reproduction from the issue, which before the change 
chose a Comet exchange and then crashed, now falls back with the type-gate 
reason;
   - a unit test that both row conversion sinks decline a top-level duplicate 
struct with that reason, that the local scan also declines `array<struct<a, 
a>>` and `map<_, struct<a, a>>` through the gate's recursion, and that a struct 
whose names differ only by case stays supported;
   - a unit test that feeds a synthetic native child with a duplicate struct 
straight into `shuffleSupported`, which reports `unsupported shuffle data 
type`, while the case-distinct struct is accepted as native shuffle; this test 
fails when only the shuffle clause is reverted;
   - the cached-relation reproduction, which now plans zero Comet exchanges and 
returns Spark's answer.
   
   Native shuffle and both concrete columnar shuffle suites pass (138 tests), 
as do the exec and native reader suites (252 tests) on Spark 3.5 against the 
rebuilt library.
   


-- 
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]

Reply via email to