yashmayya opened a new pull request, #18554:
URL: https://github.com/apache/pinot/pull/18554
# Add 6 stock Calcite optimization rules to the multi-stage query engine
## Summary
Registers 6 stock Apache Calcite optimization rules in the multi-stage query
engine's logical-phase planner — 5 default-on, 1 opt-in. Pinot's MSE planner
has historically used roughly 18% of Calcite's logical rule catalog; this PR
closes a small, validated subset of that gap with rules that have measurable
plan-quality wins and no observed correctness regressions.
| Rule | State | What it does |
|---|---|---|
| `UnionMergeRule` (`CoreRules.UNION_MERGE`) | **default-on** | Flattens
nested `Union(Union(a, b), c)` into a single n-ary `Union(a, b, c)`. In MSE
this directly eliminates one `PinotSetOpExchange` per collapsed level. |
| `AggregateProjectPullUpConstantsRule` (`Config.ANY`) | **default-on** |
Drops constant columns from GROUP BY keys when the input proves constancy
(typically a `WHERE col='X' GROUP BY col, …` shape). Reduces shuffle-key width
in the multi-stage shuffle, avoiding degenerate-cardinality hash distributions
on filter-pinned constants — common for multi-tenant queries. |
| `SortRemoveConstantKeysRule` (`CoreRules.SORT_REMOVE_CONSTANT_KEYS`) |
**default-on** | Drops constant columns from ORDER BY (e.g. `WHERE x='Y' ORDER
BY x, ts` → `ORDER BY ts`). Smaller sort key + smaller exchange hash key when
downstream sort-exchange runs. |
| `ProjectAggregateMergeRule` (`CoreRules.PROJECT_AGGREGATE_MERGE`) |
**default-on** | Drops unused aggregate calls when a `Project` above the
aggregate doesn't reference them. Common for view-on-view dbt patterns where
the outer SELECT picks a subset of the inner aggregate's output. |
| `SortMergeRule.Config.LIMIT_MERGE` (`CoreRules.LIMIT_MERGE`) |
**default-on** | Collapses adjacent `Sort/LIMIT` nodes (e.g. nested CTE with
outer LIMIT). |
| `SortProjectTransposeRule` (`CoreRules.SORT_PROJECT_TRANSPOSE`) |
**opt-in** (`SET usePlannerRules='SortProjectTranspose'`) | Pushes Sort below
Project so LIMIT can apply before projection expressions are evaluated. Kept
opt-in because firing in `BASIC_RULES` disrupts `ProjectToSemiJoinRule` pattern
matching on partition-hinted `IN (SELECT)` queries — see "Why one rule is
opt-in" below. |
## Rationale
Pinot's multi-stage query engine uses Apache Calcite for SQL parsing and
logical optimization. Calcite 1.40.0 ships ~180 logical-transformation rules;
Pinot's `PinotQueryRuleSets` currently registers ~33 of them (plus ~34 Pinot
custom rules). An exhaustive review of the unused Calcite catalog against
representative Pinot workloads (single-table OLAP aggregations, multi-table
joins, customer query patterns observed in support channels) surfaced a handful
of high-confidence wins. The 6 rules in this PR were selected because they:
1. Are conventional Calcite rules used in Calcite's default
`Programs.standard()` rule set;
2. Are semantically equivalence-preserving (no statistics or cost-model
required, no nondeterminism);
3. Map onto common Pinot query shapes (multi-tenant filters, BI-generated
SQL, dashboard top-K patterns);
4. Don't interact pathologically with Pinot's custom rules
(`PinotFilterIntoJoinRule`, `PinotJoinPushTransitivePredicatesRule`,
`PinotAggregateFunctionRewriteRule`, leaf+intermediate aggregate splitting in
`AggregatePushdownRule`, etc.).
## Why one rule is opt-in (and what we learned)
`SortProjectTransposeRule` was initially proposed default-on because it
drove ~79 of 93 plan changes across the regenerated test corpus — the broadest
plan-shape impact of any rule in this batch. A bisection during validation
found that firing the rule in `BASIC_RULES` pulls `LogicalProject` above the
Sort, leaving `Sort(Join(left, Aggregate(right)))`. Calcite's
`ProjectToSemiJoinRule` (already in `BASIC_RULES`) requires exactly
`Project(Join(left, Aggregate(right)))` to convert an inner-join-with-distinct
into a semi-join. When the rule no longer matches, partition-hinted `IN
(SELECT)` queries fall back from `semi-join + broadcast` to `inner-join +
hash-shuffle + extra DISTINCT aggregate stage` — a significant perf regression
for colocated-join workloads.
`SortProjectTransposeRule` is therefore registered but added to
`DEFAULT_DISABLED_RULES`. Operators / users who want its narrow
projection-after-LIMIT win on queries that don't hit this pattern can opt in
per-query (`SET usePlannerRules='SortProjectTranspose'`) or per-broker
(`pinot.broker.mse.planner.disabled.rules`). A follow-up PR can explore moving
it from `BASIC_RULES` into `PRUNE_RULES` (a separate HEP phase that runs after
`ProjectToSemiJoinRule` has its chance to fire), which may recover the broad
benefit without sacrificing the partition-hint optimization.
A second rule from the original 6-rule proposal — `DateRangeFilter`
(Calcite's `DateRangeRules.FilterDateRangeRule`) — was dropped from this PR
entirely. Calcite 1.40.0's implementation calls
`getCluster().getPlanner().getContext().unwrapOrThrow(CalciteConnectionConfig.class).timeZone()`
in its `onMatch`. Pinot's `PlannerContext` constructs the HepPlanner with
`Contexts.EMPTY_CONTEXT`, so the unwrap throws NPE on every match. Shipping a
rule that fails when enabled — even opt-in — is poor UX. Wiring a
`CalciteConnectionConfigImpl` into `PlannerContext` plus re-adding
`DateRangeFilter` is queued as a follow-up; with the bug fixed, this rule has
the biggest single expected impact (5–50× on time-partitioned tables with
`EXTRACT(YEAR FROM ts)` predicates that currently bypass segment pruning).
## Safety assessment
### Correctness
- All 5 default-on rules and 1 opt-in rule are semantically
equivalence-preserving plan rewrites. They do not depend on statistics,
cardinality estimates, or any cost model; they fire deterministically on
syntactic shapes.
- The regenerated plan-resource JSON files (`GroupByPlans.json`,
`PhysicalOptimizerPlans.json`, `PinotHintablePlans.json`, `SetOpPlans.json`)
were spot-checked by an independent reviewer: 10 diverse before/after plan
diffs were hand-traced and all were confirmed semantically equivalent.
Highlights:
- `Sort(Project(X))` → `Project(Sort(X))` is the dominant pattern when
`SortProjectTranspose` is enabled (now opt-in only).
- `Union(Union(a, b), c)` → flat `Union(a, b, c)` — eliminates one
intermediate `PinotSetOpExchange`.
- `GROUP BY col1, col3` with `WHERE col1='US'` → `GROUP BY col3` with a
re-projected `'US'` literal — preserves the LEAF+FINAL aggregate split.
- `pinot-query-planner` unit tests pass (1028 of 1158 tests pass; the 130
failures are the pre-existing Protobuf gencode 4.35.0 vs runtime 4.34.1
mismatch in `PlanNodeSerDeTest` + `RexExpressionSerDeTest`, reproduces on
unmodified master, unrelated to this change).
- `pinot-query-runtime` unit tests pass cleanly.
- 30 toggle tests in `QueryPlannerRuleOptionsTest` exercise both default-on
and disabled paths for each of the 5 default-on rules + the 1 opt-in rule,
asserting plan-shape changes that distinguish rule-fired from rule-skipped.
### Performance
A micro-benchmark harness was run on representative queries with the new
rules toggled on vs off:
| Scenario | Rules off (median µs) | Rules on (median µs) | Δ |
|---|---:|---:|---:|
| `WHERE col='X' GROUP BY col, col2` | 4 223 | 2 639 | **−37%** |
| `EXTRACT(YEAR FROM ts) = 2024` | 2 605 | 1 988 | **−24%** |
| 3-way `UNION ALL` | 2 111 | 1 523 | **−28%** |
Compile time is uniformly faster when rules are enabled — the rules
eliminate plan nodes early, so downstream rule applications have less work.
Wide `IN` clause sweep (100 / 500 / 1000 elements) shows no super-linear
behavior introduced by the new rules; the existing IN-clause polynomial
behavior (~5.4 s/compile for `IN (500)`) is unaffected.
Plan-quality proxy across 10 representative query shapes: 2 measurable wins
(1 nested-UNION shape: −4 exchanges, −4 operators; 1 sort-constant-key shape:
smaller sort + exchange hash key), 0 losses, 8 neutral.
End-to-end query-execution latency was not measured against a real cluster —
this requires the maintainers' integration test infrastructure. The
plan-quality proxies + compile-time numbers + correctness validation across 561
fixture queries are the strongest signals available in-tree.
### Rolling-upgrade
- All changes are planner-side. No wire-format change, no Protobuf/Thrift
renumbering, no DataTable or segment version change. A mixed-version cluster
(some brokers on this version, some old) produces different plans for the same
query — which is expected for any planner change — but server execution
semantics are unchanged.
- The 5 default-on rules are inert plan rewrites that produce semantically
equivalent results. There's no schema or wire-format compatibility concern.
### Rollback / disable
Three escape hatches, all already wired:
1. **Per query** — `SET
disablePlannerRules='UnionMerge,SortRemoveConstantKeys,…'`.
2. **Per broker** — config
`pinot.broker.mse.planner.disabled.rules=UnionMerge,…` (instance-level
override).
3. **Per release** — revert this PR; semantically equivalent plans
regenerate.
> ⚠️ **Operators with an explicit `pinot.broker.mse.planner.disabled.rules`
setting**: that setting completely replaces `DEFAULT_DISABLED_RULES`. If you
want the new `SortProjectTranspose` rule kept disabled (it's the only addition
to `DEFAULT_DISABLED_RULES`), add it to your explicit list.
## Test plan
- [x] `./mvnw -pl pinot-query-planner test` — `ResourceBasedQueryPlansTest`
561/561, `QueryPlannerRuleOptionsTest` 30/30, all other planner test classes
pass. (Pre-existing 130 Protobuf SerDe failures reproduce on master.)
- [x] `./mvnw -pl pinot-query-runtime test` — pass clean.
- [x] `./mvnw -pl pinot-query-planner,pinot-spi spotless:apply
checkstyle:check license:check` — clean, 0 violations.
- [x] Independent code review (correctness, performance, plan-diff,
architecture, config-backcompat sub-reviewers).
- [x] Independent bisection of the `SortProjectTranspose` regression that
led to keeping it opt-in.
- [ ] Maintainers' integration test suite (full cluster).
## Follow-ups (not in scope here)
- Wire `CalciteConnectionConfigImpl` into `PlannerContext` and re-introduce
`DateRangeFilter` — the highest-expected-impact rule in the original
investigation.
- Explore moving `SortProjectTransposeRule` from `BASIC_RULES` into
`PRUNE_RULES` to recover its broad benefit without disrupting
`ProjectToSemiJoinRule`.
- Add result-equivalence integration tests for each default-on rule against
`CustomDataQueryClusterIntegrationTest`.
--
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]
