fix(cubesql): Prune unused members from inner wrapped select

[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 (if issue reference is not provided)

This PR makes SQL API pushdown project only the columns an outer wrapped select reads from its inner select, instead of every member of the scanned cube or view. Related tests are included.

[claude]

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

---

Review: 1 high, 2 medium, 2 low — 5 inline comments posted

Details

• Read the diff and the surrounding code in wrapper.rs / converter.rs
• Analyze the pruning logic for correctness edge cases
• Review test coverage
• Check prior review threads
• Post findings

Findings

#	Sev	Where	Finding
1	high	wrapper.rs:1633	Dropping members drops cubes from the ungrouped join; under a one_to_many join that changes row multiplicity, so outer COUNT(*) / inner LIMIT results change. DISTINCT is guarded for the analogous reason; fan-out is not. Design question for the author.
2	medium	wrapper.rs:1675	is_sort_key matches by exact Expr equality while the consumer (generate_sql_for_push_to_cube, wrapper.rs:4404) resolves order columns by expr_name. A disagreement prunes a sort key and errors the query with Can't find column … in projection.
3	medium	test_wrapper.rs	No coverage for the all-unused fallback branch (wrapper.rs:1695-1704) — the COUNT(*) test wraps a DISTINCT, which is exempt — and none for dropping a joined cube, the change's stated motivation.
4	low	wrapper.rs:1665	expr_to_columns(...)? propagates (wildcard exprs error), turning a best-effort optimization into a hard query failure; every other bail-out in the function falls back to the unpruned from.
5	low	wrapper.rs:1696	keep.is_empty() is unreachable — keep.iter().all(...) on an empty vec already returned six lines up. Suggestion attached.

What checks out

• The schema-layout assumption (without_window_fields then window fields) matches converter.rs:2358-2370, and the fields.len() != projection_expr.len() + window_expr.len() guard bails when it doesn't.
• Name-only matching for used_names errs toward keeping columns, as the comment says.
• The reference sweep looks complete for this node shape: self.subqueries bails out early, join plans are rendered independently and can only reach from through the join condition, which is in the chain, and inner window exprs were rebased onto from's own columns by the converter so they don't depend on which projection exprs survive.
• Qualifiers and alias survive the rebuild, and the subset schema can't introduce duplicate fields.

Not run

cargo test/cargo check for cubesql — no build cache in this checkout, a cold build of the crate does not fit the job. All findings are from reading; #5 is the only one verifiable purely statically.

Review threads

Listing returned 0 threads of mine (none of any kind on this PR) — nothing to resolve, nothing re-affirmed, no finding skipped as a duplicate.

· branch [`cubesql/prune-unused-members`](https://github.com/cube-js/cube/tree/cubesql/prune-unused-members)

[codecov]

Codecov Report

❌ Patch coverage is 98.26464% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.83%. Comparing base (5dd3e2f) to head (c9abb6d).

Files with missing lines	Patch %	Lines
...t/cubesql/cubesql/src/compile/test/test_wrapper.rs	98.01%	11 Missing ⚠️
...t/cubesql/cubesql/src/compile/engine/df/wrapper.rs	98.63%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #11972       +/-   ##
===========================================
+ Coverage   62.21%   80.83%   +18.61%     
===========================================
  Files         248      509      +261     
  Lines       19979   108427    +88448     
  Branches     4072     4072               
===========================================
+ Hits        12430    87644    +75214     
- Misses       6958    20192    +13234     
  Partials      591      591               

Flag	Coverage Δ
cube-backend	62.21% <ø> (ø)
cubesql	85.03% <98.26%> (?)

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 8m 3s —— View job

---

Review round 2 — 1 medium (re-affirmed), 2 low, 1 comment nit. No blocking defect found in the pruning logic itself.

Full review

• Read the diff and the surrounding code (wrapper.rs, converter.rs, BaseQuery.js templates)
• Analyze correctness / edge cases
• Check tests
• Handle prior review threads
• Post findings

What I checked and found sound

• Schema/expr alignment: converter.rs:2358 builds the wrapped-select schema as projection fields followed by window fields, so zipping from.projection_expr with fields and keeping the tail with chain(repeat(&true)) is correct, and the fields.len() != projection_expr.len() + window_expr.len() guard catches the converter's empty-projection fallback (converter.rs:2268), where projection_expr is empty but the schema is full.
• No panic in the all-unused fallback: an empty projection_expr exits earlier through keep.iter().all(...) on an empty vec, so keep[dimension_index.unwrap_or(0)] can't index out of bounds.
• Downstream rendering doesn't index the schema positionally — generate_column_expr (wrapper.rs:2384) resolves everything by name, and cube_params in make_member_def (wrapper.rs:2296) come from per-column used_members, so dropping a member really does drop its cube from the member defs.
• Name-only matching is safe in the conservative direction, and inner-select fields all share one qualifier, so same-name collisions can't occur within from.schema.

Posted this round

Where	Severity	Finding
wrapper.rs:1661	low	self.group_expr is passed to expr_to_columns raw; a GroupingSet (ROLLUP/CUBE) may not be traversed, and the rest of the file flattens with extract_exprlist_from_groupping_set (wrapper.rs:2111). Also worth confirming expr_to_columns really errors on Expr::Wildcard as the comment claims. (Follow-up reply corrects the suggestion: the helper returns an owned Vec, bind it before chaining.)
test_wrapper.rs:5201	low	The view test asserts dimensions only. request.join_hints is copied unchanged from the original scan (wrapper.rs:4481) and still names Logs; BaseQuery.js:370/:2790 feed those query-level hints into joinGraph.buildJoin, so the join this PR drops members to avoid may survive. Assert the hints, and prune them if they still carry the dropped cube.
wrapper.rs:1630-1637	nit	10-line doc comment; the load-bearing sentence is one, and the fan-out argument belongs next to the guard at line 1649.

Not posted as a new thread: from_with_pruned_projection still never consults from.limit / from.offset, so an inner LIMIT over a dropped one_to_many join returns a different set of rows. That is the same root cause as the still-open thread on wrapper.rs:1638, which explicitly asked about limit/offset; the response was the new doc paragraph, not a guard, so the concern is folded into that thread rather than duplicated.

Review threads

The listing returned 5 of my own threads. Resolved 4 (keep.is_empty() unreachable, is_sort_key exact-Expr equality, expr_to_columns hard failure, missing tests for both fallback branches — all addressed by the current diff). Re-affirmed 1 (PRRT_kwDOCOH19M6lJnaP, row multiplicity under a dropped fan-out join).

Not run

cargo is not permitted in this environment, so the new tests were not executed and the DataFusion-fork behaviour of expr_to_columns on GroupingSet / Wildcard was read from call sites, not verified.

· branch [`cubesql/prune-unused-members`](https://github.com/cube-js/cube/tree/cubesql/prune-unused-members)

[claude]

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

---

Review complete — 1 high re-affirmed (existing thread), 0 new inline comments.

Details

• Read the diff and surrounding code in wrapper.rs
• Analyze pruning correctness and edge cases
• Review test coverage
• Handle prior review threads
• Post findings

Review threads

Listing returned 4 of my own threads. Resolved 3, re-affirmed 1.

Resolved — the current diff addresses them:

• GroupingSet flattening → wrapper.rs:1651 now uses extract_exprlist_from_groupping_set(&self.group_expr).
• Test asserted members but not join hints → test_wrapper.rs now asserts request.join_hints == None.
• Over-long doc comment → now 3 lines at wrapper.rs:1626-1628, with the row-multiplicity argument moved next to the guard it informs.

Re-affirmed (left open, no duplicate posted): the row-multiplicity thread. Both findings I had this round fold into it, so nothing new was posted inline.

Standing concern

Pruning drops a member, which drops the cube it belongs to, which drops its one_to_many join — and that changes the number of rows the inner select returns, not just its columns. The guard at wrapper.rs:1643 covers the one case where that is visible through the inner select itself (from.distinct); it does not cover the two places the same change leaks out:

1. A cardinality-sensitive outer select. Nothing inspects self.select_type, self.distinct, or self.limit. SELECT customer_gender, COUNT(*) FROM OrdersLogsView WHERE <filter Cube can't push> GROUP BY 1 compiles to an outer Aggregate over an inner Projection that projects every view member. Pruning drops Logs.content, so Logs is no longer joined and each count falls from orders × logs to orders. Whether the filter happens to be pushable then decides the answer.

2. An inner limit/offset. from.limit is not part of the guard, and test_wrapper_count_over_filter_keeps_single_dimension prunes an inner carrying LIMIT 10. On a multi-cube view, SELECT DISTINCT customer_gender FROM (SELECT * FROM OrdersLogsView LIMIT 10) t slices 10 rows out of a join-multiplied set before the change and out of an unmultiplied one after, so the two return different gender sets. The is_sort_key carve-out preserves the ORDER BY columns but not the cardinality the LIMIT slices.

The "matches querying those members directly" rationale holds for the SELECT DISTINCT shape this PR targets — it does not extend to either of these, where the row count itself is the result.

Nothing to flag on security. Mechanically the rest checks out: the projection-then-window field order the zip assumes matches how converter.rs:2360 builds the schema; keep.iter().all() on an empty vec returns early, so the keep[dimension_index.unwrap_or(0)] fallback can't index out of bounds; window exprs are rebased onto the input namespace at converter.rs:2337 so they don't depend on the projection columns being pruned.

One low note not worth an inline: is_sort_key calls expr_name(expr, &from.schema) with the select's output schema while expr lives in the input namespace. It falls back to keeping the column on error, so nothing breaks today.

Not verified

cargo check / cargo test were not run — cargo is outside the allowed tool set for this run. Findings above are from static reading only.

· [branch `cubesql/prune-unused-members`](https://github.com/cube-js/cube/tree/cubesql/prune-unused-members)

[claude]

Claude finished @MazterQyou's task in 8m 44s —— View job

---

Reviewed #11972 — approach looks sound; 0 high, 0 medium, 3 low (3 inline). Could not build or run cargo test: cargo isn't in my allowed tools here.

Full review

• Read the diff and both changed files in full
• Traced the pruning path against converter.rs schema construction and the push-to-Cube SQL generation
• Reviewed test coverage
• Handled prior review threads
• Posted findings

What holds up

The core invariant is right, and it's the reason this is safe: every expression of the inner select (filter_expr, order_expr, rebased window_expr) references the CubeScan under it, not its own projection outputs — converter.rs:2294-2360 rebases window and order exprs through replace_map before the node is built. So the only two ways pruning the inner projection can break something are (a) an outer expression whose column was dropped, covered by UsedColumnNamesVisitor over exactly the list expressions() returns, and (b) the push-to-Cube ORDER BY resolution at wrapper.rs:4408-4423, which looks sort keys up in projection_expr by expr_name — covered by is_sort_key using the same comparison.

Guards I checked and found adequate:

Guard	Why it's needed
subqueries.is_empty() (wrapper.rs:1631)	subquery plans hold OuterColumn refs the visitor can't see; Expr::InSubquery's subquery is a Column into that list (converter.rs:1257), so the guard covers it
from.distinct / non-Projection (wrapper.rs:1643)	dropping columns changes duplicate identity
fields.len() == projection_expr.len() + window_expr.len() (wrapper.rs:1647)	keeps the zip aligned and rejects the converter's empty-projection fallback (converter.rs:2269), where fields come from from.schema() and projection_expr stays empty
has_wildcard (wrapper.rs:1667)	unnamed reads
keep-at-least-one (wrapper.rs:1700)	empty select_concat renders SELECT FROM — the template has no * fallback (BaseQuery.js:4662)

keep is never indexed out of bounds: an empty keep short-circuits at the .all() early return. Window fields ride along via chain(repeat(&true)), and they sit after the projection fields in the schema, matching converter.rs:2357. Pruning only runs on the non-push-to-Cube branch of generate_sql (wrapper.rs:4534), which is the right scope — a push-to-Cube outer select merges members into one request and can't have a non-pushed projection under it.

Comment density is within the 3-line rule throughout, and each comment states a why the code can't.

Findings (all low)

1. wrapper.rs:1684 — map_or(true, ...) inside .any(...) makes one unnameable sort expression mark every column a sort key, silently disabling pruning for the whole select; a function-wide bail-out hidden in a per-column predicate.
2. wrapper.rs:1733 — the doc comment says "ungrouped CubeScan" but nothing checks request.ungrouped; find_member_in_ungrouped_scan is a plain name lookup.
3. test_wrapper.rs:5124 — the measure closure spells out seven None fields although V1CubeMetaMeasure derives Default, unlike the dimension closure next to it. (First suggestion block had an off-by-one range; corrected in a follow-up comment on the same lines — apply that one.)

Tests

Six new tests cover the pruned case, the DISTINCT stop, the sort-key retention, the keep-one-dimension fallback, the cross-cube view drop (asserting join_hints == None), and the filter/no-filter equivalence that pins the actual bug. test_wrapper_limit_over_filter_reads_same_members_as_without_filter is the one that fails without the fix. Not covered, and worth considering: an inner select carrying window_expr (the chain(repeat(&true)) branch), and an inner projection with an aliased expression that the outer reads by alias.

One note on scope rather than a defect: join_hints are copied from the scan node's request (wrapper.rs:4470), not recomputed from the surviving members, so the "unused cube isn't joined" guarantee holds only while the scan carries no hint naming it — which is what the view test asserts.

Review threads

The listing returned 1 thread of mine, PRRT_kwDOCOH19M6lJnaP (wrapper.rs:1629, the row-multiplicity/fan-out semantics finding). @MazterQyou answered it twice with a before/after table and named the two tests that pin the behaviour; the concern stands as a deliberate semantic change rather than a defect, so it stays open for the reviewer to close — not resolved, and not re-posted. 0 resolved, 0 re-affirmed inline.

· branch [`cubesql/prune-unused-members`](https://github.com/cube-js/cube/tree/cubesql/prune-unused-members)

[claude]

Claude finished @MazterQyou's task in 9m 29s —— View job

---

Review complete — pruning logic looks sound; 1 medium, 2 low posted inline

Full review

• Gather context (diff, PR body, prior threads)
• Read wrapper.rs changes in full file context
• Read test_wrapper.rs changes
• Verify build / run relevant tests — not done, see below
• Reconcile with prior review threads
• Post findings

Findings

#	Severity	Where	Concern
1	Medium	rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs:1626	join_hints is copied verbatim at wrapper.rs:4471, so a cube whose members were all pruned is still joined when the scan carries hints. The doc claim and the LIMIT consistency the new tests pin hold only for the single-entity view case.
2	Low	wrapper.rs:1630-1669	The subqueries and has_wildcard bail-outs and the join-condition leg of the visitor chain have no test; deleting any of them keeps the suite green while the failure mode is a hard query-time error.
3	Low	wrapper.rs:1685	is_sort_key returns true when expr_name fails, so it means "may be a sort key"; rename suggested (call site at 1693 needs it too).

What I verified and did not flag

• Schema/projection alignment. The fields.len() != projection_expr.len() + window_expr.len() guard plus the keep.iter().chain(repeat(&true)) tail depend on projection fields preceding window fields. That holds: converter.rs:2360-2370 builds the schema as without_window_fields (= projection_expr for a Projection select) chained with the window fields.
• No panic on the fallback index. keep[dimension_index.unwrap_or(0)] is unreachable for an empty keep, because keep.iter().all(..) is vacuously true and returns early at 1695.
• Sort-key naming is consistent across the prune. is_sort_key compares names computed against the pre-prune schema; the render-time lookup at wrapper.rs:4408 uses expr_name against the pruned schema, and expr_name is schema-independent for Column, Alias and Sort, so the two agree.
• Unpruned self.from uses are harmless. input_schema at wrapper.rs:2079 and inputs() at 4733 still see every column; a superset only affects type resolution and EXPLAIN output, and the pruning is confined to SQL generation.
• cube_name resolution. make_member_def derives it per member from used_members, falling back to ungrouped_scan_cubes, so a pruned cube is not named by a surviving member.
• Missed column references. The visitor chain covers every expr-bearing field of WrappedSelectNode (projection, flattened group, aggr, window, filter, having, order, join conditions), and Expr::InSubquery only occurs with a non-empty self.subqueries, which bails.
• Comment hygiene. Every explanatory comment in the diff is within 3 lines and carries rationale a later edit could otherwise undo (the DISTINCT guard, the name-only match, the dimension preference). Nothing to trim.

Not re-raised

The row-count change under an inner LIMIT or an outer aggregate — an unused one-to-many join no longer multiplying rows — was raised in an earlier round and rebutted concretely: the pruned behaviour is what every other cubesql path already does, the pre-PR behaviour made an inner LIMIT depend on whether the filter was pushable, and test_wrapper_limit_over_filter_reads_same_members_as_without_filter pins it. Withdrawn. Finding 1 is a different root cause (hints, not members) and a different fix direction.

Not run

cargo is not permitted in this environment, so the crate was not built and the nine new tests were not executed. Everything above is static analysis. CI's cubesql job is the check that matters here.

Review threads

The unresolved listing returned 4 of my own threads. All 4 resolved — the hidden per-column sort bail-out is now an up-front collect with an early return (wrapper.rs:1677), "ungrouped" is gone from the is_dimension_column doc (1727), and both CubeMetaMeasure::default() suggestions were applied (test_wrapper.rs:5124). 0 re-affirmed. The LIMIT-semantics thread carrying the rebuttal was already resolved, so there was nothing to reply to there.

· branch [`cubesql/prune-unused-members`](https://github.com/cube-js/cube/tree/cubesql/prune-unused-members)

[claude]

Claude finished @MazterQyou's task in 8m 17s —— View job

---

Sound change, 2 findings posted inline — 1 medium (COUNT(*) row-count regression), 1 low (join subplans unscanned).

Full review

• Read PR diff and changed files
• Analyze pruning logic in wrapper.rs
• Review test coverage
• Handle prior review threads
• Post findings (2 inline)

Findings

Sev	Where	What
Medium	wrapper.rs:4971	UsedColumnNamesVisitor yields no names for COUNT(*), so the inner select collapses to one dimension and a multi-cube view loses its one_to_many join — changing the count. CollectMembersVisitor::handle_count_rows (wrapper.rs:4928) takes the opposite position on the identical expression.
Low	wrapper.rs:1646	Join legs contribute only their condition; the join plan is never visited, so a correlated Expr::OuterColumn into from could be pruned. self.subqueries is bailed on for exactly this shape.

What holds up

• maybe_sort_key mirrors the push-to-Cube order resolution at wrapper.rs:4397-4428 precisely — same expr_name, same self.schema, same projection_expr list — so a sort key that find_column would look for is preserved, and aggr_expr/group_expr/window_expr (the other legs find_column searches) are untouched by pruning.
• The fields.len() != projection_expr.len() + window_expr.len() guard lines up with how converter.rs:2258-2370 builds the schema: without_window_fields first, window fields appended. So keep.iter().chain(repeat(&true)) keeps window fields correctly, and a Projection with empty projection_expr (where converter.rs:2268 substitutes from's fields into the schema but not the expr list) bails out rather than mis-aligning.
• Window and order exprs on the inner node are rebased to the underlying from columns by converter.rs:2337-2357, so pruning the projection can't dangle them.
• The visitor's expression set is exactly UserDefinedLogicalNode::expressions() for WrappedSelectNode (wrapper.rs:4744-4755) — nothing an outer select can reference is missed, modulo the join-plan gap above.
• No panic in the "keep nothing" fallback: keep.iter().all() on an empty vec returns true and returns early before keep[dimension_index.unwrap_or(0)].
• Pruning is confined to the non-push-to-Cube generate_sql path; generate_sql_for_push_to_cube returns before it.
• Comments are within the 3-line budget and each states a reason the code can't.

Test coverage

9 integration tests in test_wrapper.rs plus 4 unit tests in wrapper.rs's mod tests. The bail-outs (subqueries, has_wildcard, join conditions) are each pinned by a unit test that fails if its guard is removed. test_wrapper_limit_over_filter_reads_same_members_as_without_filter is the one that fails without the fix. The gap is the COUNT(*) case above: test_wrapper_count_over_filter_keeps_single_dimension covers it single-cube, where the fan-out difference can't show.

Not run: cargo test — no permission to invoke cargo in this environment, and no prebuilt rust/cubesql/target. Every finding above is static analysis.

Review threads

The listing returned 3 of my own threads, all unresolved, no human-opened thread among them. All 3 resolved, each with a reply stating why:

• PRRT_kwDOCOH19M6lMRTJ — join hints vs. pruned members: doc comment now scopes the claim, and test_wrapper_limit_over_filter_keeps_join_hints pins it.
• PRRT_kwDOCOH19M6lMRxh — is_sort_key misleading name: renamed to maybe_sort_key at binding and call site.
• PRRT_kwDOCOH19M6lMRma — no tests for the bail-outs: the four new unit tests cover them.

Re-affirmed: 0. One judgement call worth flagging: the medium finding overlaps the already-resolved ~wrapper.rs:1629 thread where you rejected an outer-aggregate guard. I posted it anyway because the argument is different — the handle_count_rows precedent in the same file, and your own new test showing COUNT(*) does reach this path — not a re-run of the semantics discussion. Close it again if the precedent doesn't move you.

· branch `cubesql/prune-unused-members`

[claude]

Claude finished @MazterQyou's task in 6m 55s —— View job

---

Review complete — 1 low finding; the pruning logic itself holds up

Full review

• Gather context (diff, PR body, prior threads)
• Read changed files in full
• Analyze correctness / edge cases
• Check test coverage
• Handle prior review threads
• Post findings

Finding (posted inline)

rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs:1705 — the "keep at least one column" fallback keep[dimension_index.unwrap_or(0)] = true can land on a projection column that maps to MemberField::Literal (__user, __cubeJoinField), producing a load request with dimensions: [] and measures: []. Needs a measure-preferring second pass before falling back to index 0. Details and a patch sketch are in the inline comment.

What I checked and found correct

Area	Verdict
fields.len() != projection_expr.len() + window_expr.len() guard	Correct — converter.rs:2359 builds the schema as without_window_fields (= projection_expr for Projection) then window fields, so the zip alignment holds and the guard catches any shape it doesn't
Projection + !distinct gate	WrappedSelectType::Aggregate and DISTINCT inners are the two where dropping columns changes rows; both bail out
Sort-key retention	generate_sql_for_push_to_cube resolves order by expr_name against projection_expr (wrapper.rs:4407-4418) and errors when it misses; maybe_sort_key keeps exactly those, and an unnameable sort expr bails out entirely
Window fields	window_expr_rebased inlines projection columns via replace_col_to_expr (converter.rs:2337), so window exprs don't read the projection output — pruning it is safe, and keep.chain(repeat(&true)) keeps every window field
COUNT(*) → reads_every_column	Mirrors CollectMembersVisitor::handle_count_rows, so the two paths agree on which members a row count needs
Join hints	Come from ungrouped_scan_node.request.join_hints, untouched by projection pruning — matches the doc comment and test_wrapper_limit_over_filter_keeps_join_hints
Panic safety of keep[...]	keep is empty only when projection_expr is, and the all-true early return fires first
Schema construction	A subset of an already-unique field list stays unique, so DFSchema::new_with_metadata can't newly fail
Blast radius	generate_sql returns early for push_to_cube, so pruning only ever runs for a non-push-to-cube outer select
Comments	All within 3 lines and load-bearing — each states a constraint (DISTINCT rows, sort-key lookup by name, uncorrelated join plans) that a later edit would otherwise re-break

Test coverage is thorough: the integration tests in test_wrapper.rs pin the multi-cube-view drop, the sort-key and aliased-sort-key retention, the window case, the DISTINCT-inner stop, join-hint retention, and the COUNT(*) pass-through; the unit tests in wrapper.rs cover wildcard, COUNT(*), subqueries, and join-condition columns directly.

Not run

No Rust build or test run — rust/cubesql/target is absent on this checkout and the datafusion git dependency isn't vendored, so a cargo check would be a cold full build. CI covers it; nothing below rests on runtime behaviour I couldn't derive from the source.

Review threads

The listing returned 2 of my own threads. Resolved both, re-affirmed none:

• wrapper.rs:4972 (COUNT(*) collapses the inner select) — fixed by the Expr::AggregateFunction { fun: Count, .. } arm in UsedColumnNamesVisitor::pre_visit.
• wrapper.rs:1647 (join plans never visited, possible correlated OuterColumn) — fixed by the note beside the subqueries bail-out at wrapper.rs:1630.

· branch [`cubesql/prune-unused-members`](https://github.com/cube-js/cube/tree/cubesql/prune-unused-members)

[claude]

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

---

Reviewed — 1 medium finding posted inline; the change is otherwise sound. Threads: 1 prior thread returned, 1 resolved, 0 re-affirmed.

Review details

• Read the diff and surrounding code
• Analyze correctness / edge cases
• Review tests
• Handle prior review threads
• Post findings

Finding

Medium — rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs:1700: the "inner query still needs at least one column" fallback tests keep, which spans only projection_expr, while window fields are retained unconditionally via keep.iter().chain(std::iter::repeat(&true)) at line 1723. When every projection column is unread but a window field survives, the fallback still forces a projection column back in. Against orders_logs_view_meta(), SELECT DISTINCT rn FROM (SELECT content, notes, ROW_NUMBER() OVER (ORDER BY customer_gender) rn FROM OrdersLogsView WHERE customer_gender ILIKE '%fe%ma%') t picks OrdersLogsView.content → Logs is joined back in, which is the join test_wrapper_distinct_over_filter_drops_unused_view_cube asserts away, and the one-to-many fan-out changes the ROW_NUMBER() values. Posted inline with a suggested shape (build keep at fields.len() with window positions pre-set to true).

Fix this →

Not posted inline

scan_member_for_column (wrapper.rs:1743) uses .ok()? on find_member_in_ungrouped_scan, conflating "not a member" with "lookup failed"; a genuine error degrades silently to unwrap_or(0). Low impact — the fallback picks a valid column either way.

What holds up

• Guards are conservative in the right direction: non-empty subqueries, non-Projection or distinct inner select, fields.len() mismatch against projection_expr + window_expr, Wildcard, and COUNT(*) all bail to the unpruned from. The COUNT(*) rule mirrors CollectMembersVisitor::handle_count_rows (wrapper.rs:4932), so the two stay in step.
• Schema ordering the zip relies on — projection fields then window fields — is what converter.rs:2357 builds for WrappedSelectType::Projection, and the length check bails if it ever isn't.
• Sort keys are kept by name matching expr_name, the same lookup generate_sql_for_push_to_cube uses at wrapper.rs:4412 to resolve order against the projection, so a pruned sort key can't produce the "Can't find column … in projection" error.
• Pruning happens only at SQL-generation time; the logical plan node keeps its original from, so inputs()/schema stay consistent.
• Inner filter_expr, window_expr and order_expr reference the CubeScan rather than the projection output, so they survive pruning — the window test confirms the ROW_NUMBER() measure keeps its ${…notes} reference while notes leaves the projection.
• Comment budget is respected: every inline comment in the new code is ≤3 lines and each states a reason the code can't.
• Test coverage is good — e2e tests for pruning, the DISTINCT stop, the sort-key and aliased-sort-key keeps, window retention, join-hint retention, the view/cross-cube case, and the COUNT(*) bail, plus unit tests for the subquery/wildcard/join-condition guards and the fallback's dimension preference.

Not verified

cargo is not permitted in this sandbox, so I did not build or run the cubesql test suite. The finding above is from reading the code, not from a failing run.

Threads

Listing returned 1 thread of mine (PRRT_kwDOCOH19M6lRnww, wrapper.rs, outdated). Resolved it — its concern, unwrap_or(0) landing on a MemberField::Literal like __user and emitting a request with neither dimensions nor measures, is fixed by the .or_else() member fallback now at wrapper.rs:1706-1712. 0 re-affirmed.

· branch [`cubesql/prune-unused-members`](https://github.com/cube-js/cube/tree/cubesql/prune-unused-members)

[claude]

Claude finished @MazterQyou's task in 6m 54s —— View job

---

Review — 1 low, no blockers. Pruning logic holds up; one alias gap in the fallback heuristic.

Full review

• Read the diff and both changed files in full
• Verified the pruning guards against the converter and the rewrite rules
• Reviewed test coverage
• Checked prior review threads
• Posted findings

Finding

Low — scan_member_for_column ignores Expr::Alias (rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs:1748, posted inline). When the outer select reads nothing and the inner projection is aliased (SELECT sumPrice AS s, customer_gender AS g), every probe returns None and the fallback drops to unwrap_or(0), which can keep a measure and pull the join back in — the exact fan-out the PR removes. test_wrapper_constant_over_filter_keeps_single_dimension passes only because its subquery is SELECT *. One-line fix suggested inline.

What I checked and found sound

• Field ordering. keep assumes schema is projection-then-window. Confirmed at converter.rs:2361-2369: without_window_fields then the window fields.
• The asterisk shape. A Projection select with an empty projection_expr would make the visitor see no columns while its schema still spans every inner column. Not reachable: converter.rs:2235-2246 expands an empty projection into explicit columns of from + joins before the node exists, so the visitor sees all of them. The fields.len() != projection_expr.len() + window_expr.len() guard covers the Aggregate variant of the same shape on the inner side.
• Window columns. The inner's window_expr resolves against the CubeScan, not against its own projection, so pruning a projection column can't orphan a window. test_wrapper_distinct_over_filter_keeps_only_read_window pins that (dimensions == [], only the window measure).
• Scope. Pruning is applied only in generate_sql, after the push_to_cube early return; generate_sql_for_push_to_cube requires from to be a CubeScanNode (wrapper.rs:4041-4058), so it can never see a pruned WrappedSelectNode. It does cascade through nested selects via generate_sql_for_node_rec, which is what test_wrapper_count_over_distinct_keeps_distinct_columns exercises.
• input_schema at wrapper.rs:2088 still comes from the unpruned self.from. Only used for integer-division type detection, and a superset resolves strictly more — no mismatch.
• Name-only matching (is_used) is conservative in the safe direction: a same-named column under a different qualifier is kept, never dropped.
• COUNT(*) bails out via reads_every_column, mirroring CollectMembersVisitor at wrapper.rs:4959-4963, so the all-members behaviour is identical whether or not the filter pushes down — test_wrapper_count_over_filter_reads_same_members_as_without_filter asserts both sides.
• Comments are within the 3-line limit and each carries a reason the code can't state itself.

Test coverage is the strongest part of this PR: the fan-out consistency claim from the PR discussion is pinned by a before/after pair (test_wrapper_limit_over_filter_reads_same_members_as_without_filter), join hints are covered separately, and the unit tests in wrapper.rs exercise the guards directly.

Not re-raised

The limit/offset and outer-aggregate row-count question was raised and answered by @MazterQyou in the PR discussion with a concrete before/after table; the behaviour is the one cubesql already has on every other path.

Not run

Rust build and tests were not run — no cargo build cache in this checkout, and a cold cargo check on cubesql does not fit the tool timeout. The findings above are from reading the code, not from a compiled run.

Review threads

The listing returned 1 thread of mine. Resolved 1 (PRRT_kwDOCOH19M6lSNTX, wrapper.rs:1703 — the keep/chain(repeat(&true)) span concern, now addressed by seeding keep over fields). Re-affirmed 0. The new finding is not a duplicate of it.

· branch `cubesql/prune-unused-members`

[claude]

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

---

Review: ready to merge after two small fixes (0 high, 0 medium, 2 low). The aliased-literal sort key is the one worth fixing before merge.

Details

• Gather context
• Review wrapper.rs changes
• Review tests
• Handle prior review threads
• Post review

Findings

Severity	Where	Issue
Low	rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs:1686	maybe_sort_key compares a column's alias name, but the inner sort key can be the bare literal (Int64(5)), so an aliased literal sort key gets pruned. The non-push-to-Cube inner select then renders ORDER BY 5 as a column position. I derived this from the code; I couldn't reproduce it because cargo wasn't permitted in this job.
Low	rust/cubesql/cubesql/src/compile/test/test_wrapper.rs:5199	A comment is attached to the end of an assert_eq! line, apparently left over from an edit.

Other things I checked

• Row-count semantics after pruning: I agree with the author's reply. Pruning matches what the query returns when the filter is pushed to Cube, and the regression tests pin it.
• Unguarded row-count expressions: SUM(1) and COUNT(*) OVER () aren't treated as reading every column, unlike COUNT(<literal>). CollectMembersVisitor has the same gap in the push-to-Cube path, so behaviour is consistent. I didn't post this inline.
• When pruning is skipped: it's skipped for subqueries, wildcards, DISTINCT, non-projection inner selects, and projection/schema length mismatches. That covers the cases where dropping a column could change results or break SQL.
• Join conditions: they are part of the used-column set, which is correct because join plans are uncorrelated.
• Tests: coverage is good. The end-to-end tests cover pruning, sort keys (plain and aliased), windows, join hints, COUNT(*), and the no-read fallback, and there are unit tests for the helper. There's no test for an aliased literal sort key, which is the first finding.
• Comments: they're all within 3 lines and explain why, not what.
• Planner: this change is in the cubesql wrapper only, so the Tesseract/legacy split doesn't apply.

Review threads

The listing returned 1 prior thread of mine. I resolved it (it was about scan_member_for_column not handling aliased columns, and the code now strips the alias first). I re-affirmed none. Both findings above are new threads.

[claude]

An aliased literal sort key gets pruned. expr_name on a projection column Literal(5) AS five returns five. When sort pushdown has already replaced the alias with the literal (the case handled by literal_aliases at wrapper.rs:2192), the inner order_expr is Sort(Int64(5)), so its name is Int64(5). The names don't match, so five is dropped when the outer select doesn't read it. In a non-push-to-Cube inner select, literal_aliases then finds no projected literal to restore, and the inner query renders ORDER BY 5 as a position. It either sorts by whatever column now sits at position 5, or fails with "ORDER BY position 5 is not in select list" once the projection is narrowed below 5 columns. Push-to-Cube isn't affected, because its find_column lookup at wrapper.rs:4420 already fails on this plan before and after this PR.

I couldn't run cargo in this job, so I derived this from the code rather than reproducing it. Matching the unaliased projection expression as well closes the gap:

let maybe_sort_key = |expr: &Expr| {
    let names = [expr, unalias(expr)].map(|e| expr_name(e, &from.schema));
    names.iter().any(|n| n.as_ref().map_or(true, |n| sort_key_names.contains(n)))
};

(unalias is currently local to generate_column_exprs, so it would need hoisting.)

[claude]

A comment got attached to the end of this assert_eq! line, which looks like a leftover from an edit. It pushes the line well past rustfmt's width, and it describes the assert on the next line, not this one.

Suggested change

	assert_eq!(dimensions, vec![json!("${OrdersLogsView.customer_gender}")]); // No hints either: `Logs` would be joined by a hint naming it even without members
	assert_eq!(dimensions, vec![json!("${OrdersLogsView.customer_gender}")]);
	// No hints either: `Logs` would be joined by a hint naming it even without members

[claude]

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

---

Reviewing PR

• Gather context / read diff
• Read changed code in context (wrapper.rs call site, push-to-Cube path)
• Review tests
• Handle prior review threads (running)
• Post review