Skip to content

Commit c58efe9

Browse files
committed
fix: allow scalar subquery as LIMIT row count (#2359)
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 #2359 Signed-off-by: 付典 <fudianchn@gmail.com>
1 parent 7ced34d commit c58efe9

2 files changed

Lines changed: 46 additions & 20 deletions

File tree

src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5383,8 +5383,19 @@ PlainSelect PlainSelect() #PlainSelect:
53835383
[ LOOKAHEAD(<K_ORDER> <K_BY>) orderByElements = OrderByElements() { plainSelect.setOrderByElements(orderByElements); } ]
53845384
[ LOOKAHEAD(2) forClause = ForClause() {plainSelect.setForClause(forClause);} ]
53855385
[ LOOKAHEAD(2) <K_EMIT> <K_CHANGES> { plainSelect.setEmitChanges(true); } ]
5386-
[ LOOKAHEAD(7) limit = LimitBy() { plainSelect.setLimitBy(limit); } ]
5387-
[ LOOKAHEAD(<K_LIMIT>) limit = LimitWithOffset() { plainSelect.setLimit(limit); } ]
5386+
// Parse the LIMIT row count once (this accepts a parenthesized subquery too), then
5387+
// optionally attach ClickHouse's `LIMIT ... BY ...`. Checking BY right after the limit
5388+
// expression avoids a numeric LOOKAHEAD, which cannot see past a long parenthesized
5389+
// subquery and would wrongly commit to LIMIT BY (issue #2359).
5390+
[ LOOKAHEAD(<K_LIMIT>) limit = LimitWithOffset()
5391+
[ LOOKAHEAD(<K_BY>) <K_BY> expressionList = ExpressionList() { limit.setByExpressions(expressionList); } ]
5392+
{
5393+
if (limit.getByExpressions() != null) {
5394+
plainSelect.setLimitBy(limit);
5395+
} else {
5396+
plainSelect.setLimit(limit);
5397+
}
5398+
} ]
53885399
[ LOOKAHEAD(<K_OFFSET>) offset = Offset() { plainSelect.setOffset(offset); } ]
53895400
[ LOOKAHEAD(<K_LIMIT>, { limit==null }) limit = LimitWithOffset() { plainSelect.setLimit(limit); } ]
53905401
[ LOOKAHEAD(<K_FETCH>) fetch = Fetch() { plainSelect.setFetch(fetch); } ]
@@ -6773,24 +6784,6 @@ Limit PlainLimit() #PlainLimit:
67736784
}
67746785
}
67756786

6776-
/**
6777-
* Clickhouse LIMIT BY
6778-
* @see <a href='https://clickhouse.com/docs/en/sql-reference/statements/select'>SELECT Query</a>
6779-
*/
6780-
Limit LimitBy():
6781-
{
6782-
Limit limit;
6783-
ExpressionList byExpressions;
6784-
}
6785-
{
6786-
limit = LimitWithOffset()
6787-
<K_BY> byExpressions = ExpressionList()
6788-
{
6789-
limit.setByExpressions(byExpressions);
6790-
return limit;
6791-
}
6792-
}
6793-
67946787
Offset Offset():
67956788
{
67966789
Offset offset = new Offset();

src/test/java/net/sf/jsqlparser/expression/LimitExpressionTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import net.sf.jsqlparser.JSQLParserException;
1313
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
14+
import net.sf.jsqlparser.statement.select.ParenthesedSelect;
1415
import net.sf.jsqlparser.statement.select.PlainSelect;
1516
import net.sf.jsqlparser.test.TestUtils;
1617
import org.junit.jupiter.api.Assertions;
@@ -31,6 +32,38 @@ public void testIssue933() throws JSQLParserException {
3132
"SELECT * FROM tmp3 LIMIT (SELECT 2)", true);
3233
}
3334

35+
@Test
36+
public void testIssue2359() throws JSQLParserException {
37+
// PostgreSQL allows any expression, including a scalar subquery, as the LIMIT row
38+
// count. A long parenthesized subquery used to fail because the ClickHouse
39+
// "LIMIT ... BY ..." branch was chosen by a numeric LOOKAHEAD that cannot see past
40+
// the subquery.
41+
String sql = "WITH some_table AS (SELECT 1 AS some_column), "
42+
+ "another_table AS (SELECT 'some_value' AS condition_column) "
43+
+ "SELECT some_column FROM some_table ORDER BY some_column "
44+
+ "LIMIT (SELECT COUNT(*) FROM another_table WHERE condition_column = 'some_value')";
45+
46+
PlainSelect plainSelect = (PlainSelect) CCJSqlParserUtil.parse(sql);
47+
Assertions.assertTrue(
48+
plainSelect.getLimit().getRowCount() instanceof ParenthesedSelect);
49+
Assertions.assertNull(plainSelect.getLimitBy());
50+
51+
TestUtils.assertSqlCanBeParsedAndDeparsed(sql, true);
52+
53+
// A function wrapping a scalar subquery must work as the row count too.
54+
TestUtils.assertSqlCanBeParsedAndDeparsed(
55+
"SELECT a FROM t LIMIT GREATEST(0, (SELECT COUNT(*) FROM u WHERE c = 'x'))",
56+
true);
57+
}
58+
59+
@Test
60+
public void testLimitByClickHouseUnchanged() throws JSQLParserException {
61+
// ClickHouse "LIMIT ... BY ..." must keep parsing and round-tripping after the LIMIT
62+
// row-count disambiguation was rewritten (issue #2359).
63+
TestUtils.assertSqlCanBeParsedAndDeparsed("SELECT id FROM t LIMIT 5 BY id", true);
64+
TestUtils.assertSqlCanBeParsedAndDeparsed("SELECT id FROM t LIMIT 2, 5 BY id", true);
65+
}
66+
3467
@Test
3568
public void testIssue1373() throws JSQLParserException {
3669
TestUtils.assertSqlCanBeParsedAndDeparsed(

0 commit comments

Comments
 (0)