Skip to content

Commit 2734247

Browse files
committed
fix(migrations): robustly strip psql meta commands without breaking SQL
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/...`
1 parent 99a7d7d commit 2734247

14 files changed

Lines changed: 2119 additions & 34 deletions

File tree

internal/cmd/createdb.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,14 @@ func CreateDB(ctx context.Context, dir, filename, querySetName string, o *Option
8484
if err != nil {
8585
return fmt.Errorf("read file: %w", err)
8686
}
87-
ddl = append(ddl, migrations.RemoveRollbackStatements(string(contents)))
87+
ddlText, warnings, err := migrations.PreprocessSchemaForApply(string(contents), string(queryset.Engine))
88+
if err != nil {
89+
return err
90+
}
91+
for _, warning := range warnings {
92+
fmt.Fprintln(o.Stderr, warning)
93+
}
94+
ddl = append(ddl, ddlText)
8895
}
8996

9097
now := time.Now().UTC().UnixNano()

internal/cmd/generate.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,9 @@ func parse(ctx context.Context, name, dir string, sql config.SQL, combo config.C
269269
return nil, true
270270
}
271271
if err := c.ParseCatalog(sql.Schema); err != nil {
272+
for _, warning := range c.Warnings() {
273+
fmt.Fprintln(stderr, warning)
274+
}
272275
fmt.Fprintf(stderr, "# package %s\n", name)
273276
if parserErr, ok := err.(*multierr.Error); ok {
274277
for _, fileErr := range parserErr.Errs() {
@@ -279,6 +282,9 @@ func parse(ctx context.Context, name, dir string, sql config.SQL, combo config.C
279282
}
280283
return nil, true
281284
}
285+
for _, warning := range c.Warnings() {
286+
fmt.Fprintln(stderr, warning)
287+
}
282288
if debugDumpCatalog.Value() == "1" {
283289
debug.Dump(c.Catalog())
284290
}

internal/cmd/process.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,12 @@ func processQuerySets(ctx context.Context, rp ResultProcessor, conf *config.Conf
121121
if err := grp.Wait(); err != nil {
122122
return err
123123
}
124-
if errored {
125-
for i, _ := range stderrs {
126-
if _, err := io.Copy(stderr, &stderrs[i]); err != nil {
127-
return err
128-
}
124+
for i := range stderrs {
125+
if _, err := io.Copy(stderr, &stderrs[i]); err != nil {
126+
return err
129127
}
128+
}
129+
if errored {
130130
return fmt.Errorf("errored")
131131
}
132132
return nil

internal/cmd/verify.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,14 @@ func Verify(ctx context.Context, dir, filename string, opts *Options) error {
102102
if err != nil {
103103
return fmt.Errorf("read file: %w", err)
104104
}
105-
ddl = append(ddl, migrations.RemoveRollbackStatements(string(contents)))
105+
ddlText, warnings, err := migrations.PreprocessSchemaForApply(string(contents), string(current.Engine))
106+
if err != nil {
107+
return err
108+
}
109+
for _, warning := range warnings {
110+
fmt.Fprintln(stderr, warning)
111+
}
112+
ddl = append(ddl, ddlText)
106113
}
107114

108115
var codegen plugin.GenerateRequest

internal/cmd/vet.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,14 @@ func (c *checker) fetchDatabaseUri(ctx context.Context, s config.SQL) (string, f
439439
if err != nil {
440440
return "", cleanup, fmt.Errorf("read file: %w", err)
441441
}
442-
ddl = append(ddl, migrations.RemoveRollbackStatements(string(contents)))
442+
ddlText, warnings, err := migrations.PreprocessSchemaForApply(string(contents), string(s.Engine))
443+
if err != nil {
444+
return "", cleanup, err
445+
}
446+
for _, warning := range warnings {
447+
fmt.Fprintln(c.Stderr, warning)
448+
}
449+
ddl = append(ddl, ddlText)
443450
}
444451

445452
resp, err := c.Client.CreateDatabase(ctx, &dbmanager.CreateDatabaseRequest{
@@ -554,7 +561,13 @@ func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
554561
if err != nil {
555562
return fmt.Errorf("read schema file: %w", err)
556563
}
557-
ddl := migrations.RemoveRollbackStatements(string(contents))
564+
ddl, warnings, err := migrations.PreprocessSchemaForApply(string(contents), string(s.Engine))
565+
if err != nil {
566+
return err
567+
}
568+
for _, warning := range warnings {
569+
fmt.Fprintln(c.Stderr, warning)
570+
}
558571
if _, err := db.ExecContext(ctx, ddl); err != nil {
559572
return fmt.Errorf("apply schema %s: %w", schema, err)
560573
}

internal/compiler/compile.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,25 @@ func (c *Compiler) parseCatalog(schemas []string) error {
5252
merr.Add(path, "", 0, err)
5353
continue
5454
}
55-
contents := migrations.RemoveRollbackStatements(string(blob))
56-
contents = migrations.RemovePsqlMetaCommands(contents)
55+
contents, warnings, err := migrations.PreprocessSchema(string(blob), string(c.conf.Engine))
56+
if err != nil {
57+
merr.Add(path, string(blob), 0, err)
58+
continue
59+
}
60+
var applyContents string
61+
if c.usesManagedAnalyzer() {
62+
applyContents, _, err = migrations.PreprocessSchemaForApply(string(blob), string(c.conf.Engine))
63+
if err != nil {
64+
merr.Add(path, string(blob), 0, err)
65+
continue
66+
}
67+
}
68+
c.warns = append(c.warns, warnings...)
5769
files = append(files, schemaFile{name: path, contents: contents})
5870
c.schema = append(c.schema, contents)
71+
if c.usesManagedAnalyzer() {
72+
c.applySchema = append(c.applySchema, applyContents)
73+
}
5974
}
6075

6176
if c.coreAnalysis {

internal/compiler/compile_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package compiler
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
9+
"github.com/sqlc-dev/sqlc/internal/config"
10+
"github.com/sqlc-dev/sqlc/internal/multierr"
11+
"github.com/sqlc-dev/sqlc/internal/opts"
12+
)
13+
14+
func TestParseCatalogManagedAnalyzerRejectsSemanticPsqlCommandsForApply(t *testing.T) {
15+
dir := t.TempDir()
16+
schema := filepath.Join(dir, "schema.sql")
17+
if err := os.WriteFile(schema, []byte("\\include extra.sql\nCREATE TABLE foo (id int);\n"), 0600); err != nil {
18+
t.Fatal(err)
19+
}
20+
21+
c, err := NewCompiler(config.SQL{
22+
Engine: config.EnginePostgreSQL,
23+
Schema: []string{schema},
24+
Database: &config.Database{
25+
Managed: true,
26+
},
27+
}, config.CombinedSettings{}, opts.Parser{})
28+
if err != nil {
29+
t.Fatal(err)
30+
}
31+
32+
err = c.ParseCatalog([]string{schema})
33+
if err == nil {
34+
t.Fatal("expected managed analyzer schema preprocessing to reject semantic psql command")
35+
}
36+
merr, ok := err.(*multierr.Error)
37+
if !ok || len(merr.Errs()) != 1 {
38+
t.Fatalf("expected one schema error, got %T: %v", err, err)
39+
}
40+
if !strings.Contains(merr.Errs()[0].Err.Error(), `psql meta-command \include is not supported`) {
41+
t.Fatalf("unexpected error: %v", err)
42+
}
43+
}

internal/compiler/engine.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ type Compiler struct {
3939
coreAnalysis bool
4040
coreDialect core.Option
4141

42-
schema []string
42+
schema []string
43+
applySchema []string
44+
warns []string
4345
}
4446

4547
// Option configures a Compiler.
@@ -159,6 +161,17 @@ func (c *Compiler) ParseCatalog(schema []string) error {
159161
return c.parseCatalog(schema)
160162
}
161163

164+
func (c *Compiler) usesManagedAnalyzer() bool {
165+
return c.analyzer != nil && c.conf.Database != nil && c.conf.Database.Managed
166+
}
167+
168+
func (c *Compiler) analyzerMigrations() []string {
169+
if c.usesManagedAnalyzer() {
170+
return c.applySchema
171+
}
172+
return c.schema
173+
}
174+
162175
func (c *Compiler) ParseQueries(queries []string, o opts.Parser) error {
163176
r, err := c.parseQueries(o)
164177
if err != nil {
@@ -172,6 +185,12 @@ func (c *Compiler) Result() *Result {
172185
return c.result
173186
}
174187

188+
// Warnings returns a copy of any non-fatal schema preprocessing warnings
189+
// collected while parsing the catalog.
190+
func (c *Compiler) Warnings() []string {
191+
return append([]string(nil), c.warns...)
192+
}
193+
175194
func (c *Compiler) Close(ctx context.Context) {
176195
if c.analyzer != nil {
177196
c.analyzer.Close(ctx)

internal/compiler/parse.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (c *Compiler) parseQuery(stmt ast.Node, pp *preprocess.Result, o opts.Parse
9393
inference.Query = rawSQL
9494
}
9595

96-
result, err := c.analyzer.Analyze(ctx, raw, inference.Query, c.schema, inference.Named)
96+
result, err := c.analyzer.Analyze(ctx, raw, inference.Query, c.analyzerMigrations(), inference.Named)
9797
if err != nil {
9898
return nil, err
9999
}

0 commit comments

Comments
 (0)