fix(cubesql): Allow SQL pushdown for views spanning several data sources

[MazterQyou]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Description of Changes Made

This PR allows SQL pushdown for queries over views whose cubes span several data sources, giving such a scan one wrapper context per data source it reaches and pushing a member only into the context of its own data source, so a pushed down select completes only when everything it references shares one.

Notes

• A scan over a view spanning data sources is now explored under one wrapper context per data source it reaches, so planning for such views costs somewhat more than before, when they were never wrapped at all. A select referencing members of another data source stops at the first one, so the extra work is bounded.
• Queries that reference members of two data sources are unchanged: grouped ones stay a plain scan, which a rollup join can still serve, and ones that need pushdown are still refused at rewrite with "Can't detect Cube query".
• Single data source deployments are unaffected: the scan reaches one data source, one context is produced, and every member fits it.

[claude]

Claude finished @MazterQyou's task in 4m 45s —— View job

---

Review: approach is sound; 0 blocking, 1 test-coverage gap, 3 nits (4 inline comments)

Full review

• Gather context / read diff
• Review core changes (ctx.rs, wrapper rules, language node type)
• Trace data-source consumers to check the widened bound is safe everywhere
• Review tests
• Post review

What the change does

WrapperReplacerContext::input_data_source goes from Option<String> to Option<Vec<String>>, so a scan over a view whose members span data sources carries all of them as an open bound instead of being refused pushdown outright. WrapperRules::get_data_source classifies that into a new InputDataSource::{Unrestricted, Specific, OneOf}, and can_rewrite_template requires the template in every candidate for OneOf. The real source is resolved from referenced members at SQL generation, which already errors on a genuine conflict.

Correctness

The safety argument holds up. I traced every consumer of the bound during rewrite: data_source_to_sql_generator is only reached from rules/wrapper/mod.rs:221 inside can_rewrite_template (wrapper.rs uses it too, but that is generation time, after the source is resolved), so the all-of check is the complete gate — no rewrite decision reads dialect specifics behind its back. Canonicalisation is right as well: data_sources_for_member_names sorts and dedups, and the node is only constructed from it (cube_scan_wrapper.rs:184), so two contexts naming the same sources unify in the e-graph rather than splitting the class.

Two things I checked and consider fine:

• Joins. Both sides share ?input_data_source, so an OneOf could in principle resolve per side at generation time. In practice a push-to-Cube join collapses into one ungrouped scan and generate_sql collects members from the join condition (wrapper.rs:3915), so a condition spanning both sources hits DataSourceError::Conflict. Worth confirming you reached the same conclusion, since it is the one shape the tests do not exercise.
• The Unrestricted fallback. A wrapped query over a multi-source view that references only synthetic fields falls through to data_source_for_cube_names, and data_source_for_cube_name documents that "the first member with a known data source wins" — an arbitrary pick. That is pre-existing, and the all-of template check means whatever is picked can render the SQL, so it is not a new bug; it is just newly reachable now that these views get wrapped. Flagging in case it deserves a follow-up.

InputDataSource::OneOf mapping Some([]) to Unrestricted is defensive-only (the builder maps empty to None), which is fine.

Findings

#	Severity	Where	What
1	medium (test coverage)	test_wrapper.rs:4422	The missing-template test strips the template from both sources, so any semantics would pass it too — the all rule that is the core of this PR is untested
2	low	union.rs:139	Set operations over a multi-source view now silently lose pushdown; correct, but unpinned by any test
3	low	cube_scan_wrapper.rs:131	10-line comment restating what mod.rs and the InputDataSource doc already say
4	nit	ctx.rs:162	sort_unstable(); dedup() is simpler than linear contains + sort

Testing / CI

Rust CI was still running when I looked; I did not build locally (a cubesql cargo check is far slower than this job's budget). The new test_data_sources_for_member_names unit test covers dedup, ordering, synthetic fields, and the missing-member error — good shape.

Security / performance

Nothing security-relevant: no new input parsing, no auth path touched. Performance-wise the extra allocation is a small Vec per transform invocation over a handful of data sources, and the widened bound admits more rewrite alternatives for multi-source views — a deliberate cost the PR description already calls out.

· branch `cubesql/sql-push-down-view-multiple-data-sources`

[codecov]

Codecov Report

❌ Patch coverage is 95.91078% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.30%. Comparing base (007e565) to head (498a40a).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
...t/cubesql/cubesql/src/compile/test/test_wrapper.rs	94.71%	13 Missing ⚠️
...ubesql/src/compile/rewrite/rules/wrapper/column.rs	87.50%	3 Missing ⚠️
...compile/rewrite/rules/wrapper/cube_scan_wrapper.rs	94.11%	3 Missing ⚠️
...sql/src/compile/rewrite/rules/wrapper/aggregate.rs	88.88%	2 Missing ⚠️
...l/cubesql/src/compile/rewrite/rules/wrapper/mod.rs	96.29%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11802      +/-   ##
==========================================
- Coverage   84.67%   80.30%   -4.37%     
==========================================
  Files         261      500     +239     
  Lines       86648   106354   +19706     
  Branches        0     3886    +3886     
==========================================
+ Hits        73366    85409   +12043     
- Misses      13282    20395    +7113     
- Partials        0      550     +550     

Flag	Coverage Δ
cube-backend	60.16% <ø> (?)
cubesql	84.74% <95.91%> (+0.07%)	⬆️

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.

[claude]

Claude finished @MazterQyou's task in 2m 22s —— View job

---

Re-review after force-push: all 4 prior findings addressed; 0 blocking, 1 nit (1 inline comment)

Full review

• Gather context / read updated diff
• Check which prior review threads are addressed
• Re-review core changes
• Review new tests
• Resolve stale threads / post review

Prior round

All four are addressed; I resolved the threads.

#	Prior finding	Now
1	Missing-template test stripped the template from both sources, so any semantics would pass it	get_test_tenant_ctx_with_multi_data_source_view_and_templates takes per-data-source templates, and test_wrapper_multi_data_source_view_template_missing_in_one_source_stays_post_processed removes expressions/window_function from other only. That test fails under any — the all rule is now pinned
2	union.rs losing pushdown for multi-source views was untested	test_wrapper_union_over_multi_data_source_view_not_pushed_down asserts LogicalPlan::Union with two plain scans
3	10-line comment in cube_scan_wrapper.rs	Trimmed to the three load-bearing lines; mod.rs holds the canonical explanation
4	contains + sort_unstable in ctx.rs	Now sort_unstable(); dedup()

Re-check of the core

Nothing regressed in the rewrite. I re-traced every get_data_source caller (aggregate_function, case, like_expr, between_expr, in_list_expr, literal ×2, udaf_function, subquery, is_null_expr, …) — all of them feed straight into can_rewrite_template, so the all-of check for OneOf remains the complete gate; no rewrite decision reads dialect specifics behind it. data_sources_for_member_names still sorts and dedups, and the node is only built from it, so contexts naming the same sources unify rather than splitting the e-class. Option<Vec<String>> is already an established node field shape (date_range, mod.rs:384), so the language change needs nothing extra.

Two behavior-preserving details worth confirming rather than flagging:

• cube_scan_wrapper.rs:164 maps an empty list to None, exactly where the old code mapped DataSource::Unrestricted to None. Same semantics; the only real change is that a conflict is no longer an Err.
• union.rs:143 uses var_iter + find_map rather than singular_eclass! like get_data_source does, so an e-class holding both a singleton and a multi-source e-node would take the singleton and push down. The old flat_map(..).next() had the same laxness and I could not construct a plan that reaches it, so this is pre-existing, not new.

The Unrestricted fallback I raised last round still stands as a possible follow-up (not this PR): a wrapped query over a multi-source view referencing only synthetic fields falls through to data_source_for_cube_names, where "the first member with a known data source wins" is an arbitrary pick. The all-of template check means whatever is picked can render the SQL, so it is not a bug — just newly reachable.

Findings

#	Severity	Where	What
1	nit	test_wrapper.rs:4478	logical_plan.find_cube_scan_wrapped_sql_deep(); is an assertion written as a bare call that only holds because the helper panics — asserting on the SQL says what the test means

One thing I looked at and decided not to file: a custom_templates entry naming a data source that does not exist is silently ignored by the helper. Both call sites fail loudly in that case (the removal is what causes the post-processing they assert), so the vacuous-pass risk is not real today.

Testing / CI

Rust CI was still in progress when I looked (Build, Build native, Drivers tests all in_progress, no failures). I did not build locally — a cubesql cargo check exceeds this job's budget. The unit test test_data_sources_for_member_names covers dedup, ordering, synthetic fields, and the missing-member error; the six wrapper tests cover pushdown, the genuine cross-source failure, the plain-scan path, both template-gap shapes, and union.

Security / performance

Unchanged from last round: no new input parsing, no auth path touched, one small Vec per transform invocation, and the widened bound admits more rewrite alternatives for multi-source views — the deliberate cost the PR description calls out.

· branch `cubesql/sql-push-down-view-multiple-data-sources`

[claude]

Claude finished @MazterQyou's task in 1m 12s —— View job

---

Re-review after push: last nit addressed, nothing new — 0 findings, no inline comments

Full review

• Read updated diff
• Check prior thread (bare find_cube_scan_wrapped_sql_deep() nit)
• Re-review core changes
• Review tests
• Resolve stale thread / post review

Prior round

The one open nit is addressed; I resolved the thread. test_wrapper_multi_data_source_view_template_missing_in_one_source_stays_post_processed no longer ends in a bare call — it now binds the node and asserts what the test actually means:

let wrapped = logical_plan.find_cube_scan_wrapped_sql_deep();
assert!(!wrapped.wrapped_sql.sql.contains("OVER"), ...);
let measures = wrapped.request.measures.expect(...);
assert!(measures.len() == 1 && measures[0].contains("PatchMeasure"), ...);

That reads better than my suggestion did: it pins both halves of the shape — the window function stayed above the wrapper, and the filtered measure below it still went to Cube — rather than only the absence of OVER. Both helpers exist and return owned nodes (test/utils.rs:63, :90), so nothing is dropped.

Re-check of the core

This push is the previous diff plus that test change; the rewrite side is byte-identical to what I reviewed last round, and I re-confirmed the invariants rather than assuming them:

• get_data_source still classifies None | Some([]) → Unrestricted, Some([one]) → Specific, else OneOf, and every caller of it (aggregate_function.rs:142, case.rs:99, and the rest) feeds straight into can_rewrite_template. So the all-of check for OneOf remains the complete gate — no rewrite decision reads dialect specifics behind it.
• data_sources_for_member_names sorts and dedups (ctx.rs:165-166), and cube_scan_wrapper.rs:177 is the only place the node is constructed, so contexts naming the same sources unify in the e-graph instead of splitting the class.
• Specific now goes through is_some_and, which keeps the old "missing generator ⇒ cannot rewrite" answer; empty list → None sits exactly where DataSource::Unrestricted → None used to. The only real semantic change is that a conflict is no longer an early Err.

Two standing observations, neither a finding on this PR:

• union.rs:143 uses var_iter + find_map rather than singular_eclass!, so an e-class holding both a singleton and a multi-source e-node would take the singleton and push down. The old flat_map(..).next() had the same laxness and I could not construct a plan that reaches it — pre-existing.
• The Unrestricted fallback: a wrapped query over a multi-source view referencing only synthetic fields falls through to data_source_for_cube_names, where "the first member with a known data source wins" is an arbitrary pick. Not a bug (the all-of template check means whatever is picked can render), just newly reachable — possible follow-up.

Testing

Seven tests now cover the change: test_data_sources_for_member_names (dedup, ordering, synthetic fields, missing-member error) plus pushdown, the genuine cross-source failure, the plain-scan path, both template-gap shapes, and union over the multi-source view. Rust CI was in progress when I looked, no failures; I did not build locally — a cubesql cargo check exceeds this job's budget.

Comments / docs

The cube_scan_wrapper.rs comment is 3 lines and load-bearing, mod.rs:494-498 holds the canonical explanation of the field, and the InputDataSource variants are documented without re-spelling their types. Nothing to flag.

Security / performance

Unchanged: no new input parsing, no auth path touched, one small Vec per transform invocation, and the widened bound admits more rewrite alternatives for multi-source views — the deliberate cost the PR description calls out.

· branch `cubesql/sql-push-down-view-multiple-data-sources`

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[claude]

Claude finished @MazterQyou's task in 5m 0s —— View job

---

Re-review after redesign: approach is sound — 0 blocking, 1 test-coverage gap, 3 low/nits (4 inline comments)

Full review

• Read updated diff
• Check prior review threads (all 5 already resolved by the author's force-push)
• Re-review the core rewrite change
• Trace the member gate for completeness
• Review tests
• Check CI
• Post review

What changed since my last review

This is a different design, not an increment. WrapperReplacerContext::input_data_source stays Option<String>; instead wrapper-cube-scan-wrap becomes a multi-transform (transforming_rewrite_multi / MultiTransformingPattern, rewrite/mod.rs:832,2722) that emits one substitution — one wrapper context — per data source the scan reaches. Members are then filtered into their own context by member_fits_data_source (rules/wrapper/mod.rs:241), gated at four sites: column.rs pushdown_dimension / pushdown_simple_measure, aggregate.rs pushdown_measure_impl / transform_filtered_measure. union.rs and can_rewrite_template are untouched, which follows: a context is bound to exactly one data source again, so every existing single-source check keeps working unchanged.

I like this considerably better than the OneOf bound. The template check is exact rather than pessimistic (..._template_missing_in_other_source_pushes_down now asserts a pushdown where the previous revision refused one), the set-operation path needs no change, and the Unrestricted fallback I flagged twice — an arbitrary "first member with a known data source wins" becoming newly reachable — is gone, since the context always names the data source of the members it took.

Correctness

I traced the gate for completeness, since an ungated path would put a foreign member into a bound context and turn a working plain scan into a hard Conflict at generation:

• Every member reference in a PushToCube:true context has to come through a column resolution, and all three of those are gated (pushdown_dimension covers dimensions/time dimensions/segments/virtual fields, so filters and ordering expressions are covered too since they reach members as columns).
• wrapper-push-down-column (PushToCube:false) is correctly left alone — those columns name the inner query's output, not Cube members.
• transform_powerbi_max_case (aggregate.rs:1353) resolves a member and is not gated. That is right: its output is literal_int(0), so the member never appears in the pushed-down select and contributes no data source. Worth confirming you reached the same conclusion, since it is the one member resolution in the diff's neighbourhood without a gate.
• member_fits_data_source mapping Err(Missing) to "does not fit" only loses a pushdown that generation would have failed anyway (data_source_for_member_names over every used member is already fatal at wrapper.rs:3925), and cube_scan_wrapper already bailed on Missing before this PR. No new regression.
• MultiTransformingPattern neither overrides vars() nor dedups the ids it returns, matching TransformingPattern right above it; Rewrite::new's var check is vacuous for both.

Behaviour-preserving details I checked rather than flagged: hoisting push_to_cube_out and taking .next() on alias_to_cube / ungrouped instead of looping is the same answer, because the old loop unconditionally return trued on its first iteration. data_sources.is_empty() ⇒ vec![None] sits exactly where DataSource::Unrestricted ⇒ None used to.

Findings

#	Severity	Where	What
1	medium (test coverage)	test_wrapper.rs:4308	Every positive pushdown assertion reads default only, which sorting makes the first emitted context — a bug that produced only the first substitution would leave all of them green
2	low	test_wrapper.rs:4410	Doc comment still describes the abandoned OneOf design ("open until SQL generation", "has to hold for every data source"), and now contradicts the test right below it
3	low	column.rs:200-210	Data-source mismatch return falses out of the transform where aggregate.rs continues; context_data_source also sits inside the per-column loop
4	nit	cube_scan_wrapper.rs:169	7-line comment whose last five lines restate the member_fits_data_source doc

Testing / CI

Unit (Rewrite Engine) and Check fmt/clippy both pass. The three Build native failures are all aarch64-unknown-linux-gnu and unrelated — aarch64-linux-gnu-gcc: error: unrecognized command line option '-m64' and a missing python3.12 interpreter in the cross image. I did not build locally; a cubesql cargo check exceeds this job's budget.

Nine tests cover the change: test_data_sources_for_member_names (dedup, ordering, synthetic fields, missing-member error) plus single-source pushdown, the cross-source refusal, the plain-scan path, both template shapes (now one negative and one positive, which is the improvement), union pushed down on one source, union refused across sources, and the cross-source query that must stay a plain scan so a rollup join can still serve it. The per-data-source custom_templates helper is a good shape; a custom_templates entry naming a nonexistent data source is still silently ignored, but both call sites fail loudly in that case, so the vacuous-pass risk is not real today.

Security / performance

No new input parsing, no auth path touched. The multi-transform is the real cost: a scan over an N-source view is now explored under N contexts instead of not being wrapped at all, and members that do not fit stop each one early — the bound the PR description states, and the added tests confirm the cross-source shapes terminate in a plain scan rather than exploding.

· branch `cubesql/sql-push-down-view-multiple-data-sources`

[claude]

Claude finished @MazterQyou's task in 2m 20s —— View job

---

Re-review after push: all 4 prior findings addressed, nothing new — 0 findings, no inline comments

Full review

• Read updated diff
• Check the 4 open prior threads against the new code
• Re-review the core rewrite change
• Review tests
• Check CI
• Resolve stale threads / post review

Prior round

All four are addressed; I resolved the threads.

#	Prior finding	Now
1	Every positive pushdown assertion read default only, so a bug emitting just the first substitution would stay green	test_wrapper_filtered_measure_over_multi_data_source_view_second_source asserts a PatchMeasure on MultiSourceView.agentCount from COUNT(DISTINCT CASE WHEN content = 'error' THEN agentCount END) — the second context is now pinned as real
2	Doc comment still described the abandoned OneOf design and contradicted the test below it	Rewritten to three lines that describe this design ("bound to the data source of the members the query references"), and no longer conflicts with ..._template_missing_in_other_source_pushes_down
3	return false out of the whole transform where aggregate.rs continues; context_data_source inside the per-column loop	Both fixed — column.rs:134 hoists the read above for column in columns.iter(), and both gates (:203, :250) continue. aggregate.rs hoisted its two reads the same way, so all four sites now read alike
4	7-line comment in cube_scan_wrapper.rs restating member_fits_data_source	Trimmed to the three load-bearing lines; the guard it explains (data_sources.len() > 1 && !push_to_cube_out) is right below it

Re-check of the core

The rewrite side is otherwise the diff I reviewed last round, and I re-confirmed the invariants rather than assuming them:

• The member gate is still complete. I re-enumerated every member resolution reachable from a PushToCube:true wrapper context: column.rs:190 / :249 and aggregate.rs:1233 / :1487 are all gated. aggregate.rs:1387 (transform_powerbi_max_case) is still the one ungated resolution, and still correctly so — its applier is alias_expr(literal_int(0), ..) (:511), so the measure never reaches the pushed-down select and contributes no data source. wrapper-push-down-column stays untouched, which is right: those columns name the inner query's output, not Cube members.
• Hoisting did not widen the gate. context_data_source depends on nothing in either loop, and the Err(()) it can return is the same "no generator for this data source" case can_rewrite_template already answers false for (mod.rs:259), so bailing early loses no rewrite that could have generated SQL.
• The &'meta str trick holds. get_key_value borrows the key from meta instead of the e-graph, which is what lets the value live across egraph.index_mut(..) in pushdown_measure_impl. The one-line comment at each call site is the right amount for a non-obvious borrow.
• Canonicalisation. data_sources_for_member_names sorts and dedups (ctx.rs:165-166), and cube_scan_wrapper.rs is the only constructor of the node, so contexts naming the same source unify rather than splitting the e-class. MultiTransformingPattern neither overrides vars() nor dedups returned ids, matching TransformingPattern right above it.

Worth noting the design now matches the repo's own rewrite guidance (rust/cubesql/CLAUDE.md): the cross-node fact — every member reaching the same data source — is enforced by reusing one ?input_data_source pattern variable and letting unification do it, rather than by a transform comparing several matched nodes.

Testing / CI

Nine tests cover the change: test_data_sources_for_member_names (dedup, ordering, synthetic fields, missing-member error), single-source pushdown on both data sources, the cross-source refusal, the plain-scan path, both template shapes (one negative, one positive), union pushed down within one source, union refused across sources, and the cross-source grouped query that must stay a plain scan so a rollup join can still serve it.

CI for this push was queued/in progress when I looked (Rust, Build, Build native, Drivers tests), no failures yet; the previous revision's Unit (Rewrite Engine) and Check fmt/clippy both passed. I did not build locally — a cubesql cargo check exceeds this job's budget.

Security / performance

Unchanged: no new input parsing, no auth path touched. The multi-transform remains the real cost — an N-source view is explored under N contexts — and hoisting context_data_source out of the loops shaves a per-alternative hash lookup off each of the four gates.

· branch `cubesql/sql-push-down-view-multiple-data-sources`