Skip to content

fix: parse comma-separated tuple assignments in SET lists - #2750

Open
fudianchn wants to merge 1 commit into
JSQLParser:masterfrom
fudianchn:fix/set-tuple-assignment-lists
Open

fudianchn wants to merge 1 commit into
JSQLParser:masterfrom
fudianchn:fix/set-tuple-assignment-lists

Conversation

@fudianchn

Copy link
Copy Markdown
Contributor

AI disclosure: this change was prepared with AI coding agents, reviewed and revised line by line by me.

What

Allow comma-separated tuple assignments after existing assignments in the shared SET-list grammar. Preserve scalar, tuple and subquery assignment ASTs and both output paths.

Why

UPDATE t SET a = 1, (b, c) = (2, 3) fails at the comma on upstream master. The same grammar also accepts a tuple following another assignment without a comma. PostgreSQL's UPDATE syntax defines scalar and tuple assignments as alternatives in one comma-separated list.

How

  1. Consume the repeated assignment separator before choosing the scalar or tuple alternative.
  2. Retain existing UpdateSet construction, expression-list/subquery handling and shared visitors/deparsers.
  3. Add regression tests across all eight grammar consumers, with explicit column/value ownership and reparsing of both outputs.

Root cause

The repeated scalar alternative includes the comma, but the repeated tuple alternative does not. This rejects a non-first tuple when its separator is present and accepts it when the separator is missing.

Testing

  • UpdateSetParsingTest adds 69 cases. On base bb55bb9d, 43 fail and 26 normal controls pass; the fix passes all 69. The failures cover 40 non-first tuple cases across eight entrypoints, a correlated subquery with following WHERE/RETURNING, and two invalid missing-comma inputs. Guards cover scalar-only, single/first tuple assignments, trailing commas and duplicate commas.
  • Tests cover tuple middle/end/consecutive positions, function arguments with internal commas, arithmetic, JDBC parameters and subquery RHS values. Both toString and StatementDeParser outputs are compared to the complete statement and reparsed, with explicit AST ownership checks.
  • Two isolated mutations are rejected: making the comma optional fails two rejection tests; discarding repeated scalar AST entries fails all 16 carrying scalar/tuple-first guards. Restoring the final grammar and disabling Gradle's build cache passes all 69 cases and the grammar ambiguity gate.
  • Final JDK 17 Gradle check passes with 9471 cases, zero failures/errors and 25 skipped. Final Maven clean verify/Spotless check passes with 9453 cases, zero failures/errors and 25 skipped. The changed-file license plugin check scans and passes the new Java test; the grammar header is unchanged from the base and was checked separately because the plugin does not scan it. Project JMH parseSQLStatements, unchanged 54-statement corpus, version=latest: interleaved baseline/fixed, three forks per state, two one-second warmups and five one-second measurements per fork. Baseline 30.339 ms/op, 99.9% CI [21.466,39.213]; fixed 29.500 ms/op, CI [21.600,37.401], 15 samples per state. The intervals overlap; no measurable regression in this benchmark. This does not measure a tuple-assignment-specific workload.
  • No database-server execution or Windows/macOS local test matrix was run.

Behavior notes

  • The eight library consumers are UPDATE, INSERT SET, INSERT ON DUPLICATE KEY UPDATE, INSERT ON CONFLICT DO UPDATE, UPSERT SET, MERGE UPDATE SET, piped SET and SELECT SETTINGS. The non-UPDATE cases exercise shared library grammar contracts, without claiming tuple assignment support in every database dialect.
  • Missing commas before repeated tuples are now rejected. Existing scalar-only and first-tuple forms retain their AST/output behavior. A tuple remains one UpdateSet containing its own column/value lists; a = 1, (b, c) = (2, 3) produces two UpdateSet entries.
  • No public AST API, tokens, dialect configuration or deparser changes are introduced. Merged Visit pipe expressions through shared deparser paths #2618 and Validate every UPDATE assignment through shared UpdateSet traversal #2623 already provide shared pipe rendering and complete assignment validation; this grammar correction retains those paths.

Verification of the original issue

No existing issue is linked. The failure was reproduced on upstream master bb55bb9de8377de9880b14e4fe3b3cc94bf63498; the locally verified fixed commit is af426e07110fa252d0ee34c23c54b071362d9eb2.

UPDATE t SET a = 1, (b, c) = (2, 3);
UPDATE t SET a = 1, (b, c) = (SELECT x, y FROM s), d = 4;

Both now parse and preserve the separate scalar/tuple assignments through both output paths and reparsing. UPDATE t SET a = 1 (b, c) = (2, 3) is rejected.

Signed-off-by: 付典 <fudianchn@gmail.com>
@fudianchn
fudianchn marked this pull request as ready for review October 2, 2026 07:25

This branch has not been deployed

No deployments
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.

1 participant