joelrobin18 opened a new pull request, #57958:
URL: https://github.com/apache/spark/pull/57958
### What changes were proposed in this pull request?
`hll_union_agg` has no `lgConfigK` parameter, so it infers precision from
the sketches it reads. For a group whose sketches are all NULL it never sees
one, and `eval` falls back to a `Union` at the DataSketches default
(`lgConfigK=12`). That empty sketch is observable -- it gets stored -- and
could not afterwards be merged with sketches of any other precision, so the
function produced output it could not itself consume.
This PR makes empty sketches exempt from the precision check, on the grounds
that an empty sketch holds no coupons and therefore carries no precision to
reconcile:
- `HllUnionAgg` gains a `mergeSketch` helper used by both `update` and
`merge`. It skips `compareLgConfigK` unless both sides are non-empty, and
re-seeds an empty `Union` at the first non-empty sketch's `lgConfigK`. Routing
both paths through one helper matters because the failure reproduced through
`merge` as well, so fixing only `update` would leave the partial-to-final
aggregation path broken.
- Scalar `HllUnion` gets the same exemption, and picks the non-empty side's
`lgConfigK` instead of an unconditional `Math.min` -- otherwise an empty sketch
at 12 would silently downsample a populated sketch at 15.
Merging an empty sketch is now a no-op with respect to precision, which also
makes the result independent of the order rows reach the aggregate.
### Why are the changes needed?
Rolling up stored sketches fails under default settings when any group had
only NULL input:
```
[HLL_UNION_DIFFERENT_LG_K] Sketches have different `lgConfigK` values: 12
and 15.
Set the `allowDifferentLgConfigK` parameter to true to enable unions of
different lgConfigK values.
```
The failure is self-referential: `hll_union_agg` emits a sketch that
`hll_union_agg` then refuses. Reproduction:
```sql
WITH sketches AS (
SELECT 'has_data' AS grp, hll_sketch_agg(CAST(id AS STRING), 15) AS sketch
FROM (SELECT explode(sequence(1, 1000)) AS id)
UNION ALL
SELECT 'all_null' AS grp, CAST(NULL AS BINARY) AS sketch
),
stored AS (SELECT grp, hll_union_agg(sketch) AS sketch FROM sketches GROUP
BY grp)
SELECT hll_sketch_estimate(hll_union_agg(sketch)) FROM stored;
```
The workaround -- setting `allowDifferentLgConfigK = true` -- is a poor fit,
since it also permits genuine precision loss between two populated sketches,
which is the case the check exists to catch. An all-NULL group is common in
practice (a partition with no matching rows), so a user can hit this without
ever having mixed precisions deliberately.
### Does this PR introduce _any_ user-facing change?
Yes, a bug fix. Queries that previously failed with
`HLL_UNION_DIFFERENT_LG_K` when unioning an empty sketch with a
differing-precision sketch now succeed, and the populated sketch's `lgConfigK`
is preserved.
No change for previously-succeeding queries: sketches of equal precision are
unaffected, and two **non-empty** sketches with differing `lgConfigK` still
raise `HLL_UNION_DIFFERENT_LG_K` unless `allowDifferentLgConfigK` is set. One
narrow behavior change worth noting for review: `hll_union(empty@12,
populated@15)` previously downsampled to 12 when `allowDifferentLgConfigK =
true`; it now returns 15, since the empty side has no precision to impose.
### How was this patch tested?
New tests, all passing locally:
- `DatasketchesHllSketchSuite` -- 4 tests covering the aggregate and scalar
`hll_union`, in both merge orders; plus assertions that mismatched
**non-empty** sketches still throw `HLL_UNION_DIFFERENT_LG_K`, and that
`allowDifferentLgConfigK = true` still downsamples.
- `DataFrameAggregateSuite` -- 2 end-to-end SQL tests reproducing the JIRA
scenario, run at 1 and 2 partitions so both `update` and the partial-to-final
`merge` path are exercised.
Existing `hll_sketch_agg` / `hll_union_agg` tests and the `hll.sql` golden
file pass unmodified. The full `sql/catalyst` suite also passes (10531 tests,
380 suites, 0 failures).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code
--
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]
