Skip to content

fix: treat a positional parameter '?' as a valid end of the ternary then-branch - #2477

Merged
manticore-projects merged 1 commit into
JSQLParser:masterfrom
fudianchn:fix/ternary-positional-then-branch
Aug 16, 2026
Merged

fix: treat a positional parameter '?' as a valid end of the ternary then-branch#2477
manticore-projects merged 1 commit into
JSQLParser:masterfrom
fudianchn:fix/ternary-positional-then-branch

Conversation

@fudianchn

Copy link
Copy Markdown
Contributor

What

A ternary whose then-branch ends with a plain positional parameter parses as a TernaryExpression again:

SELECT a ? ? : c FROM t
SELECT a ? b + ? : c FROM t
SELECT a ? ? : ? FROM t

Why / Root cause

Follow-up to #2476, found while regression-testing the snapshot build (reported in #2476 (comment)): canEndExpression() whitelisted S_PARAMETER ($N style) but not the bare ?. The bare ? is an anonymous token (no token-kind constant, outside the non-reserved range), so the scan skipped the : closing the then-branch and the parser silently committed to the jsonb reading. On current master the statements above parse as JsonOperator(a, JsonExpression(?:c)) while 7b64825 still parsed them as TernaryExpression(a, JdbcParameter, c) (regression). In the wrong tree the ? is no longer a JdbcParameter node (relevant for parameter extraction), and the wrong AST deparse round-trips stably (a ? ?:c), which is why the existing round-trip case did not catch it.

How

Add || "?".equals(t.image) to the default branch of canEndExpression(), the same image comparison already used for ")" and "]": a positional parameter is a complete primary expression, so a : directly after it closes the then-branch. The ?| / ?& operators have different images and are unaffected. The statements fixed by #2476 are unaffected as well: there the : follows null or an operator token, so the new check is not reached (j ? :key, j ? 'k' AND x = :p, j ? ? all keep their current reading).

Scope

One condition in the ternary-vs-jsonb disambiguation predicate plus an AST-shape regression test; grammar productions and AST classes are untouched.

Testing

TernaryExpressionTest.testTernaryWithPositionalParameterThenBranch pins the AST shape (root TernaryExpression, then-branch JdbcParameter / Addition, else-branch Column): it fails on current master with ClassCastException: JsonOperator cannot be cast to TernaryExpression and passes with this change. The round-trip list gains SELECT a ? b + ? : c FROM t and SELECT a ? ? : ? FROM t. All 45 TernaryExpressionTest tests pass with this change.

Performance

gradle jmh, JSQLParserBenchmark.parseSQLStatements on performance.sql, version=latest, 10 forks x 10 iterations (100 samples per run, two interleaved runs per build) on a 32-core host:

build ms/op
master (5080d19) 3.731 ± 0.023 / 3.748 ± 0.029
this PR (21ac7c4) 3.760 ± 0.029 / 3.736 ± 0.021

The two builds interleave across the repeated runs (pair 1: +0.78%, pair 2: -0.32%); every delta lies within the 99.9% confidence intervals: no regression.

…hen-branch

A bare '?' (anonymous token, no kind constant) was missing from
canEndExpression(), so a ternary whose then-branch ends with a positional
parameter was silently misread as a jsonb operator: SELECT a ? ? : c FROM t
parsed as JsonOperator(a, JsonExpression(?:c)) instead of
TernaryExpression(a, JdbcParameter, c). Recognize it by image, like the
closing brackets, and pin the AST shape with a regression test.

Follow-up to JSQLParser#2476.

Signed-off-by: 付典 <fudianchn@gmail.com>
@manticore-projects
manticore-projects merged commit f41c0b8 into JSQLParser:master Aug 16, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants