fix: robustly strip psql meta commands - #4479
Open
ignatremizov wants to merge 2 commits into
Open
Conversation
Author
|
@kyleconroy FYI |
Replace naive PostgreSQL schema preprocessing with a single-pass state machine that distinguishes top-level psql meta-commands from valid SQL backslashes, literals, identifiers, comments, and dollar-quoted bodies. The previous implementation could leave pg_dump/client backslash directives in schema-loading paths or strip too aggressively, breaking valid SQL containing: - Backslashes in string literals, including `E'...'` escapes and simple `standard_conforming_strings` variants - Meta-command text in comments or documentation - Dollar-quoted function bodies, including Unicode-tagged bodies - Double-quoted identifiers and identifiers containing `$` Changes: - Add engine-aware `PreprocessSchema()` and `PreprocessSchemaForApply()` helpers so rollback removal always applies while PostgreSQL psql stripping is mode-aware. - Replace line-based PostgreSQL filtering with a single-pass lexer that tracks single quotes, double quotes, dollar quotes, line comments, nested block comments, and statement boundaries. - Handle escape-string prefixes, simple `standard_conforming_strings` changes, Unicode dollar-quote tags, identifier-boundary checks, documented psql meta-commands, and broader unknown top-level backslash directives. - Preserve SQL after a valid inline `\\` separator that follows a meta-command, including glued and one-sided-whitespace forms observed in psql 13.22 / 14.19 / 15.14 / 16.10 / 17.6 / 17.10; preserve invalid leading `\\` input instead of normalizing it into SQL. - Strip semantic psql commands such as `\connect`, includes, `\copy`, `\gexec`, `\q`, `\quit`, and `\r` with warnings in parse/codegen paths, but reject them in schema-application paths where sqlc cannot reproduce their effects safely. - Reject psql conditionals (`\if`, `\elif`, `\else`, `\endif`) instead of flattening branches and changing SQL semantics. - Remove `\copy ... from stdin` payload rows through an exact `\.` terminator in parse mode, and reject unterminated copy data. - Treat `standard_conforming_strings` and transaction-scoped script behavior as best-effort parsing aids rather than full psql emulation; report approximation warnings in parse mode while suppressing that parse-only warning for live apply mode. - Wire preprocessing and warning propagation into compiler parsing, generate processing, `createdb`, `verify`, managed `vet`, and PostgreSQL sqltest seeding paths. - Add regression coverage for documented meta-commands, unknown directives, literals, comments, dollar quotes, inline separators, semantic warnings, apply-mode rejections, copy data, line endings, and managed/PostgreSQL preprocessing rollout. Performance improvements: - Pre-allocate output buffers with `strings.Builder.Grow()`. - Keep parsing single-pass rather than rescanning line slices. - Reuse engine-aware preprocessing helpers across schema-loading paths. Testing: - `go test ./internal/migrations ./internal/compiler ./internal/schemautil ./internal/cmd ./internal/sqltest/...`
Extract the repeated apply-time schema glob/read/preprocess/warn loop into a shared helper so command setup and PostgreSQL sqltest seeding use the same schema preprocessing path. Changes: - Add `schemautil.LoadSchemasForApply()` to expand schema globs, read files, run `PreprocessSchemaForApply()`, and surface warnings through a caller-provided callback. - Update `createdb`, `verify`, and managed `vet` setup to reuse the shared loader instead of open-coding the same apply-time preprocessing loop. - Update PostgreSQL sqltest seeding helpers to reuse the shared loader for both regular and read-only database setup. - Preserve read-only PostgreSQL fixture cache behavior by hashing the preprocessed DDL returned by the shared loader. - Leave schema semantics unchanged: unsafe psql meta-commands are still rejected by apply-mode preprocessing, and invalid SQL continues to fail with the database's normal PostgreSQL error. Behavioral effect: - Apply-time callers and PostgreSQL test helpers now share one codepath, reducing drift between managed-database setup and sqltest seeding. - No special-case rewriting of pg_dump `public` schema DDL is performed; callers still see PostgreSQL's native apply-time behavior. Testing: - `go test ./internal/migrations ./internal/compiler ./internal/schemautil ./internal/cmd ./internal/sqltest/...`
ignatremizov
force-pushed
the
fix/strip-psql-meta
branch
from
August 23, 2026 12:27
5bd8a65 to
17f6e32
Compare
Author
|
@kyleconroy rebased onto the latest main again. Is there anything else needed before this can be merged? |
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.
Summary
This PR makes PostgreSQL schema preprocessing safer for
pg_dump/psql-flavored input without turning sqlc into apsqlinterpreter.It preserves valid PostgreSQL SQL, strips non-semantic client commands, warns when ignored commands may affect results, and rejects script semantics that cannot be reproduced safely.
Changes
PostgreSQL schema preprocessing
psqlmeta-commands only outside SQL lexical contexts.\meta ... \\ SQL...separator form while preserving trailing SQL, including glued and one-sided-whitespace forms accepted by supportedpsqlversions.\\input instead of normalizing it into SQL.\copy ... from stdindata blocks.Safe handling by execution mode
\connect,\copy,\gexec,\i,\ir,\q,\quit, and\r, and surface warnings when ignoring them may change results.psqlconditionals (\if,\elif,\else,\endif) instead of flattening branches and changing SQL semantics.standard_conforming_stringschanges as a best-effort parsing aid and warn when parse/codegen preprocessing must approximate session behavior.Schema-loading integration
PreprocessSchema()andPreprocessSchemaForApply()entry points.generatecreatedbverifyvetShared apply-time loading
schemautil.LoadSchemasForApply()to centralize schema glob expansion, file loading, apply-mode preprocessing, and warning delivery.Behavioral impact
pg_dump/psql-flavored schema files without corrupting valid SQL containing backslashes.psqlscript execution remains intentionally out of scope.Testing
go test ./internal/migrations ./internal/compiler ./internal/schemautil ./internal/cmd ./internal/sqltest/localgo test ./internal/migrations -coverprofile=/tmp/sqlc-migrations.coverreports100.0%statement coverage forinternal/migrations.Related
Replaces #4177.
Addresses gbarr's review comment on #4082, which closes #4065.
Co-authored-by: Andrew Benton andrew@sqlc.dev