Don't push non-deterministic or volatile filters below a join

[yashmayya]

Problem

PinotFilterJoinRule is a copy-paste fork of Calcite's FilterJoinRule#perform (it exists so filters aren't pushed into the right side of a lookup join). The fork predates CALCITE-7373, fixed in Calcite 1.42.0 — the version this repo already pins. Because the method is forked, the fix was never picked up.

A conjunct like rand() < 0.1 references no input fields, so its InputFinder bitmap is empty. RelOptUtil.classifyFilters treats an empty bitmap as a subset of the left input's fields, classifies the predicate as pushable, and relocates it below the join, where it is evaluated per left-input row instead of per join-output row.

The same gap also fed RelOptUtil.simplifyJoin, which is the more damaging half. On master:

SELECT a.col1, b.col2 FROM a LEFT JOIN b ON a.col1 = b.col1 WHERE b.col3 > 100 * rand()
  ->  LogicalJoin(joinType=[inner])     -- was LEFT

The non-deterministic predicate was treated as null-rejecting, so the left join became an inner join and the null-padded outer rows were dropped entirely. FULL JOIN was likewise simplified to RIGHT. Wrong results, not just a different plan.

Two variability axes, only one of which reaches Calcite

Porting the upstream guard verbatim fixes rand() / UUID_V4 / UUID_V7, but not everything it should, because @ScalarFunction carries two independent notions:

	propagates to Calcite?	examples
isDeterministic = false	yes, via PinotSqlFunction#isDeterministic → RexUtil.isDeterministic	rand(), UUID_V4, UUID_V7
FunctionVolatility.VOLATILE	no	now(), ago(), sleep(), stageId(), workerId(), cid(), startTime(), endTime()

Volatile functions deliberately keep isDeterministic = true so PinotEvaluateLiteralRule can still fold them once at plan time. That means upstream's RexUtil.isDeterministic check does not see them. On master:

SELECT a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 WHERE stageId(a.col1) >= 0
  ->  LogicalFilter(condition=[>=(STAGEID($0), 0)])   pushed into leaf a
  ->  LogicalFilter(condition=[>=(STAGEID($0), 0)])   AND duplicated into leaf b

PinotJoinPushTransitivePredicatesRule infers the predicate for the other side across the join equality, so a per-stage function ends up evaluated independently in two different leaf stages.

Scope of the fix

This closes the case above — where the filter would have been pushed below the join in the first place. It does not make volatile expressions globally immovable, and two gaps are pinned by tests rather than fixed here:

• Transitive duplication from an existing below-join filter. SELECT s.col1, b.col2 FROM (SELECT col1 FROM a WHERE stageId(col1) >= 0) s JOIN b ON s.col1 = b.col1 still duplicates the predicate onto both leaves, because it was never above the join for this rule to hold back.
• Volatile conjunct in an INNER join ON clause. RelOptUtil.pushDownJoinConditions hoists a one-sided call into that input's Project during sql-to-rel, before any rule runs, so the join-condition guard only ever sees a bare RexInputRef. It is load-bearing for outer joins, where the ON clause is preserved.

Both shapes now have plan tests, the inner-join one explicitly labelled KNOWN GAP. Follow-up work for the other relocation sites (PinotJoinPushTransitivePredicatesRule, PinotProjectJoinTransposeRule, and the stock FilterSetOpTransposeRule / FilterAggregateTransposeRule / JoinPushExpressionsRule) is tracked separately.

Fix

• Port the upstream CALCITE-7373 guard (verified byte-for-byte against the Calcite 1.42.0 release artifact).
• Surface volatility on PinotSqlFunction#isVolatile(), plumbed from the existing FunctionInfo volatility that FunctionRegistry already computes. Also mark the hard-coded PinotOperatorTable NOW entry volatile — it shadows the registry entry, so the operator NOW() actually binds to was disagreeing with it.
• Add PinotRuleUtils.isRelocatable(RexNode) and use it in the rule, marked PINOT MODIFICATION so the next Calcite re-diff preserves it. It delegates the determinism half to RexUtil.isDeterministic (so that half tracks upstream automatically) and additionally rejects SqlOperator#isDynamicFunction() — Calcite's own "fold once, never re-evaluate" marker, which is deterministic and would otherwise slip through.

STABLE is intentionally not blocked — a stable function is constant within one query, so relocating it is safe.

No regression for now()-based time filters

This was the main risk, since WHERE ts > now() - <interval> over a join is a very common Pinot pattern. It is unaffected, because now() / ago() are niladic or literal-argument and get constant-folded before these rules run:

WHERE a.ts > now() - 86400000
  ->  LogicalFilter(condition=[>($7, 1786399455434)])   still pushed to the leaf scan

Pinned by QueryCompilationTest#testVolatileNowFilterIsStillPushedBelowJoin (asserted in Java rather than as a plan snapshot, since the folded epoch literal differs every run).

Trade-off (deliberate)

The guard bails on the whole condition rather than per conjunct, matching upstream. So a deterministic conjunct sharing a WHERE clause with a non-deterministic one also stays above the join:

WHERE a.col3 > 5 AND rand() < 0.1   ->  neither conjunct is pushed to the leaf

That costs leaf-stage filtering and shuffles more rows for those queries. Per-conjunct splitting would be finer-grained, but it would be a new deviation in a fork whose drift is the very bug being fixed, and it interacts with the origAboveFilters no-op detection that guards against repeated rule firing. Correctness first; a finer-grained version belongs upstream. Pinned by an explicit test so it can't change silently.

Related: keeping the filter above the join also blocks the semi-join rewrite for IN (subquery) AND rand() < ..., which then plans as an inner join over a distinct aggregate. Also covered by a test.

Tests

JoinPlans.json — 11 in join_planning_tests, 1 in lookup_join_planning_tests:

• non-deterministic WHERE on inner / left join
• non-deterministic WHERE on the null-generating side of a left and a full join (join-type simplification suppressed)
• non-deterministic ON condition (the second guard, reached via JoinConditionPushRule with a null filter)
• uuid_v4(), to show the guard isn't RAND-specific
• volatile stageId() filter neither pushed below the join nor duplicated onto both inputs
• semi-join / IN subquery
• mixed deterministic + non-deterministic conjuncts, pinning the trade-off above
• lookup join, since that's why this fork exists
• volatile ON conjunct kept in the join condition for an outer join
• the INNER-join ON hoisting gap, labelled KNOWN GAP so it is visible rather than implied-fixed
• a control asserting a purely deterministic filter is still pushed to the leaf, so the guard can't silently over-block

Plus PinotRuleUtilsTest (9 cases) covering isRelocatable directly — all three axes, nested operands, STABLE staying relocatable, and both NOW registrations agreeing — the now() pushdown test above, and operator-level volatility aggregation across overloads in pinot-common (rand is the mixed case: rand() VOLATILE, rand(long) IMMUTABLE).

pinot-common 2125, pinot-query-planner 1434, pinot-query-runtime 4491 — all pass. No existing expected plan changed.

Known remaining drift

The same forked method is also missing CALCITE-7319 (correlation-variable handling), also from 1.42.0. It appears inert — Pinot decorrelates before these rules run, and the LogicalCorrelate shapes that survive (UNNEST) have an Uncollect right input, so a $cor-bearing filter never sits directly above a join. Documented in a comment rather than fixed here, to keep this PR single-concern. The comment uses the repo's existing grep-able SYNCED WITH Calcite <version> marker (as in PinotRelDecorrelator) so the next Calcite bump re-diffs this method.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.98%. Comparing base (863b9da) to head (51313a0).
⚠️ Report is 50 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19216      +/-   ##
============================================
+ Coverage     65.69%   66.98%   +1.29%     
  Complexity     1423     1423              
============================================
  Files          3439     3452      +13     
  Lines        218064   218545     +481     
  Branches      34679    34744      +65     
============================================
+ Hits         143255   146398    +3143     
+ Misses        63257    60445    -2812     
- Partials      11552    11702     +150     

Flag	Coverage Δ
custom-integration1	?
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	66.98% <100.00%> (+1.29%)	⬆️
lane-a	100.00% <ø> (?)
lane-b	0.00% <ø> (?)
temurin	66.98% <100.00%> (+1.29%)	⬆️
unittests	66.98% <100.00%> (+1.29%)	⬆️
unittests1	57.67% <100.00%> (+0.65%)	⬆️
unittests2	39.08% <51.85%> (+1.06%)	⬆️

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.

[Copilot]

Pull request overview

Prevents incorrect join-filter relocation for non-deterministic, dynamic, and volatile functions.

Changes:

• Adds volatility metadata and relocatability checks.
• Guards join filter pushdown and join-type simplification.
• Adds comprehensive planner and utility regression tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pinot-common/.../FunctionRegistry.java	Aggregates overload volatility.
pinot-common/.../PinotSqlFunction.java	Exposes operator volatility.
pinot-common/.../FunctionUtilsTest.java	Tests volatility aggregation.
pinot-query-planner/.../PinotFilterJoinRule.java	Blocks unsafe filter relocation.
pinot-query-planner/.../PinotRuleUtils.java	Adds relocatability detection.
pinot-query-planner/.../PinotOperatorTable.java	Marks hard-coded NOW volatile.
pinot-query-planner/.../PinotRuleUtilsTest.java	Tests variability axes.
pinot-query-planner/.../QueryCompilationTest.java	Verifies folded now() pushdown.
pinot-query-planner/.../JoinPlans.json	Adds join-planning regressions and controls.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.