feat: support parameterized ($n) LIMIT/OFFSET in prepared statements

[vismaytiwari]

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

Draft / WIP — opening this to align on the approach for #23345 before polishing it. Stacked on #26109 (see below).

What this does

#23345: Prisma-style SELECT ... LIMIT $2 OFFSET $3 fails on RisingWave. #26109 fixes the parser (OFFSET now parses as an expression, like LIMIT), but $n still errors at bind time with non-const expression — because LIMIT/OFFSET are folded to a constant u64 during binding, which runs before bind parameters are substituted (ParamRewriter / BoundStatement::bind_parameter runs later, at the Bind step).

This PR defers that fold for parameter-dependent LIMIT/OFFSET:

• BoundQuery.limit/offset: Option<u64> → Option<ExprImpl> (carry the bound expression instead of an eagerly-folded constant).
• Binding still validates an already-constant value eagerly (negative value / evaluation error rejected at bind), so those errors precede side effects such as CREATE TABLE AS. A non-constant is rejected at bind unless it contains a $n parameter, in which case it is carried through.
• BoundQuery::rewrite_exprs_recursive now rewrites limit/offset, so ParamRewriter substitutes $n inside them.
• A new eval_limit_or_offset in the planner folds the (now-substituted) expression to u64, preserving the existing semantics: NULL → no-limit (LIMIT) / zero (OFFSET), negative rejection, non-const error.

Plan nodes are unchanged (LogicalLimit/LogicalTopN still take u64); constant limits/offsets produce identical plans.

Relationship to #26109

This is stacked on #26109 (parser + constant-expression parity). Until #26109 merges, the diff here includes its commits too; I'll rebase to just the delta afterward. Please review #26109 first.

Open questions / why this is a draft

• Design check for @lmatz: feat: support expression in limit clause #19834 deliberately folds LIMIT during binding. This PR reverses that for the parameter case (defers to plan time, after substitution). Does this direction look right to you before I polish it?
• Parameters nested inside a subquery in LIMIT (e.g. LIMIT (SELECT $1)) are not detected and are rejected at bind — acceptable?
• Simple-protocol parameterized CREATE TABLE AS remains a theoretical edge (table created before the insert is planned).
• Tests: prepared-statement e2e for LIMIT $1 / OFFSET $2 still to add.

Part of #23345.

Checklist

• Tests (TODO)

[buu-nguyen]

I encountered the very same issue. Looking forward to this PR being merged.

[lmatz]

Request changes — the direction is correct, but this needs protocol-level test coverage and a clarified scope before merge.

What's right: $n parameters are only substituted at Bind, so folding LIMIT/OFFSET to a constant during binding rejected OFFSET $n. Carrying limit/offset as an expression through param-rewrite and folding in the planner fixes exactly that path.

Scope: this supports a constant / parameter-foldable subset, not full PostgreSQL LIMIT/OFFSET expression semantics. PostgreSQL treats the value as an int8 expression evaluated once at executor startup/rescan (nodeLimit.c:recompute_limits), allowing volatile functions and non-correlated scalar subqueries and rejecting only column refs / aggregates / window functions / SRFs (parse_clause.c:transformLimitClause). Folding to a plan-time constant is a reasonable and largely necessary divergence, but it should be stated explicitly rather than described as PG-compatible.

Required before merge:

1. Extended-protocol tests: prepare OFFSET $1 / LIMIT $2, Bind+Execute, and re-Bind the same statement with different values across executions to confirm per-Bind evaluation.
2. Boundary semantics: NULL → 0 (LIMIT NULL → no limit); int4→int8 coercion; int8 overflow. A negative value must be rejected only after binding (Bind/Execute), never at Parse.
3. Error/docs: on a value that doesn't reduce to a constant, fail with e.g. LIMIT/OFFSET must reduce to a constant after parameter binding (so $1 + 1, casts stay valid); don't claim general PostgreSQL expression compatibility.

Recommended (here or as a tracked follow-up): accept a parameter/constant in OFFSET <expr> ROWS FETCH NEXT <expr> ROWS ONLY; the parser currently accepts only a bare number in that spelling.

Should succeed:

OFFSET 10
OFFSET $1                              -- re-bind different values across executions
OFFSET $1 + 1                          -- folds to a constant after binding
LIMIT $1 OFFSET $2
OFFSET NULL                            -- => 0

Should error (assert the message):

OFFSET $1  (bound to -1)               -- runtime error at Bind/Execute, not Parse
OFFSET input_column                    -- column ref (also rejected by PG)
OFFSET count(*)                        -- aggregate (also rejected by PG)
OFFSET row_number() OVER ()            -- window fn (also rejected by PG)
OFFSET generate_series(1,2)            -- SRF (also rejected by PG)
OFFSET floor(random()*3)               -- volatile: PG allows; RW divergence, documented
OFFSET (SELECT 2)                      -- subquery: PG allows; RW divergence, documented

[vismaytiwari]

Thanks for the detailed review @lmatz — all addressed in 0ba98d1, and I built + ran it locally (sqlparser unit tests, the e2e_extended_mode binary, and the batch slt all pass).

• Scope framing: agreed — dropped the "PG-compatible" wording. LIMIT/OFFSET are folded to a plan-time constant after parameter substitution, which diverges from PG (which evaluates the int8 expression once at executor start, allowing volatile functions and non-correlated subqueries). The slt comments now state this explicitly.
• 1. Extended-protocol tests: added limit_offset_with_param to the extended-mode suite. It prepares OFFSET $1 and re-binds the same statement with different values across executions (asserting the row counts change per-Bind), plus LIMIT $1 OFFSET $2 and FETCH FIRST $1 ROWS ONLY.
• 2. Boundary semantics: LIMIT NULL → no limit and OFFSET NULL → 0 (covered in both the protocol test and the slt); int4→int8 via cast_assign. A negative value bound to $1 is rejected at Bind/Execute — the fold happens in the planner, after substitution — never at Parse; the protocol test asserts this.
• 3. Error/docs: a value that doesn't reduce to a constant after binding now errors with <clause> must reduce to a constant after parameter binding, but found a non-constant expression, so $1 + 1 and casts stay valid. No PG-compat claim.
• FETCH (your recommended item): included here — the parser now accepts an expression/parameter as the FETCH FIRST <expr> ROWS quantity, mirroring OFFSET (Fetch.quantity is now Expr).

The reject matrix (column reference, aggregate, window function, SRF, volatile, subquery) is covered in the batch slt. Let me know if you'd like the volatile/subquery divergence called out anywhere beyond the slt comments.

[lmatz]

can you put the detailed error message in the test, there is some example above in this file, thanks

[vismaytiwari]

Done — added the detailed error messages to all the rejected cases (5e534b4). Used the inline statement error <message> form since the interspersed query blocks were desyncing the ---- result-block parsing, but the assertions are the real messages: e.g. OFFSET must not be negative, but found: -1, Invalid column: v for a column ref, and ... after OFFSET, but found non-const expression for the aggregate/window/volatile/SRF forms. Verified locally against a running cluster.

[lmatz]

Review: Request changes

1. Crash: LIMIT NULL with a positive OFFSET

LIMIT NULL folds to u64::MAX (the null_value for LIMIT in planner/query.rs); a downstream offset + limit then overflows. LIMIT_ALL_COUNT = u64::MAX / 2 exists to keep offset + limit from overflowing, but the NULL path uses u64::MAX and bypasses it. LIMIT NULL alone is fine; a positive OFFSET is required. Two paths, different blast radius:

• Without ORDER BY — panic in BatchLimit::two_phase_limit; the current pgwire connection is closed, the frontend survives:

SELECT * FROM generate_series(1,3) LIMIT NULL OFFSET 1;
-- attempt to add with overflow

• With ORDER BY (local TopN, e.g. over generate_series) — panic in TopNHeap::new on the rw-batch-local thread; the frontend process exits (134), port 4566 goes down, and all connections drop:

SELECT * FROM generate_series(1,3) ORDER BY 1 LIMIT NULL OFFSET 1;

• With ORDER BY on a real table under query_mode=distributed — the overflow panics earlier, in the frontend planner BatchTopN::to_distributed (batch_topn.rs:51); the current connection is closed but the frontend and compute nodes survive.

The ordered path therefore has at least two unprotected limit + offset additions (planner and executor) — the same root-cause overflow, not a separate bug. OFFSET's null_value is 0, so OFFSET is unaffected. The u64::MAX sentinel is pre-existing, but this PR's parameterized path means LIMIT $1 bound to NULL now reaches this from the extended protocol / client drivers.

2. FETCH errors report the wrong clause; an inaccurate comment

• FETCH quantity is validated through the LIMIT path, so a bad FETCH value reports LIMIT must not be negative / ...after LIMIT even though the user wrote FETCH (e.g. ... FETCH FIRST -1 ROWS ONLY). The clause name in the message is wrong for FETCH.
• The expr_has_parameter comment describes walking the "expression tree", but the default ExprVisitor does not descend into subqueries, so the comment overstates what it checks.

3. Tests to add

Assert exact row counts (not just "does not panic"), so a release build that wraps instead of panicking is also caught. Cover ordered + unordered and simple + extended protocol:

• LIMIT NULL OFFSET 1 and ... ORDER BY 1 LIMIT NULL OFFSET 1 — 3-row input must return 2 rows (no limit, but OFFSET 1 still applies; the ordered variant returns exactly 2, 3), not panic.
• The same via extended protocol: LIMIT $1 OFFSET 1 with $1 bound to NULL, ordered and unordered.
• int8 overflow: OFFSET $1 + 1 bound to i64::MAX — must be a clean error.
• OFFSET $1 + random() — the "still non-constant after $n substitution" planner branch.

[vismaytiwari]

Thanks @lmatz — great catches. Addressed in e13303f + 5bc8512, all verified locally on a running cluster.

1. LIMIT NULL + OFFSET crash. A NULL LIMIT folded to u64::MAX; folding it to LIMIT_ALL_COUNT (u64::MAX / 2) instead keeps offset + limit in range — it's the same value the no-limit path already uses via unwrap_or(LIMIT_ALL_COUNT). Verified against your examples over generate_series, ordered and unordered, simple and extended protocol: LIMIT NULL OFFSET 1 (and LIMIT $1 OFFSET 1 with $1 bound to NULL) now returns the rows past the offset instead of panicking.

2. FETCH clause + comment. The clause name is now carried from the binder through to the planner, so a bad FETCH quantity reports FETCH FIRST — FETCH FIRST -1 ROWS ONLY now errors with FETCH FIRST must not be negative, but found: -1 (asserted in the slt). Fixed the expr_has_parameter comment to note the default ExprVisitor does not descend into subqueries.

3. Tests. Added exact row-count assertions (ordered + unordered, simple + extended) for LIMIT NULL OFFSET 1; an int8-overflow case (OFFSET $1 + 1 bound to i64::MAX → clean Numeric out of range); and the OFFSET $1 + random() planner branch (non-constant after substitution → the new error message).

One thing to flag separately, since it turned out not to be specific to this PR: while testing over a real table I hit a pre-existing panic — SELECT * FROM t OFFSET 1 (no LIMIT) under query_mode = local over a table scan panics at assert!(batch_size > 0) in chunk_coalesce.rs. It comes from the same LogicalLimit(LIMIT_ALL_COUNT, offset) plan that any unbounded-LIMIT + OFFSET query produces, and reproduces on main without this PR (the plan-building at plan_query and the batch executor are untouched here). So I kept it out of scope and used generate_series for the LIMIT NULL tests to exercise this fix cleanly. Happy to open a separate issue for it if you'd like.