Skip to content

feat: run hll_sketch_estimate natively by default and narrow HLL incompatibility reasons - #7550

Draft
tlfua wants to merge 1 commit into
apache:mainfrom
tlfua:tlfua/feat/7396-hll-audit
Draft

tlfua wants to merge 1 commit into
apache:mainfrom
tlfua:tlfua/feat/7396-hll-audit

Conversation

@tlfua

@tlfua tlfua commented Nov 7, 2026 •

Copy link
Copy Markdown

Which issue does this PR close?

Closes #7396.

Rationale for this change

All four HLL expressions required allowIncompatible because "estimates may differ slightly". That reason was written against datasketches 0.3.0. After the upgrade to 0.6.0 (#7395), I compared Comet's code paths with the DataSketches-Java 6.2.0 logic Spark uses (experiment run, results in #7396 (comment) and #7396 (comment)):

Expression Result vs Spark
hll_sketch_estimate Bit-identical over 954 cases (lgK 4-21, all modes, HLL_4/6/8 inputs)
hll_sketch_agg Sketch bytes differ (compact vs updatable layout). Estimate of a multi-partition merge differs by up to about 0.8%
hll_union Sketch bytes differ. With allowDifferentLgConfigK and different lgK, estimate differs by up to about 1.6%
hll_union_agg Sketch bytes differ. Estimates match

So hll_sketch_estimate can run natively by default, while the other three still produce bytes that differ from Spark and stay Incompatible, now with a reason that says exactly what differs.

Note: the experiment mirrors Spark's Java code and Comet's accumulator logic outside a real Spark/DataFusion plan. The tests in this PR verify end to end in CI.

What changes are included in this PR?

  • CometHllSketchEstimate: now Compatible (no allowIncompatible needed on Spark 4.0+).
  • CometHllSketchAgg, CometHllUnion, CometHllUnionAgg: still Incompatible, with reasons replaced by the concrete differences above. Shared text lives in a new CometHllReasons.
  • The error class difference (Comet execution error vs Spark's HLL_INVALID_INPUT_SKETCH_BUFFER / HLL_UNION_DIFFERENT_LG_K) moves from getIncompatibleReasons to getCompatibleNotes. Both engines fail on the same inputs. The old reason also listed the wrong SQLSTATE for HLL_INVALID_INPUT_SKETCH_BUFFER (22546, not 22000).
  • Docs: expressions.md updated, and audit entries added to expression-audits/agg_funcs.md and misc_funcs.md.

Follow-ups filed by this audit:

How are these changes tested?

  • New SQL file test expressions/misc/hll_sketch_estimate.sql (Spark 4.0+, Parquet dictionary on and off). Sketches are built by Spark for lgK 4/12/21 and 5 to 10000 distinct values, plus empty, NULL, and hll_union_agg results. The estimate query must run natively and match Spark. An invalid-buffer case is query ignore(...) and linked to [Bug] hll_* errors surface as Comet errors instead of HLL_INVALID_INPUT_SKETCH_BUFFER / HLL_UNION_DIFFERENT_LG_K #7549.
    • Dictionary encoding: the native Parquet scan decodes dictionary columns to the required Binary type before any expression sees them (pinned by dictionary_columns_are_unwrapped_to_required_types in native/core/src/parquet/schema_adapter.rs, the same guarantee feat: run length on binary input natively #5874 relied on for length), so spark_hll_sketch_estimate always receives a BinaryArray. The parquet.enable.dictionary=true run of this test checks that end to end.
  • CometExpressionSuite: new test checks that hll_sketch_estimate runs natively with no config and that hll_sketch_agg falls back by default with the compact-layout reason. Existing HLL tests no longer set HllSketchEstimate.allowIncompatible.

Local checks (no Maven build locally, CI runs the suites):

  • scalafmt --config scalafmt.conf --test (v3.6.1) on the changed Scala files: passed.
  • npx prettier --check on the changed Markdown files: passed.

Requesting the run-spark-4.0-tests / run-all-spark-profiles labels, since the serdes only apply on Spark 4.x.

…mpatibility reasons

hll_sketch_estimate now matches Spark bit-for-bit with datasketches 0.6.0,
so it becomes Compatible. hll_sketch_agg, hll_union and hll_union_agg stay
Incompatible with precise reasons: Comet writes LIST/SET mode sketches in
the compact layout (apache#7548)
and merged estimates can differ slightly. The error class mismatch is now a
compatibility note (apache#7549).

Closes apache#7396

Co-authored-by: Copilot <[email protected]>
Copilot-Session: 64723000-41c4-41e1-9042-9efb440c7d08
@github-actions github-actions Bot added the enhancement New feature or request label Nov 7, 2026
@tlfua
tlfua force-pushed the tlfua/feat/7396-hll-audit branch from 9fe68dd to 5124d53 Compare November 7, 2026 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audit the HLL expressions against Spark on datasketches 0.6.0 and revisit the allowIncompatible opt-in

1 participant