tlfua opened a new issue, #7548:
URL: https://github.com/apache/datafusion-comet/issues/7548

   ### Describe the bug
   
   On Spark 4.x, Comet's native `hll_sketch_agg`, `hll_union` and 
`hll_union_agg` return sketch bytes that differ from Spark's when the sketch is 
still in LIST or SET mode (a small number of distinct values).
   
   Spark returns `HllSketch.toUpdatableByteArray`, which is documented as the 
"updatable binary representation". The datasketches crate (0.6.0) has only a 
compact `serialize()`. For LIST and SET it sets `COMPACT_FLAG` and writes the 
compact layout (`list.rs:131`, `hash_set.rs:163`).
   
   Both layouts decode to the same sketch, so `hll_sketch_estimate` and 
`hll_union` read either one correctly and the estimates match. The bytes 
themselves still differ, so:
   
   - any query that compares, groups by, hashes, or stores the raw sketch bytes 
gets a different result under Comet;
   - DataSketches-Java `HllSketch.writableWrap` rejects Comet's output because 
it only accepts updatable images (`HllSketch.java:251-254` in 6.2.0).
   
   This is why these three expressions still need 
`spark.comet.expression.<name>.allowIncompatible=true` after the audit in 
https://github.com/apache/datafusion-comet/issues/7396 .
   
   ### Steps to reproduce
   
   ```sql
   -- Spark 4.1, few distinct values -> LIST mode
   SELECT hex(hll_sketch_agg(col)) FROM VALUES (1), (2), (3) AS t(col);
   ```
   
   Run it once with Comet disabled and once with 
`spark.comet.expression.HllSketchAgg.allowIncompatible=true`. The hex strings 
differ: Comet sets `COMPACT_FLAG` and writes only the coupons, while Spark 
writes the updatable layout. `hll_sketch_estimate` returns 3 in both cases.
   
   Comparison experiment (DataSketches-Java 6.1.1 and 6.2.0 vs Comet with 
datasketches 0.6.0, 4030 cases): 
https://github.com/tlfua/datafusion-comet/actions/runs/42348769378 . Summary: 
https://github.com/apache/datafusion-comet/issues/7396#issuecomment-6703600973 .
   
   ### Expected behavior
   
   Comet writes the same updatable layout as Spark, so the bytes match and 
these expressions can run without the opt-in.
   
   Possible fixes:
   
   1. Add an updatable serialization to datasketches-rust 
(https://github.com/apache/datasketches-rust ) and use it in Comet.
   2. Re-encode LIST/SET sketches into the updatable layout in Comet before 
returning them.
   
   ### Additional context
   
   Surfaced by the HLL audit for 
https://github.com/apache/datafusion-comet/issues/7396 . Affected Comet files: 
`native/spark-expr/src/agg_funcs/hll_sketch.rs`, `hll_sketch_agg.rs`, 
`hll_union_agg.rs`, `native/spark-expr/src/hll_scalar.rs` and the `CometHll*` 
serdes under `spark/src/main/spark-4.x/org/apache/comet/serde/`.
   
   Separately, the estimate after merging partial sketches can differ by up to 
0.81% (`hll_sketch_agg` with more than one partition) because SET-to-HLL 
promotion during a union visits coupons in a different order. Spark's own 
estimate for the same input changes with partitioning by a similar or larger 
amount.
   
   ### Willingness to contribute
   
   I would be willing to contribute a fix for this bug with guidance from the 
DataFusion community
   


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