davecromberge commented on code in PR #19371:
URL: https://github.com/apache/pinot/pull/19371#discussion_r3948887094
##########
pinot-materialized-view/src/main/java/org/apache/pinot/materializedview/rewrite/equivalence/AggregationEquivalenceRegistry.java:
##########
@@ -48,7 +48,23 @@ public final class AggregationEquivalenceRegistry {
new SketchMergeEquivalence("DISTINCTCOUNTHLL", "DISTINCTCOUNTRAWHLL",
"DISTINCTCOUNTHLL"),
new SketchMergeEquivalence("DISTINCTCOUNTHLLPLUS",
"DISTINCTCOUNTRAWHLLPLUS", "DISTINCTCOUNTHLLPLUS"),
new SketchMergeEquivalence("DISTINCTCOUNTTHETASKETCH",
"DISTINCTCOUNTRAWTHETASKETCH",
- "DISTINCTCOUNTTHETASKETCH")
+ "DISTINCTCOUNTTHETASKETCH"),
+ new SketchMergeEquivalence("DISTINCTCOUNTCPCSKETCH",
"DISTINCTCOUNTRAWCPCSKETCH",
Review Comment:
Fixed in fb92ae6965: `operandsCompatible` now normalizes each family to
nominal-entries space and rejects a rewrite when the query precision exceeds
the stored sketch precision, with higher/equal/lower tests on both the result
and raw-self rules.
##########
pinot-materialized-view/src/main/java/org/apache/pinot/materializedview/rewrite/equivalence/AggregationEquivalenceRegistry.java:
##########
@@ -48,7 +48,23 @@ public final class AggregationEquivalenceRegistry {
new SketchMergeEquivalence("DISTINCTCOUNTHLL", "DISTINCTCOUNTRAWHLL",
"DISTINCTCOUNTHLL"),
new SketchMergeEquivalence("DISTINCTCOUNTHLLPLUS",
"DISTINCTCOUNTRAWHLLPLUS", "DISTINCTCOUNTHLLPLUS"),
new SketchMergeEquivalence("DISTINCTCOUNTTHETASKETCH",
"DISTINCTCOUNTRAWTHETASKETCH",
- "DISTINCTCOUNTTHETASKETCH")
+ "DISTINCTCOUNTTHETASKETCH"),
+ new SketchMergeEquivalence("DISTINCTCOUNTCPCSKETCH",
"DISTINCTCOUNTRAWCPCSKETCH",
+ "DISTINCTCOUNTCPCSKETCH"),
+ new SketchMergeEquivalence("DISTINCTCOUNTTUPLESKETCH",
"DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH",
+ "DISTINCTCOUNTTUPLESKETCH"),
+ new SketchMergeEquivalence("SUMVALUESINTEGERSUMTUPLESKETCH",
"DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH",
+ "SUMVALUESINTEGERSUMTUPLESKETCH"),
+ new SketchMergeEquivalence("AVGVALUEINTEGERSUMTUPLESKETCH",
"DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH",
+ "AVGVALUEINTEGERSUMTUPLESKETCH"),
+
+ /// Sketch-based: user wants the merged sketch itself, MV stores the
same raw sketch
+ new SketchMergeEquivalence("DISTINCTCOUNTRAWTHETASKETCH",
"DISTINCTCOUNTRAWTHETASKETCH",
Review Comment:
Fixed in 61d2c7d3af: theta rewrites are now rejected when either side
carries the filter/post-aggregation shape (>=4 operands), restricting matching
to the simple-union form.
##########
pinot-materialized-view/src/main/java/org/apache/pinot/materializedview/rewrite/equivalence/AggregationEquivalenceRegistry.java:
##########
@@ -48,7 +48,23 @@ public final class AggregationEquivalenceRegistry {
new SketchMergeEquivalence("DISTINCTCOUNTHLL", "DISTINCTCOUNTRAWHLL",
"DISTINCTCOUNTHLL"),
new SketchMergeEquivalence("DISTINCTCOUNTHLLPLUS",
"DISTINCTCOUNTRAWHLLPLUS", "DISTINCTCOUNTHLLPLUS"),
new SketchMergeEquivalence("DISTINCTCOUNTTHETASKETCH",
"DISTINCTCOUNTRAWTHETASKETCH",
- "DISTINCTCOUNTTHETASKETCH")
+ "DISTINCTCOUNTTHETASKETCH"),
+ new SketchMergeEquivalence("DISTINCTCOUNTCPCSKETCH",
"DISTINCTCOUNTRAWCPCSKETCH",
+ "DISTINCTCOUNTCPCSKETCH"),
+ new SketchMergeEquivalence("DISTINCTCOUNTTUPLESKETCH",
"DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH",
Review Comment:
Added in 1d54cfc8c0: MaterializedViewClusterIntegrationTest now builds
CPC/theta/tuple sketch MVs and asserts MV selection plus base-table baseline
equality across the cardinality, sum, average and raw shapes.
##########
pinot-materialized-view/src/test/java/org/apache/pinot/materializedview/rewrite/equivalence/AggregationEquivalenceRegistryTest.java:
##########
@@ -70,7 +70,11 @@ public void testSketchMergeRulesAreSplitSafe() {
String[][] pairs = {
{"DISTINCTCOUNTHLL", "DISTINCTCOUNTRAWHLL"},
{"DISTINCTCOUNTHLLPLUS", "DISTINCTCOUNTRAWHLLPLUS"},
- {"DISTINCTCOUNTTHETASKETCH", "DISTINCTCOUNTRAWTHETASKETCH"}
+ {"DISTINCTCOUNTTHETASKETCH", "DISTINCTCOUNTRAWTHETASKETCH"},
+ {"DISTINCTCOUNTCPCSKETCH", "DISTINCTCOUNTRAWCPCSKETCH"},
Review Comment:
Added in fb92ae6965 (SketchMergeEquivalenceTest): each new CPC, tuple and
raw-self rule now asserts rewrite() produces the expected operator, MV-column
operand and trailing literals.
##########
pinot-materialized-view/src/main/java/org/apache/pinot/materializedview/rewrite/equivalence/SketchMergeEquivalence.java:
##########
@@ -40,6 +40,12 @@
/// - `DISTINCTCOUNTHLL` / `DISTINCTCOUNTRAWHLL`
/// - `DISTINCTCOUNTHLLPLUS` / `DISTINCTCOUNTRAWHLLPLUS`
/// - `DISTINCTCOUNTTHETASKETCH` / `DISTINCTCOUNTRAWTHETASKETCH`
+/// - `DISTINCTCOUNTCPCSKETCH` / `DISTINCTCOUNTRAWCPCSKETCH`
+/// - `DISTINCTCOUNTTUPLESKETCH`, `SUMVALUESINTEGERSUMTUPLESKETCH`,
+/// `AVGVALUEINTEGERSUMTUPLESKETCH` /
`DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH`
+///
+/// Each raw variant is also registered user-side, so a query wanting the
merged sketch itself can
Review Comment:
Fixed in 9c16c95c29: the Javadoc now names only the Theta, CPC and
integer-sum tuple raw variants and states that the HLL/HLLPlus raw variants are
not registered.
--
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]
