Parse returns the tree without the protobuf round trip - #11
Merged
Conversation
pg_query_go's Parse decodes protobuf because the parse tree is built in C and that is how it crosses the cgo boundary. oliphant inherited the shape: the Go parser built a tree, ParseToProtobuf marshalled it, and Parse unmarshalled it straight back. That deep copy is about three quarters of Parse's running time and doubles its allocations — 87us to 24us on a single query, 341 allocations to 179 — and sqlc pays it on every schema and query file of every run. Parse now calls parser.ParseToTree, oliphant's one deliberate addition to pg_query_go's surface (PLAN.md now records it as the exception to "the API does not change"). ParseToProtobuf is untouched: it is upstream API, and consumers that want the bytes still get them. The round trip survives as a fallback for the one input class where it is load-bearing rather than redundant. proto3 string fields must be valid UTF-8, so a tree carrying a raw invalid byte fails to marshal, and upstream surfaces that as an error from Parse. Such a byte can only come from the input verbatim: the scanner already rejects escapes that would synthesize one, and identifier truncation is pg_mbcliplen-equivalent, so it never splits a character. utf8.ValidString is therefore a sufficient guard, and costs about 1ms on the 1.1MB stress input against the 188ms it saves there. Three properties pin the change. Across the parse corpus — 46,756 cases that parse — the direct tree is proto.Equal to the round-tripped one and the two paths agree on every error. Inputs holding an invalid byte still fail with the protobuf UTF-8 error. And the one difference protobuf hid is made explicit rather than left to be discovered: a direct tree can share subtrees, since a multi-column UPDATE ... SET (a, b, c) = (...) points every ResTarget at one MultiAssignRef source exactly as the C tree does, and serialization has no pointers to preserve. Read-only callers, which is all pg_query_go's API admits, cannot tell. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MB4HGvrQk93hmnpSPm21N8
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.
pg_query_go's
Parsedecodes protobuf because the parse tree is built in C and that is how it crosses the cgo boundary. oliphant inherited the shape: the Go parser built a tree,ParseToProtobufmarshalled it, andParseunmarshalled it straight back.That deep copy is roughly three quarters of
Parse's running time, and sqlc pays it on every schema and query file of every run:Parsenow callsparser.ParseToTree. That is oliphant's one deliberate addition to pg_query_go's surface, so PLAN.md § 1 records it as the exception to "the API does not change" and CLAUDE.md's hard rule points at it.ParseToProtobufis untouched — it is upstream API, and consumers who want the bytes still get them.The UTF-8 fallback
The round trip survives for the one input class where it is load-bearing rather than redundant. proto3 string fields must be valid UTF-8, so a tree carrying a raw invalid byte fails to marshal and upstream surfaces that as an error from
Parse:Such a byte can only come from the input verbatim. The scanner already rejects escapes that would synthesize one (
E'\xff'raisesinvalid byte sequence for encoding "UTF8", matching PostgreSQL), andtruncateIdentifierispg_mbcliplen-equivalent, so it never splits a character.utf8.ValidString(input)is therefore a sufficient guard, and it costs about 1 ms on that 1.1 MB input against the 188 ms it saves.Subtree sharing
One difference cannot be guarded away, so it is made explicit rather than left to be discovered: a direct tree can share subtrees where the round trip deep-copied them. A multi-column assignment points every
ResTargetat oneMultiAssignRefsource, exactly as the C tree does, and serialization has no pointers to preserve:526 sites across the corpus, all of this shape. Callers that only read the tree — which is all pg_query_go's API admits, and all sqlc's
convert.godoes — cannot tell.proto.Equalcannot either, since it compares by value.Testing
parser/tree_test.gopins all three properties:TestParseToTreeMatchesProtobuf— across the parse corpus, all 46,756 cases that parse, the direct tree isproto.Equalto the round-tripped one and the two paths agree on every error.TestParseToTreeInvalidUTF8— an invalid byte still fails with the protobuf error; valid-UTF-8 inputs that stress the escape and truncation paths still take the fast path and parse.TestParseToTreeSharesMultiAssignSource— the sharing above, asserted against the round trip's deep copy, with the trees still equal by value.go test ./...is green. Checked against the real consumer too: with areplacepointing here, sqlc builds and its wholeTestReplay/basecorpus passes, so every PostgreSQL golden regenerates identically.Follow-ups, not in this PR
Scan,DeparseandSummaryhave the same marshal/unmarshal shape and could take the same treatment. This is deliberately justParse, which is the one on sqlc's hot path.Generated by Claude Code