Visorgood commented on issue #5971:
URL: 
https://github.com/apache/datafusion-comet/issues/5971#issuecomment-5750089628

   Hello @0lai0!
   
   Verified the part you flagged: nothing on the columnar path consumes a 
serialized partitioning.
   
   `prepareJVMShuffleDependency` partitions entirely on the JVM – 
`UnsafeProjection`, `LazilyGeneratedOrdering`, Spark's `RangePartitioner`. 
`CometShuffleDependency.outputPartitioning` is a Catalyst `Partitioning`, not a 
proto message. `PartitioningOuterClass.RangePartition` is built only in 
`CometNativeShuffleWriter` (line 355). That writer is reached only via 
`CometNativeShuffleHandle`; the columnar path gets a different handle.
   
   So your fix holds. The near-identical probe in the native branch (~line 556) 
must stay – there the serialized `SortOrder` really does go native, and it also 
rolls up fallback reasons from the expression tag.
   
   I've confirmed it with a failing test. After #5506 scalar float/double are 
`Compatible`, but `strictFloatingPointReason` uses `containsType` recursively, 
so a `struct<double, int>` sort key still returns `Incompatible` and trips the 
gate:
   
   ```
   - PROBE range partitioning on a struct containing a double *** FAILED ***
       has 0 CometShuffleExchangeExec node which doesn't match the expected: 1
       
       Exchange rangepartitioning(c#293 ASC NULLS FIRST, 4), REPARTITION_BY_NUM
       +- CometProject [c#293], [struct(a, _1#289, b, _2#290) AS c#293]
          +- CometNativeScan parquet [_1#289,_2#290]
   ```
   
   The scan and projection stay on Comet; only the exchange drops to Spark. 
Nothing in the query is unsupported – the gate alone forces the transition. The 
#5506 test next to it still passes.
   
   `take`. My plan: drop the probe from the columnar branch, keep the collation 
check, add this test beside the #5506 one at 
`CometColumnarShuffleSuite.scala:795`, and update that test's comment now both 
halves are covered.


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