fix(tesseract): render FILTER_PARAMS on the measure side of a join back

[waralexrom]

Fixes #11740.

Problem

When a measure that needs the full-key plan is requested together with a dimension of a one_to_many joined cube, the planner builds a keys subquery joined back to a second copy of the fact source by primary key.

The keys-side copy is planned with the query's filters, so the cube's FILTER_PARAMS bindings render as real predicates. The measure side was planned with no filter context at all, so every binding collapsed to always-true:

  FROM (SELECT * FROM orders
        WHERE (tenant_id = $1) AND (created_at >= $2 AND created_at <= $3)
       ) AS "orders_key_orders"              -- keys side: restricted
  LEFT JOIN (SELECT * FROM orders
             WHERE 1 = 1 AND 1 = 1           -- measure side: both bindings gone
            ) AS "orders_key_orders"
    ON "keys"."orders__id" = "orders_key_orders".id

Results stayed correct — the keys side restricts the output — but the database built the join against the entire unfiltered fact table: all tenants, all time. On a large fact table the hash build outgrows the memory limit and the query fails.

The legacy planner is unaffected: its FILTER_PARAMS proxy reads the query-level allFilters regardless of which sub-select is being rendered, so both copies come out restricted. CUBEJS_TESSERACT_SQL_PLANNER=false is therefore a workaround, but the legacy planner is scheduled for removal.

Cause

AggregateMultipliedSubquery's outer select and MeasureSubquery's select carry no WHERE clause of their own — the keys subquery already restricts the rows. In Tesseract the filter set that FILTER_PARAMS / FILTER_GROUP bindings resolve against is taken from the select's WHERE filter, so for these two selects it was empty and every binding fell back to always_true.

What changed

• SelectBuilder now takes the filters its sources' FILTER_PARAMS / FILTER_GROUP bindings resolve against separately from its WHERE clause (set_filter_params_filters), defaulting to the WHERE filter as before.
• Both sources of the join back set it from the keys subquery's own filter, so the two copies of the fact source cannot drift apart:
  • AggregateMultipliedSubquerySource::Cube — the bare cube;
  • AggregateMultipliedSubquerySource::MeasureSubquery — carried on the logical node, which the planner fills from the keys subquery it just built. This branch had the same gap; it is reached by a measure whose filters: reach another cube.
• Removed SelectBuilder::new_from_select, which had no callers and would have carried the two filter sets inconsistently.

No WHERE clause is added anywhere, and no other plan shape changes.

On the reporter's proposal

@icoolguy1995 suggested pushing the subset of the query's filters whose members belong to the key cube into the bare cube source. That is the right diagnosis, and this change is the narrower form of it: rather than synthesising a predicate over the key cube's members, it lets the cube's existing FILTER_PARAMS bindings resolve, which is what the legacy planner does and what the issue's SQL actually shows missing. It needs no member-ownership filtering (a binding renders the column stated at the binding site, never the member's own SQL, so a filter on another cube simply matches no binding) and cannot produce a reference to a cube that is not joined on the measure side.

Why this cannot change results

Both copies read the same fact rows over the same columns, and the join back is by primary key. The keys side applies pushdown ∩ WHERE, the measure side applies pushdown only, so the measure side is always a superset of the key set the join looks up: no matched row can disappear and no LEFT JOIN can turn into a NULL. Cumulative and rolling measures are planned outside this branch, so no window-widening semantics are involved.

How it was verified

• New unit tests assert both fact copies render the same predicates, for both sources of the join back. A plain count, rewritten to COUNT(DISTINCT id) over a single filtered copy, anchors them.
• New Postgres integration tests state the equivalence in numbers: the same model with and without the bindings answers the same, for both branches.
• Both new SQL tests were confirmed to fail before the fix (1 = 1 AND 1 = 1) and pass after.
• Full planner suite: 1377 tests, unit plus Postgres integration, all passing. cargo fmt, cargo clippy --all-targets clean.

Risks

Low. The only behaviour change is that FILTER_PARAMS / FILTER_GROUP bindings reached from these two selects now resolve instead of rendering 1 = 1 — that filter set feeds nothing but those bindings. A model whose cube sql uses a binding negated (WHERE NOT (...)) would see its measure side change from empty to filtered, which brings it in line with the keys side and with the legacy planner.

🤖 Generated with Claude Code

[claude]

Claude finished @waralexrom's task in 3m 49s —— View job

---

Review: sound, narrowly scoped fix — no blocking issues (1 medium, 3 low)

Full review

• Read the diff
• Review SelectBuilder filter-params change
• Review join-back planner wiring
• Review tests
• Post review

Verdict

The diagnosis and the fix both hold up. Separating "the filters bindings resolve against" from "the WHERE clause" is the right seam: VisitorContext's all_filters is documented as existing solely to feed FILTER_PARAMS/FILTER_GROUP (physical_plan/visitor_context.rs:18), so widening what these two selects pass there cannot add or remove rows on its own.

I checked the two claims the "cannot change results" argument rests on, since both are load-bearing:

1. A binding renders the binding-site column, not the member's SQL — confirmed. SqlCall::eval_filter_group builds VisitorContext::new_for_filter_params with a symbol→binding-column map (planner/sql_call.rs:534-547), so a filter on a cube that is not joined on the measure side simply matches no binding.
2. An OR group cannot leak a foreign cube's SQL into the bare-cube copy — confirmed. FilterItem::find_subtree_for_members_inner returns None for an OR group unless every child matches the target members (planner/filter/tree.rs:175-186), so a mixed OR group falls back to always_true on both copies rather than rendering half of it.

Also verified: MeasureSubquery's new filter field is not carried by inputs()/with_inputs incorrectly (it is cloned, and no transform or optimizer rewrites a node's LogicalFilter in place, so it cannot drift from keys_subquery.filter()); the added pretty_print line breaks no snapshot (no committed snapshot renders the logical plan); and SelectBuilder::new_from_select really has no remaining callers (the surviving new_from_select hit is Cte::new_from_select, unrelated).

The query_filter() extraction is a genuine dedup — the two inline LogicalFilter literals were identical, and having one source makes the "both copies see the same filters" invariant structural rather than coincidental.

I could not run cargo test in this environment (the sandbox declined the compound cd && cargo test invocation), so the test results below are read, not executed. The seed data and both new snapshots do check out by hand: tag a → orders 1,2 → count 2 / buyers 1 = 2 and 100+200 = 300 vip; tag b → orders 1,3 → 2 / 2 = 1 and 100 vip (order 3's buyer is not VIP). The June order and the t2 order sit outside the range on purpose, which is what makes the pushdown/full-scan equivalence meaningful rather than vacuous.

Findings

#	Sev	Where	Issue
1	Medium	physical_plan/builder/select.rs:354	filter_params_filters.or(self.filter) replaces the WHERE filter for binding resolution. Fine for both current callers (neither has a WHERE), but a future caller setting both would silently lose its own predicates from binding resolution — the same failure mode this PR fixes. Suggested a debug_assert! to pin the invariant. (inline)
2	Low	processors/aggregate_multiplied_subquery.rs:112-116	Not part of the diff, so noted here: the Cube branch's join_visitor_context is still built with all_filters = None. A primary-key dimension whose sql uses a FILTER_PARAMS binding would therefore render 1 = 1 on the measure side of the ON clause while the keys side renders the real predicate — the two sides of the join key stop being the same expression. Exotic (a binding inside a PK dimension), but it is one line from the fix and the same class of asymmetry. Worth either passing the same filters or a one-line note that PK expressions are assumed binding-free.
3	Low	processors/measure_subquery.rs:51-56	The same five-line rationale is written three times (field declaration + both processors). One statement at the field, one-liners at the call sites. (inline)
4	Low	tests/filter_params_multiplied_join_back.rs:172-177	params[6..9] vs params[0..3] pins the whole statement's parameter layout; an unrelated reordering panics or silently compares the wrong pair. (inline)

Test coverage

Good shape overall: two positive cases (one per AggregateMultipliedSubquerySource variant), a negative anchor (a_count_measure_needs_no_second_copy) that keeps the positive tests honest about actually reaching the join back, and end-to-end numeric equivalence on Postgres. Confirming both SQL tests fail before the fix is the right thing to have done.

Two gaps, neither blocking:

• FILTER_GROUP is untested. It is named in every comment and in the API doc, and it takes a different code path than a plain FILTER_PARAMS binding (eval_filter_group → find_subtree_for_members, where the OR-group rule above lives). A single FILTER_GROUP variant of the bare-cube test would cover the branch that the correctness argument actually leans on.
• No case with an OR filter spanning two cubes, which is the one shape where the subtree search decides between rendering and always_true. The reasoning is right; a test would keep it right.

Minor: assert_pushdown_is_result_neutral returning Option means the Postgres tests pass silently when no database is available. That matches the existing convention in this suite, so no change requested — just noting that the numeric half of the verification is conditional in CI.

• branch `tesseract-join-back-filter-params-pushdown`

[claude]

.or(self.filter) makes filter_params_filters a replacement, not an addition. Both current callers set it only on selects that have no WHERE of their own, so this is correct today — but the next caller that sets both will silently stop resolving bindings against its own WHERE predicates, which is exactly the class of bug this PR fixes (a binding quietly falling back to always_true).

Worth making the invariant explicit rather than relying on the doc comment, e.g.:

debug_assert!(
    self.filter_params_filters.is_none() || self.filter.is_none(),
    "filter_params_filters replaces the WHERE filter for binding resolution"
);

or merging the two item lists instead of choosing one.

[claude]

The same five-line rationale now appears three times — here, on MeasureSubquery::filter in logical_plan/measure_subquery.rs:10-14, and in aggregate_multiplied_subquery.rs:190-195. One load-bearing sentence at the field declaration (the one place a later edit would need it: "not a WHERE clause — the keys subquery restricts the rows; both fact copies must render the same predicate") is enough; the call sites read fine with a one-liner or nothing.

Suggested change

	// The enclosing keys subquery already restricts the rows, so this
	// select carries no WHERE of its own - but its source cubes must still
	// see the query's filters, or a `FILTER_PARAMS` binding in a cube's
	// `sql` falls back to always-true and the join is built against the
	// whole unfiltered fact table.
	select_builder.set_filter_params_filters(measure_subquery.filter.all_filters());
	// Not a WHERE of its own - see `MeasureSubquery::filter`.
	select_builder.set_filter_params_filters(measure_subquery.filter.all_filters());

[claude]

params[6..9] vs params[0..3] hard-codes the parameter layout of the whole statement (three params per copy, second copy starting at index 6 — i.e. three params emitted in between). Any unrelated change to parameter emission order turns this into either a panic on slice bounds or a comparison of the wrong pair, with a message that still says "both copies must be bound to the same values".

Since the assertions above already prove each copy renders the same predicate shape, consider deriving the slices instead of pinning them, e.g. asserting params.len() and comparing the last three against the first three, or asserting the multiset of values contains t1 twice and each timestamp twice.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.67%. Comparing base (007e565) to head (fc89e89).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #11814   +/-   ##
=======================================
  Coverage   84.67%   84.67%           
=======================================
  Files         261      261           
  Lines       86648    86648           
=======================================
  Hits        73366    73366           
  Misses      13282    13282           

Flag	Coverage Δ
cubesql	84.67% <ø> (ø)

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:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.