From c58efe9dd66edc894ac52cffeecd9c943d413d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BB=98=E5=85=B8?= Date: Thu, 13 Aug 2026 17:35:28 +0800 Subject: [PATCH] fix: allow scalar subquery as LIMIT row count (#2359) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 付典 --- .../net/sf/jsqlparser/parser/JSqlParserCC.jjt | 33 ++++++++----------- .../expression/LimitExpressionTest.java | 33 +++++++++++++++++++ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index 0cf4530ed..088492931 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -5383,8 +5383,19 @@ PlainSelect PlainSelect() #PlainSelect: [ LOOKAHEAD( ) orderByElements = OrderByElements() { plainSelect.setOrderByElements(orderByElements); } ] [ LOOKAHEAD(2) forClause = ForClause() {plainSelect.setForClause(forClause);} ] [ LOOKAHEAD(2) { plainSelect.setEmitChanges(true); } ] - [ LOOKAHEAD(7) limit = LimitBy() { plainSelect.setLimitBy(limit); } ] - [ LOOKAHEAD() limit = LimitWithOffset() { plainSelect.setLimit(limit); } ] + // Parse the LIMIT row count once (this accepts a parenthesized subquery too), then + // optionally attach ClickHouse's `LIMIT ... BY ...`. Checking BY right after the limit + // expression avoids a numeric LOOKAHEAD, which cannot see past a long parenthesized + // subquery and would wrongly commit to LIMIT BY (issue #2359). + [ LOOKAHEAD() limit = LimitWithOffset() + [ LOOKAHEAD() expressionList = ExpressionList() { limit.setByExpressions(expressionList); } ] + { + if (limit.getByExpressions() != null) { + plainSelect.setLimitBy(limit); + } else { + plainSelect.setLimit(limit); + } + } ] [ LOOKAHEAD() offset = Offset() { plainSelect.setOffset(offset); } ] [ LOOKAHEAD(, { limit==null }) limit = LimitWithOffset() { plainSelect.setLimit(limit); } ] [ LOOKAHEAD() fetch = Fetch() { plainSelect.setFetch(fetch); } ] @@ -6773,24 +6784,6 @@ Limit PlainLimit() #PlainLimit: } } -/** - * Clickhouse LIMIT BY - * @see SELECT Query - */ -Limit LimitBy(): -{ - Limit limit; - ExpressionList byExpressions; -} -{ - limit = LimitWithOffset() - byExpressions = ExpressionList() - { - limit.setByExpressions(byExpressions); - return limit; - } -} - Offset Offset(): { Offset offset = new Offset(); diff --git a/src/test/java/net/sf/jsqlparser/expression/LimitExpressionTest.java b/src/test/java/net/sf/jsqlparser/expression/LimitExpressionTest.java index 70dbd2103..3139530c6 100644 --- a/src/test/java/net/sf/jsqlparser/expression/LimitExpressionTest.java +++ b/src/test/java/net/sf/jsqlparser/expression/LimitExpressionTest.java @@ -11,6 +11,7 @@ import net.sf.jsqlparser.JSQLParserException; import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.statement.select.ParenthesedSelect; import net.sf.jsqlparser.statement.select.PlainSelect; import net.sf.jsqlparser.test.TestUtils; import org.junit.jupiter.api.Assertions; @@ -31,6 +32,38 @@ public void testIssue933() throws JSQLParserException { "SELECT * FROM tmp3 LIMIT (SELECT 2)", true); } + @Test + public void testIssue2359() throws JSQLParserException { + // PostgreSQL allows any expression, including a scalar subquery, as the LIMIT row + // count. A long parenthesized subquery used to fail because the ClickHouse + // "LIMIT ... BY ..." branch was chosen by a numeric LOOKAHEAD that cannot see past + // the subquery. + String sql = "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')"; + + PlainSelect plainSelect = (PlainSelect) CCJSqlParserUtil.parse(sql); + Assertions.assertTrue( + plainSelect.getLimit().getRowCount() instanceof ParenthesedSelect); + Assertions.assertNull(plainSelect.getLimitBy()); + + TestUtils.assertSqlCanBeParsedAndDeparsed(sql, true); + + // A function wrapping a scalar subquery must work as the row count too. + TestUtils.assertSqlCanBeParsedAndDeparsed( + "SELECT a FROM t LIMIT GREATEST(0, (SELECT COUNT(*) FROM u WHERE c = 'x'))", + true); + } + + @Test + public void testLimitByClickHouseUnchanged() throws JSQLParserException { + // ClickHouse "LIMIT ... BY ..." must keep parsing and round-tripping after the LIMIT + // row-count disambiguation was rewritten (issue #2359). + TestUtils.assertSqlCanBeParsedAndDeparsed("SELECT id FROM t LIMIT 5 BY id", true); + TestUtils.assertSqlCanBeParsedAndDeparsed("SELECT id FROM t LIMIT 2, 5 BY id", true); + } + @Test public void testIssue1373() throws JSQLParserException { TestUtils.assertSqlCanBeParsedAndDeparsed(