feat: support expression in limit clause

[lmatz]

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

What's changed and what's your intention?

fix #19830

required by SQLMesh's CI test

The fix is to evaluate the expression during binding stage so that we either accept the result (it is a number >=0 after casting to bigint) or reject it (all the other cases)

Checklist

• I have written necessary rustdoc comments.
• I have added necessary unit tests and integration tests.
• I have added test labels as necessary.
• I have added fuzzing tests or opened an issue to track them.
• My PR contains breaking changes.
• My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
• My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

• My PR needs documentation updates.

Release note

[xiangjinwu]

Not as simple as it looks like. A parsing conflict can happen:

https://github.com/postgres/postgres/blob/REL_17_2/src/backend/parser/gram.y#L13233

* Allowing full expressions without parentheses causes various parsing
* problems with the trailing ROW/ROWS key words.  SQL spec only calls for
* <simple value specification>, which is either a literal or a parameter (but
* an <SQL parameter reference> could be an identifier, bringing up conflicts
* with ROW/ROWS). We solve this by leveraging the presence of ONLY (see above)
* to determine whether the expression is missing rather than trying to make it
* optional in this rule.

* c_expr covers almost all the spec-required cases (and more), but it doesn't
* cover signed numeric literals, which are allowed by the spec. So we include
* those here explicitly. We need FCONST as well as ICONST because values that
* don't fit in the platform's "long", but do fit in bigint, should still be
* accepted here. (This is possible in 64-bit Windows as well as all 32-bit
* builds.)

For example,

select 1 offset 1 row fetch first row only;
                                  ^

This is valid in PostgreSQL. When we see this row keyword, it should be part of the fetch first [n] row syntax where n is omitted, but it may be mistakenly treated as part of the row(field0, field1) expression because we will attempt to parse an expression (instead of a numerical constant) after first.

What is the actual parsing behavior when this commit sees this SQL?

[lmatz]

how about let me revert the changes for "fetch" but keep the changes for "limit",
the CI test required by SQLMesh integration needs limit simple expression only.

let fetch still stick to a literal number

I will open a new issue for the reasoning posted above

[lmatz]

or we force parentheses around the expression in Fetch just like PG

SELECT id
FROM generate_series(1, 10) AS id
ORDER BY id
OFFSET 5 ROWS
FETCH NEXT (1+4) ROWS ONLY;

id 
----
  6
  7
  8
  9
 10
(5 rows)

SELECT id
FROM generate_series(1, 10) AS id
ORDER BY id
OFFSET 5 ROWS
FETCH NEXT 1+4 ROWS ONLY;

psql:commands.sql:11: ERROR:  syntax error at or near "+"
LINE 5: FETCH NEXT 1+4 ROWS ONLY;

[xiangjinwu]

or we force parentheses around the expression in Fetch just like PG

PostgreSQL is more complex than this, but yes parenthesized expr is a subset of what PostgreSQL allows (c_expr as defined in the linked gram.y above, while limit allows a_expr. A includes C includes parenthesized expr.).

[xiangjinwu]

• This validation logic is better placed in binder, and BoundQuery::limit remains an Option<u64>. Planner is in charge of transforming SQL into a plan graph.
• We can do cast first and then try_fold_const, rather than using is_const and fold_const separately.
• The allowed casts in PostgreSQL is assign rather than explicit. (int requires implicit, decimal requires assign, jsonb requires explicit. I tested in PostgreSQL and LIMIT '1'::jsonb is not allowed.)
• When the evaluated output is None, which means SQL NULL, PostgreSQL treats it as no limit. Let's add a test case for this.
• Test error cases: wrong type, not const, eval error, negative

[lmatz]

Test error cases: wrong type, not const, eval error, negative

wrong type, negative can be constructed, I added them

but "eval error and not const" seems to be avoided by the parsing stage already