pg: recognize psql metacommands at any top-level position (D7)#389
Merged
Conversation
psql recognizes backslash commands anywhere outside quotes, not only at line starts (engine-verified: SELECT 1; \echo MIDLINE executes, and mid-line \g buffer-sends), and a top-level backslash is never valid SQL — scan.l's operator charset has no backslash — so consuming backslash+letter through end of line can never eat a legal statement. The line-start gate was also structurally unsound: whether a segment's byte zero "was at a line start" in the original script is not decidable from the segment's own text, so any line-start rule breaks re-split stability in one direction or the other (the strict form on segments that begin with a metacommand, the lenient form on segments that begin with mid-line pseudo-metacommands). The position-context-free rule makes re-splitting stable by construction. IsMetaCommand replaces IsLineStart at all four split.go consumers and in the S3 harness's sqlForServer, giving the pipeline, the harness, and psql one converged semantic. Buffer-send commands (\g, \gset) are consumed as trivia without simulating their query-dispatch effect — a documented loud divergence for interactive idioms that dumps never produce. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The lexer treats a top-level backslash + letter like a comment, consuming through end of line. This replaces the parser's statement-boundary hook (which could not help mid-statement) and makes metacommands work uniformly: byte-zero dump headers, between statements, and mid-statement — psql executes \echo mid-buffer and the SQL continues around it (engine-verified), and pg_dump/pg_dumpall scripts lex cleanly end to end. Six psql.sql \bind statements now parse (they are consumed as the commands they are), shrinking known_failures from 160 to 154. The KB-2 parity fence used a raw-string `\n` — a literal backslash-n, now legitimately metacommand trivia — corrected to the real newline the case intended. IsLineStart is gone: the position-context-free IsMetaCommand is the single recognizer for split, lexer, and the S3 harness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fold the two open #387 review findings into the D7 change, since both sit on the sqlForServer path this PR already touches: - StripTopLevelMetacommands walks with Split's construct-skipping logic (strings, comments, dollar-quotes, COPY data — including the same-line data-mode gate), so a backslash that is string content is preserved and only true top-level metacommands are removed. The previous scan-state-blind strip corrupted SQL like SELECT 'a\nrestrict b' before sending it to the server. - The A2 segment-order check compared in the wrong direction: an extra boundary makes the segmented run fail BEFORE the server's error segment (segIdx < wantIdx). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The COPY check in StripTopLevelMetacommands used the emit bookkeeping offset as its anchor, which only advances when a metacommand is stripped — so any COPY that was not the first statement (the normal multi-statement shape) was judged by the previous statement's first word, data mode never engaged, and backslash-letter lines inside the data were stripped as metacommands. Track stmtStart separately, advancing at depth-zero semicolons exactly like Split's segment boundaries (paren depth included, so rule multi-action semicolons do not move the anchor). Stripping a metacommand does not move stmtStart: isCopyFromStdin skips metacommand trivia itself, so a statement's COPY-ness is judged from its true first word even with metacommand lines before or inside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e0bd376b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
skipWhitespaceAndComments now treats psql metacommand lines as trivia like everything else the scanner skips, so isFollowedByAtomic sees through a command between the keywords. Engine-verified: psql consumes the command and the server receives BEGIN ATOMIC — the function is created and runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Third instance of the "lexical helper unaware of metacommand trivia" family prompted a mechanical sweep of every helper doing lookahead or first-word judgment. One live case found: RestOfLineBlank saw a same-line metacommand after COPY's semicolon as meaningful content and kept data mode off, while psql executes the command and loads the data (engine-verified). A metacommand consumes the rest of the line, so it counts as blank. Sweep result for the rest: string/comment/dollar-quote internals need no trivia semantics (their own states), matchKeyword judges single word boundaries (callers skip trivia), copyscan.SkipData is data mode where backslash lines are data by design, and Split / Empty / isCopyFromStdin / skipBeginAtomic / skipWhitespaceAndComments / StripTopLevelMetacommands / the lexer already handle trivia. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Fixes D7: both line-start rules for metacommand recognition are structurally unsound at segment offset zero — whether byte zero "was a line start" in the original script is not decidable from a segment's own text, so the strict form broke re-split stability on segments beginning with a metacommand (the framework's D7 catch) and the lenient form broke it on segments beginning with mid-line pseudo-metacommands (the D6b mutation counterexample).
The rule
Top-level
\+ letter is a metacommand, at any position, consumed through end of line. Two engine-verified pillars: psql recognizes backslash commands anywhere outside quotes (SELECT 1; \echo MIDLINEexecutes; mid-line\gbuffer-sends), and a top-level backslash is never valid SQL (scan.l's operator charset has no backslash) — so the rule can never consume a legal statement, and being position-context-free makes re-split stability hold by construction.Commits
metacmd.IsMetaCommand(no line-start parameter) replacesIsLineStartat all four split.go consumers and in the S3 harness'ssqlForServer— pipeline, harness, and psql converge on one semantic.lexInitial; the parser's statement-boundary hook is removed (the lexer rule covers boundaries, byte zero, and mid-statement uniformly). Six psql.sql\bindstatements now parse, shrinking known_failures 160→154. The KB-2 parity fence's raw-string\n(a literal backslash-n, now legitimately trivia) is corrected to the real newline it intended.Documented loud divergence: buffer-send commands (
\g,\gset) are consumed as trivia without simulating their query-dispatch effect — interactive idioms dumps never produce.Verification
TestSplitMetacommandAnywhere: both line-start counterexamples pinned (each with per-segment re-split assertions), byte-zero + COPY interaction, mid-line after semicolon, string-interior immunityTestParseMetacommandAnywhere: byte-zero header, mid-statement metacommand (previously documented out-of-contract, now engine-parity), mid-line, metacommand-only input./pg/...including splittest invariants (I3 restored on the D7 shapes), pgregress 224 files (154 known / 0 new / 6 fixed)Acceptance set per thread ruling: Danny's 4 engine probes + the 6 I3 cases from the implementation prototype.
🤖 Generated with Claude Code