fix: allow scalar subquery as LIMIT row count (#2359)

[fudianchn]

What

Allow any expression, including a scalar subquery, as the LIMIT row count, e.g.:

LIMIT (SELECT COUNT(*) FROM t WHERE ...)
LIMIT GREATEST(0, (SELECT COUNT(*) FROM t WHERE ...))

JSQLParser currently rejects these with Encountered: <EOF> ... Was expecting: "BY".

Why / Root cause

As you noted in the issue, a LOOKAHEAD is too tight. 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 (SELECT COUNT(*) FROM ... WHERE ...) it wrongly committed to the LIMIT BY branch and then failed at the missing BY. Short subqueries such as LIMIT (SELECT 1) happened to stay under the 7-token window and worked, which is why only longer ones failed.

A naive fix (scanning ahead for a BY token) would not work, because a subquery may legitimately contain ORDER BY, whose BY is the same K_BY token.

How

The disambiguation is moved to where it belongs: parse the LIMIT row count once via LimitWithOffset() (which already accepts a parenthesized subquery through its LOOKAHEAD(3) ParenthesedSelect() branch), then check the immediately following token for BY:

[ LOOKAHEAD(<K_LIMIT>) limit = LimitWithOffset()
    [ LOOKAHEAD(<K_BY>) <K_BY> expressionList = ExpressionList() { limit.setByExpressions(expressionList); } ]
    {
        if (limit.getByExpressions() != null) {
            plainSelect.setLimitBy(limit);
        } else {
            plainSelect.setLimit(limit);
        }
    } ]

LimitWithOffset already carries byExpressions, so there is no AST and no public API change. The now-unused LimitBy() production is removed (it was only ever called from this one site).

Scope

All existing 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 .... The previously-unsupported OFFSET m LIMIT n BY x ordering stays unsupported (out of scope, unchanged).

Testing

LimitExpressionTest.testIssue2359 reproduces the issue (fails on master, passes with this change) and asserts the row count is a ParenthesedSelect stored on limit (not limitBy). testLimitByClickHouseUnchanged guards the rewritten LIMIT ... BY ... path.

Local: spotlessApply, checkstyleMain/Test, spotbugsMain, pmdMain clean; SelectTest (725), ClickHouseTest (15), LimitExpressionTest (6) and the full test suite green apart from pre-existing failures unrelated to this change (ParserKeywordsUtilsTest writing a temp file to C:\WINDOWS, and Mockito MockMaker init on JDK 21).

Performance

gradle jmh, JSQLParserBenchmark.parseSQLStatements on performance.sql, version=latest, 10 forks × 10 iterations (100 samples) on a 32-core host:

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.

Verification of the original issue

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

[manticore-projects]

This is the much better solution of course, thank you for fixing this!