sunchao commented on PR #5235:
URL: 
https://github.com/apache/datafusion-comet/pull/5235#issuecomment-5721037435

   Found **one P2 regression** at head `ca51e94b`. I recommend fixing it before 
merging.
   
   **P2 — Preserve empty struct children during normalization** — 
[nested_float_normalize.rs:85](https://github.com/apache/datafusion-comet/blob/ca51e94b124b1864ada8551e2740c90359d993fc/native/spark-expr/src/array_funcs/nested_float_normalize.rs#L85).
   
   Once an enclosing struct contains a float, the helper recursively rebuilds 
every child. An empty struct sibling reaches `StructArray::new`, which rejects 
zero fields and panics.
   
   This is reachable through `_partition` on an unpartitioned Iceberg table:
   
   ```sql
   SELECT arrays_overlap(
     array(named_struct('x', x, 'p', _partition)),
     array(named_struct('x', x, 'p', _partition)))
   FROM t;
   ```
   
   With ordinary double values `1.0` and `2.0`, Spark returns `[true, true]`. 
Comet fails with `CometNativeException`. I reproduced this end to end and 
verified both the Iceberg scan and projection executed natively. The native 
expression pipeline succeeds with the merge-base implementation and fails with 
this PR.
   
   Return subtrees without float leaves unchanged **at each recursive call**, 
or explicitly preserve empty structs. I verified the recursive guard fixes this 
case while retaining signed-zero and NaN normalization.
   
   Validation: **723 exact-head Rust tests passed**, along with additional 
slicing, null, and comparison checks. Existing CI is green. The failing Iceberg 
test used the verified CI merge build, whose affected code matches the reviewed 
head.
   


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