Skip to content

Support CPC and tuple sketch aggregations in materialized views - #19371

Open
davecromberge wants to merge 1 commit into
apache:masterfrom
permutive-engineering:feature-contrib/mv-sketch-aggregations
Open

Support CPC and tuple sketch aggregations in materialized views#19371
davecromberge wants to merge 1 commit into
apache:masterfrom
permutive-engineering:feature-contrib/mv-sketch-aggregations

Conversation

@davecromberge

Copy link
Copy Markdown
Member

Materialized views currently cover HLL and theta sketches. This adds CPC and integer-sum tuple.

It also registers the Raw variants as user-facing functions. A query that asks for the merged
sketch itself, rather than a cardinality, previously matched no rewrite rule and always fell
through to the base table — even when an MV stored exactly that sketch.

All three families merge by union, so re-aggregating the stored sketches gives the same answer as
scanning the base table. One stored raw tuple column also serves the cardinality, sum and average
variants, since they share an accumulator and differ only in how the final value is extracted.

Nothing changes unless materialized view rewrite is enabled, which is off by default.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.43%. Comparing base (d40dceb) to head (29b6fee).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19371      +/-   ##
============================================
- Coverage     67.44%   67.43%   -0.01%     
+ Complexity     1430     1424       -6     
============================================
  Files          3485     3485              
  Lines        223874   223874              
  Branches      35300    35300              
============================================
- Hits         150987   150978       -9     
- Misses        60890    60907      +17     
+ Partials      11997    11989       -8     
Flag Coverage Δ
integration ?
integration1 ?
integration2 ?
java-25 67.43% <ø> (-0.01%) ⬇️
lane-a ?
lane-b ?
temurin 67.43% <ø> (-0.01%) ⬇️
unittests 67.43% <ø> (-0.01%) ⬇️
unittests1 57.53% <ø> (-0.01%) ⬇️
unittests2 39.30% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang Jackie-Jiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. I found two correctness blockers and one required end-to-end coverage gap:

  1. Default-precision CPC/tuple MVs can be selected for higher-precision queries even though merging cannot recover discarded precision.
  2. The raw-Theta self-rule also matches filtered/post-aggregation shapes that cannot generally be reconstructed from an already-collapsed sketch.
  3. Per the repository testing policy, these new query semantics need an end-to-end materialized-view integration test.

I also left comments requesting expression-level rewrite assertions and a Javadoc correction. All current CI checks are green, but they do not exercise the new MV sketch paths end to end.

new SketchMergeEquivalence("DISTINCTCOUNTTHETASKETCH", "DISTINCTCOUNTRAWTHETASKETCH",
"DISTINCTCOUNTTHETASKETCH")
"DISTINCTCOUNTTHETASKETCH"),
new SketchMergeEquivalence("DISTINCTCOUNTCPCSKETCH", "DISTINCTCOUNTRAWCPCSKETCH",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] Enforce stored sketch precision. A default CPC or tuple MV matches a user query requesting higher precision because generic operand matching accepts trailing literals. The rewrite preserves the requested precision, but already-downsampled sketches cannot recover it. Compare effective query and MV precision—including defaults and numeric/string forms—and reject rewrites when query precision exceeds stored precision. Please cover result and raw-self rules with higher/equal/lower-precision tests.

"AVGVALUEINTEGERSUMTUPLESKETCH"),

/// Sketch-based: user wants the merged sketch itself, MV stores the same raw sketch
new SketchMergeEquivalence("DISTINCTCOUNTRAWTHETASKETCH", "DISTINCTCOUNTRAWTHETASKETCH",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] Restrict raw Theta self-rewrites. This self-rule also matches Theta's filtered/post-aggregation forms because their filters and set expression are trailing literals. Substituting one already-collapsed MV sketch cannot reconstruct those filtered sets and may reference columns absent from the MV. Restrict matching to the simple-union shape with compatible parameters.

"DISTINCTCOUNTTHETASKETCH"),
new SketchMergeEquivalence("DISTINCTCOUNTCPCSKETCH", "DISTINCTCOUNTRAWCPCSKETCH",
"DISTINCTCOUNTCPCSKETCH"),
new SketchMergeEquivalence("DISTINCTCOUNTTUPLESKETCH", "DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] Add end-to-end MV sketch coverage. These new query semantics currently have only catalog/registry unit coverage. Add integration cases that build the sketch MVs, compare rewrite-enabled results with a base-table baseline, assert the intended MV was selected, and cover CPC plus tuple cardinality, sum, average, and raw results.

{"DISTINCTCOUNTHLLPLUS", "DISTINCTCOUNTRAWHLLPLUS"},
{"DISTINCTCOUNTTHETASKETCH", "DISTINCTCOUNTRAWTHETASKETCH"}
{"DISTINCTCOUNTTHETASKETCH", "DISTINCTCOUNTRAWTHETASKETCH"},
{"DISTINCTCOUNTCPCSKETCH", "DISTINCTCOUNTRAWCPCSKETCH"},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Assert generated rewrite expressions. The added cases check only rule existence and split safety; they never invoke rewrite(). Assert the resulting operator, MV-column operand, and optional trailing literals for every new CPC, tuple, and raw-self rule so an incorrect constructor mapping cannot pass.

/// - `DISTINCTCOUNTTUPLESKETCH`, `SUMVALUESINTEGERSUMTUPLESKETCH`,
/// `AVGVALUEINTEGERSUMTUPLESKETCH` / `DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH`
///
/// Each raw variant is also registered user-side, so a query wanting the merged sketch itself can

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Narrow the raw-variant claim. Raw HLL and HLLPlus are not registered as user-side self-merge rules, so ‘Each raw variant’ overstates the supported surface. Explicitly name Theta, CPC, and integer-sum tuple raw variants.

@Jackie-Jiang Jackie-Jiang added enhancement Improvement to existing functionality materialized-view labels Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality materialized-view

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants