Skip to content

sql: Constant-fold parameterized top-level LIMIT - #37853

Merged
def- merged 2 commits into
MaterializeInc:mainfrom
def-:pr-sql-476
Aug 4, 2026
Merged

sql: Constant-fold parameterized top-level LIMIT#37853
def- merged 2 commits into
MaterializeInc:mainfrom
def-:pr-sql-476

Conversation

@def-

@def- def- commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Executing a prepared statement with a parameterized LIMIT failed with "Top-level LIMIT must be a constant expression, got integer_to_bigint(3)" whenever the bound parameter type was not bigint, because parameter binding wraps the value in a cast that the bare-literal check did not fold. Evaluate the expression to a literal instead, matching how OFFSET is handled. A negative LIMIT now reports "LIMIT must not be negative" instead of soft-panicking, and a non-constant LIMIT reports "Invalid LIMIT clause: ...".

Closes: SQL-476

@def-
def- requested a review from ggevay July 24, 2026 08:29
@def-
def- requested review from a team as code owners July 24, 2026 08:29
@def-

def- commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Ready for review

Executing a prepared statement with a parameterized LIMIT failed with
"Top-level LIMIT must be a constant expression, got integer_to_bigint(3)"
whenever the bound parameter type was not bigint, because parameter
binding wraps the value in a cast that the bare-literal check did not
fold. Evaluate the expression to a literal instead, matching how OFFSET
is handled. A negative LIMIT now reports "LIMIT must not be negative"
instead of soft-panicking, and a non-constant LIMIT reports
"Invalid LIMIT clause: ...".

Closes: SQL-476

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@ggevay ggevay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

CI is red because of --auto-index-selects. I'd say it's ok to just exclude that file from --auto-index-selects treatment.

Comment thread test/sqllogictest/limit_expr.slt
Comment thread src/sql/src/plan/statement/dml.rs Outdated
@def-
def- enabled auto-merge (squash) August 4, 2026 09:03
@def-
def- merged commit 5b0bee1 into MaterializeInc:main Aug 4, 2026
123 checks passed
@def-
def- deleted the pr-sql-476 branch August 4, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants