GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||
GeezerVison is showing github.com — fix: allow scalar subquery as LIMIT row count (#2359) by fudianchn · Pull Request #2464 · JSQLParser/JSqlParser Conversation
|
| build | ms/op |
|---|---|
master (LOOKAHEAD(7) LimitBy()) |
3.598 ± 0.017 |
| this change | 3.603 ± 0.017 |
The +0.14% delta lies within the 99.9% confidence intervals (the CIs overlap almost entirely): no regression.
WITH some_table AS (SELECT 1 AS some_column), another_table AS (SELECT 'some_value' AS condition_column) SELECT some_column FROM some_table ORDER BY some_column LIMIT (SELECT COUNT(*) FROM another_table WHERE condition_column = 'some_value')
fails on master and parses + round-trips with this change.
Fixes #2359
Sorry, something went wrong.
There was an error while loading. Please reload this page.
PostgreSQL allows any expression, including a scalar subquery, as the LIMIT row count, e.g. LIMIT (SELECT COUNT(*) FROM t WHERE ...) or LIMIT GREATEST(0, (SELECT ...)). JSQLParser rejected these. PlainSelect disambiguated the ClickHouse "LIMIT ... BY ..." branch from a plain limit with a numeric LOOKAHEAD(7) on LimitBy(). A numeric lookahead cannot see past a long parenthesized subquery, so for LIMIT (subquery) it wrongly committed to the LIMIT BY branch and then failed at the missing BY keyword. (Short subqueries such as LIMIT (SELECT 1) happened to stay under the lookahead window and worked, which is why the bug only surfaced for longer ones.) The disambiguation is moved to where it belongs: parse the LIMIT row count once via LimitWithOffset() (which already accepts a parenthesized subquery), then check the immediately following token for BY. A token scan for BY would not work, because a subquery may contain ORDER BY (a K_BY). The now-unused LimitBy() production is removed; LimitWithOffset already carries byExpressions, so no AST or public API change. All LIMIT shapes keep working: LIMIT n, LIMIT n, m, LIMIT n OFFSET m, OFFSET m LIMIT n, LIMIT ALL, and ClickHouse LIMIT n BY ... / LIMIT n, m BY ... Fixes JSQLParser#2359 Signed-off-by: 付典 <[email protected]>
|
This is the much better solution of course, thank you for fixing this! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
[BUG] JSQLParser Version 5.3: LIMIT with subquery fails: Was expecting: "BY"
There was an error while loading. Please reload this page.