parser: handle VALUES as a parenthesized/set-op query operand; reject non-query#53
Merged
Merged
Conversation
… non-query
The leading-parenthesized-query support added earlier (parse_parenthesized_
query / fold_set_operations) only recognized SELECT, WITH, and a nested
'(' as the inner query; anything else fell through to parse_select_stmt(),
which unconditionally advances past its first token assuming it is SELECT.
This silently mis-parsed valid SQL:
(VALUES (2), (3)) -> SELECT 2, 3 (VALUES dropped; the two
rows transposed into one
1-row, 2-column select)
(SELECT 1) UNION (VALUES (2)) -> UNION whose right arm is a mangled
SELECT 2
SELECT 1 UNION VALUES (2) -> SELECT 1 (the whole UNION VALUES
arm silently dropped)
(1 + 2) -> SELECT + 2 (the `1` eaten as if it
were the SELECT keyword)
Fix:
- parse_parenthesized_query: dispatch a leading VALUES to parse_values_
stmt(); require an explicit SELECT for the select path; and reject any
other leading token (a parenthesized non-query such as `(1 + 2)`) with an
error instead of the blind parse_select_stmt() mis-parse.
- parse_setop_operand: accept a bare VALUES arm (`... UNION VALUES (..)`),
parsed as a complete operand (VALUES has no set-op tail of its own), so
the fold no longer stops and drops the operator + arm.
`(VALUES ...)` now yields the same ValuesStmt as the unparenthesized form,
a VALUES set-op arm is preserved (parenthesized or bare), and a
parenthesized scalar expression is rejected cleanly rather than silently
mis-parsed.
Tests (test_precedence_regression): parenthesized VALUES is a ValuesStmt
with both rows; a VALUES set-op arm (parenthesized and bare) is kept; a
parenthesized non-query is rejected. Full suite 37/37; the changed paths
are clean under ASan/UBSan.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FyKDAbMWGwiZq4iJx3imsg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second-pass audit finding (regression scope of the earlier leading-parenthesized-query work).
parse_parenthesized_query/fold_set_operationsonly recognizedSELECT,WITH, and a nested(as the inner query; anything else fell through toparse_select_stmt(), which unconditionally advances past its first token assuming it isSELECT. This silently mis-parsed valid SQL:(VALUES (2), (3))SELECT 2, 3(VALUES dropped; 2 rows transposed into one 1-row/2-col select)ValuesStmt(SELECT 1) UNION (VALUES (2))SELECT 2ValuesStmtarmSELECT 1 UNION VALUES (2)SELECT 1(the wholeUNION VALUESarm dropped)ValuesStmtarm(1 + 2)SELECT + 2(the1eaten as if it wereSELECT)Fix
parse_parenthesized_query: dispatch a leadingVALUEStoparse_values_stmt(); require an explicitSELECTfor the select path; and reject any other leading token (a parenthesized non-query such as(1 + 2)) with an error instead of the blindparse_select_stmt()mis-parse.parse_setop_operand: accept a bareVALUESarm (… UNION VALUES (…)), parsed as a complete operand (VALUES has no set-op tail of its own), so the fold no longer stops and drops the operator + arm.(VALUES …)now yields the sameValuesStmtas the unparenthesized form, a VALUES set-op arm is preserved (parenthesized or bare), and a parenthesized scalar expression is rejected cleanly.Tests (
test_precedence_regression)Parenthesized VALUES is a
ValuesStmtwith both rows; a VALUES set-op arm (parenthesized and bare) is kept; a parenthesized non-query is rejected. Full suite 37/37; the changed paths are clean under ASan/UBSan.Scope
TABLE <name>as a set-op operand remains unsupported (it isn't a statement type the parser recognizes anywhere) — a separate feature gap, not addressed here.