feat(cubesql): add disablePostProcessing flag to /v1/cubesql

[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 adds an opt-in disablePostProcessing flag to /v1/cubesql that rejects queries whose post-processing would run over a result silently truncated to CUBEJS_DB_QUERY_LIMIT rows.

A query that can't be fully pushed down is completed in memory over at most CUBEJS_DB_QUERY_LIMIT rows. When nothing bounds the pushed-down subtree, that result is truncated and the post-processing above it returns a wrong answer with no error.

Setting disablePostProcessing on the request rejects those queries. Plans whose post-processing reads a result bounded by a smaller limit are left alone, as are plans with no cube scan at all: neither can be truncated. Inert under CUBESQL_STREAM_MODE, where the scan streams every row.

Extraction counts the offending plan shape in a new truncated_post_processing_scans cost, so a fully pushed-down plan wins where one exists; the request only fails when none does.

Related tests are included.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
cubed3	🟢 Ready	View Preview	Aug 3, 2026, 6:33 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[claude]

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

---

Review: solid design, one correctness gap in what counts as post-processing — 1 medium-high, 3 medium, 3 low

Full review

• Read the diff
• Review Rust changes (cost.rs, rewriter.rs, session_vars.rs)
• Review JS/TS changes (gateway, sql-server, native bindings)
• Review tests & docs
• Post review

What's good

• The framing is right: counting the offending shape as a cost tier rather than checking the extracted plan means a fully pushed-down alternative still wins where one exists, and the request only fails when none does. The doc comment explaining why the tier sits below table_scans / non_detected_cube_scans — and the test that pins it — is the kind of thing that saves the next person a day.
• truncated_post_processing is set only at the root of a pushed-down subtree and guarded on the parent's post_processing_above, so it counts boundaries rather than every node above one. Verified against the extractor: finalize receives transform(node), and children re-derive their own flag, so there's no double counting up the tree.
• Session variable is set on a per-request session (with_session creates and drops one per execSql), so the flag can't leak across requests. The rewrite cache stores egraphs, not extracted plans, so a cached egraph won't bypass the check either.
• Inert under CUBESQL_STREAM_MODE, and max_intermediate_rows: None keeps post_processing_above pinned to false on the default path so the extractor's cache key doesn't fragment. Both correct and both explained in comments.
• Field indices check out: CubeScan[4] = limit, WrappedSelect[10] = limit, CubeScanWrapper[0] = input.
• Tests on both sides plus docs, and the doc anchors (#cubejs_db_query_limit, #cubesql_stream_mode) both resolve.

Findings

#	Severity	Where	Issue
1	Medium-high	cost.rs:930-935	Distinct / TableUDFs aren't counted as post-processing → Distinct -> CubeScanWrapper silently passes the check and returns a wrong answer
2	Medium	cost.rs:853-868	CubeScanWrapper over a bare wrapped CubeScan always reads as unbounded → false rejection of a legitimately limited query
3	Medium	gateway.ts:483	disablePostProcessing unvalidated; a non-boolean silently disables the guard and returns a truncated result with 200
4	Medium	naming	Same flag name, two different behaviors across two API surfaces — see below
5	Low	sql.test.ts:722	The only native test asserting rejection early-returns under CUBESQL_STREAM_MODE, which CI sets → reports green having asserted nothing
6	Low	rewriter.rs:365	non_streaming_query_max_row_limit() as i32 -> usize; a negative value wraps to a huge number and quietly disables the check
7	Low	rewriter.rs:359	The TODO lost and penalized_ast_size_outside_wrapper, but nothing in this PR added that check — penalize_post_processing still can't break with a sane error

Details for 1, 2, 3 and 5 are in inline comments.

On #4 (naming). SQLServer.sql4sql(sqlQuery, disablePostProcessing, ...) already takes a parameter with this exact name, and it sets CUBESQL_PENALIZE_POST_PROCESSING_VAR — a soft cost penalty that never errors and applies to all post-processing. The new execSql(..., disablePostProcessing) sets a different variable with a hard-error, truncation-scoped meaning. Two adjacent methods on the same class, same parameter name, materially different semantics. The session_vars.rs doc comments distinguish them well; the TS signatures don't. Worth either renaming one (rejectTruncatedPostProcessing?) or adding a TSDoc line on both SQLServer methods pointing at the difference.

Design note (not a defect)

The check treats any in-memory node above an unbounded scan as unsafe, including row-wise ones. SELECT date_part('doy', order_date) FROM cube with no limit gets rejected, though a bare projection over the first 50k rows returns exactly what the same query returns today without the flag — truncated, not wrong. That's a deliberate conservative call and it's the right default for an opt-in guard; just noting the flag is "reject anything that reads a truncated result" rather than "reject anything that would be wrong", and the docs' framing ("the result can be wrong without any error") slightly oversells the precision.

Also worth a mention in the docs: CUBESQL_FAIL_ON_MAX_LIMIT_HIT (converter.rs:2004) already errors when a scan hits the limit. It's global and unconditional where this is per-request and shape-scoped, so they're complementary — but a reader hitting this docs section will reasonably wonder which one they want.

Not verified

I did not compile the Rust or run either test suite in this environment — the review is by inspection. Worth confirming cargo check -p cubesql and yarn test in cubejs-api-gateway before merge.

• branch `cubesql/post-processing-sort-fix`

[codecov]

Codecov Report

❌ Patch coverage is 97.30769% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.50%. Comparing base (3728d62) to head (f63321d).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...l/cubesql/src/compile/test/test_post_processing.rs	94.04%	5 Missing ⚠️
packages/cubejs-api-gateway/src/sql-server.ts	0.00%	1 Missing ⚠️
rust/cubesql/cubesql/src/compile/rewrite/cost.rs	99.27%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11462      +/-   ##
==========================================
+ Coverage   79.45%   79.50%   +0.05%     
==========================================
  Files         480      481       +1     
  Lines       98810    99059     +249     
  Branches     3636     3636              
==========================================
+ Hits        78511    78759     +248     
- Misses      19778    19779       +1     
  Partials      521      521              

Flag	Coverage Δ
cube-backend	59.10% <50.00%> (-0.01%)	⬇️
cubesql	84.00% <97.67%> (+0.04%)	⬆️

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 4m 22s —— View job

---

Re-review after force-push: 3 of 4 prior findings fixed well — 1 medium open (unvalidated flag at the gateway), 2 low

Full review

• Diff current HEAD against my prior review's findings
• Re-review Rust changes (cost.rs, rewriter.rs, session_vars.rs)
• Re-review JS/TS changes (gateway, native bindings, tests)
• Resolve stale threads / post new findings

The branch was force-pushed (previous head 27cbc37 is gone), so I re-read the whole diff rather than an incremental one.

Prior findings — now fixed

#1 Distinct / TableUDFs slipping through (was medium-high). Fixed properly, and better than the minimal patch: is_post_processing (cost.rs:906-935) is now a dedicated predicate rather than a reuse of the ast_size_outside_wrapper node list, with a doc comment stating exactly why the two lists must not be shared. I cross-checked it against every LogicalPlan:: variant the converter handles — Aggregate, Analyze, CreateExternalTable, CrossJoin, Distinct, EmptyRelation, Explain, Extension, Filter, Join, Limit, Projection, Repartition, Sort, Subquery, TableScan, TableUDFs, Union, Values, Window. The predicate covers all 13 that can post-process; the omissions are leaves (EmptyRelation, TableScan, Values), the pushdown nodes themselves (Extension), and Explain/Analyze/CreateExternalTable, none of which return post-processed data. test_disable_post_processing_counts_distinct_as_post_processing pins the exact shape and — good instinct — asserts on the plan being a full pushdown rather than on Ok, since bare Ok is what the bug produced.

#2 wrapper over a bare CubeScan (was medium). Fixed by the recursion, with a comment explaining why both shapes are real, plus four is_bounded unit tests including the conservative-default case. I re-verified the field indices against rewrite/mod.rs: CubeScan has 11 children with limit at 4 and wrapped at 8; WrappedSelect has 17 with limit at 10; CubeScanWrapper has 2 with input at 0 — so the test helpers' [limit_id; 11] / [_; 17] / [_; 2] arities are right. Also confirmed the is_bounded cases line up with what actually clamps at execution: set_max_limit_for_node (wrapper.rs:906-949) handles exactly CubeScanNode and WrappedSelectNode at the top of the wrapped plan and falls through otherwise, matching the _ => false default.

#5 native test no-op under CUBESQL_STREAM_MODE (was low). Fixed with describeUnlessStreamMode, and the comment goes further than I asked — it documents why clearing the env var from jest doesn't work (native config reads the real process environment, not jest's sandboxed process.env) and states plainly what coverage is lost and where it's made up. That's the right way to record a test-harness limitation.

Still open

#	Severity	Where	Issue
1	Medium	gateway.ts:483	disablePostProcessing still forwarded unvalidated; a non-boolean fails the Neon downcast at node_export.rs:617 and silently returns false → truncated result with a 200. Existing thread left unresolved.
2	Low	rewriter.rs:365	i32 -> usize: a negative CUBEJS_DB_QUERY_LIMIT becomes usize::MAX and silently disables the guard; 0 rejects everything. New inline comment.
3	Low	naming	SQLServer.execSql(..., disablePostProcessing) and SQLServer.sql4sql(sqlQuery, disablePostProcessing, ...) are adjacent methods taking the same parameter name for materially different behavior (hard error, truncation-scoped vs. soft cost penalty over all post-processing). The new session_vars.rs doc comments distinguish the two variables well — that distinction just doesn't reach the TS signatures. A TSDoc line on both methods would close it.

Also re-verified since the last pass, all still fine: the cost tier's position in CubePlanCost's field order (after joins, before wrapper_nodes, below table_scans / non_detected_cube_scans — matching the doc comment and the test that pins it); truncated_post_processing set only at a pushdown-subtree root and guarded on the parent's post_processing_above; max_intermediate_rows: None keeping post_processing_above pinned to false so the extractor cache key doesn't fragment on the default path; a single TopDownExtractor site, and the compiler caches holding CubeEGraph rather than extracted plans, so no cross-request leak of the flag.

Design notes (not defects)

Limit counts as post-processing, so Limit(n) -> CubeScanWrapper(unbounded) is rejected even though a top-level LIMIT with no sort below the truncation would return the right rows. In practice the limit is pushed down and this shape is rare, and the conservative direction is the right default for an opt-in guard — noting it only because it's the one remaining case where "rejected" and "would be wrong" come apart, alongside the row-wise-projection case from my last pass.

The docs section is clear and the anchors resolve. Still worth a sentence on CUBESQL_FAIL_ON_MAX_LIMIT_HIT (converter.rs:2004), which errors when a scan hits the limit — global and unconditional where this is per-request and shape-scoped. A reader landing on this section will reasonably wonder which of the two they want.

Not verified

cargo check isn't in my allowed tools here (Bash(cd … && cargo check …) was declined), so the Rust review — including the new unit tests' node arities — is by inspection against rewrite/mod.rs rather than by compiling. Worth confirming cargo test -p cubesql and yarn test in cubejs-api-gateway before merge.

• branch `cubesql/post-processing-sort-fix`

[claude]

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

---

Re-review after force-push: the negative-limit fix landed — no open correctness findings; 1 low (naming) remains

Full review

• Diff current HEAD against prior review findings
• Re-review Rust changes (cost.rs, rewriter.rs, session_vars.rs)
• Re-review JS/TS changes (gateway, native bindings, tests)
• Resolve stale threads / post findings

What changed since my last pass

rewriter.rs negative CUBEJS_DB_QUERY_LIMIT (was low, now fixed). non_streaming_query_max_row_limit().max(0) as usize closes the -1 → usize::MAX fail-open path, and the comment states the reasoning for saturating rather than erroring — a limit of zero bounds nothing, so every plan that post-processes such a scan is rejected, which is the same answer an explicit LIMIT 0 config already gets. That's the right direction for a guard: the degenerate config now fails closed. Thread resolved.

Nothing else moved in this push, and I re-verified the rest of the change rather than assuming:

• Cost tier position. truncated_post_processing_scans sits after joins and before wrapper_nodes in CubePlanCost's field order (cost.rs:287-291), i.e. below table_scans / non_detected_cube_scans / penalized_ast_size_outside_wrapper and above wrapper_nodes — exactly what the doc comment at cost.rs:270-273 claims, and what test_disable_post_processing_rejects_truncated_intermediate_result's doc comment pins.
• is_post_processing (cost.rs:899-915). Still covers all 13 LogicalPlan variants that can post-process. Worth noting for the record why the omissions are safe beyond "they're leaves": the flag ORs with the parent's value, so a non-listed pass-through node between a post-processing node and the scan doesn't reset anything. An omission only matters when the omitted node is the sole in-memory node above the scan — which is why Distinct was the real bug and SubqueryAlias-style renames aren't.
• Wrapper subtree containment. wrapped == Wrapper at the CubeScanWrapper node pins post_processing_above to false for its whole subtree, so WrappedSelect internals can't re-trigger the check from inside a pushdown.
• Test-only egraph arities ([limit_id; 11] / [_; 17] / [_; 2] / EmptyRelation([scan; 3])) all match rewrite/mod.rs; the conservative-default test builds a genuinely mixed eclass via union + rebuild, so it exercises the _ => false arm rather than just asserting it.
• Session variable stays per-request (with_session per execSql), caches hold CubeEGraph not extracted plans, max_intermediate_rows: None keeps the extractor cache key unfragmented on the default path, describeUnlessStreamMode makes the CI skip visible, docs anchors resolve.

Still open

#	Severity	Where	Issue
1	Low	naming	SQLServer.execSql(..., disablePostProcessing) and SQLServer.sql4sql(sqlQuery, disablePostProcessing, ...) are adjacent methods taking the same parameter name for materially different behavior: hard error, truncation-scoped vs. soft cost penalty over all post-processing. The session_vars.rs doc comments draw that distinction well; the TS signatures don't. One TSDoc line on each method would close it.

The gateway-validation thread is resolved — following forceContinueWait for consistency is a reasonable call, and it's your codebase's convention to make.

Design notes carried over (not defects)

Unchanged from my prior passes, restated only so they're not lost: the guard rejects "post-processing reads a truncated result", not "post-processing would be wrong". A bare row-wise projection (SELECT date_part(...) FROM cube, no limit) and a top-level Limit(n) over an unbounded wrapper both get rejected though they'd return the same rows the query returns today. Conservative is the right default for an opt-in flag; the docs' "the result can be wrong without any error" just reads slightly more precise than the check is.

Still worth one sentence in the docs on CUBESQL_FAIL_ON_MAX_LIMIT_HIT (converter.rs:2004) — global and unconditional where this is per-request and shape-scoped. A reader landing on this section will reasonably wonder which of the two they want.

Not verified

cargo check -p cubesql --tests isn't in my allowed tools here (declined again this run), so the Rust review — including the new unit tests' node arities — is by inspection against rewrite/mod.rs. Worth confirming cargo test -p cubesql and yarn test in cubejs-api-gateway before merge.

• branch `cubesql/post-processing-sort-fix`