Skip to content

Commit 53b055b

Browse files
tianzhouclaude
andauthored
fix: preserve schema qualifiers in function/procedure bodies (#354) (#358)
Previously, schema qualifiers (e.g., public.test) were unconditionally stripped from function/procedure bodies during normalization. This broke functions with restrictive search_path settings (SET search_path = '', pg_temp, etc.) where qualified references are required. Changes: - Move schema stripping from normalization time to comparison time - Make stripSchemaQualifications skip dollar-quoted blocks so function bodies applied to embedded postgres preserve their qualifiers - Export StripSchemaPrefixFromBody for use by the diff package - Fix dollar-quote regex to match PostgreSQL grammar (no $1$ false positives) - Add unit tests for splitDollarQuotedSegments Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a79a7e1 commit 53b055b

17 files changed

Lines changed: 185 additions & 30 deletions

File tree

internal/diff/function.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func functionsEqualExceptAttributes(old, new *ir.Function) bool {
371371
if old.Name != new.Name {
372372
return false
373373
}
374-
if old.Definition != new.Definition {
374+
if !definitionsEqualIgnoringSchema(old.Definition, new.Definition, old.Schema) {
375375
return false
376376
}
377377
if old.ReturnType != new.ReturnType {
@@ -409,7 +409,7 @@ func functionsEqual(old, new *ir.Function) bool {
409409
if old.Name != new.Name {
410410
return false
411411
}
412-
if old.Definition != new.Definition {
412+
if !definitionsEqualIgnoringSchema(old.Definition, new.Definition, old.Schema) {
413413
return false
414414
}
415415
if old.ReturnType != new.ReturnType {
@@ -458,7 +458,7 @@ func functionsEqualExceptComment(old, new *ir.Function) bool {
458458
if old.Name != new.Name {
459459
return false
460460
}
461-
if old.Definition != new.Definition {
461+
if !definitionsEqualIgnoringSchema(old.Definition, new.Definition, old.Schema) {
462462
return false
463463
}
464464
if old.ReturnType != new.ReturnType {
@@ -522,6 +522,15 @@ func functionRequiresRecreate(old, new *ir.Function) bool {
522522
return false
523523
}
524524

525+
// definitionsEqualIgnoringSchema compares two function/procedure definitions,
526+
// stripping the given schema qualifier from both before comparing. This allows
527+
// definitions that differ only in schema qualification (e.g., "public.test" vs "test")
528+
// to be treated as equal, while preserving the original qualifiers in the IR for
529+
// correct DDL generation. (Issue #354)
530+
func definitionsEqualIgnoringSchema(a, b, schema string) bool {
531+
return ir.StripSchemaPrefixFromBody(a, schema) == ir.StripSchemaPrefixFromBody(b, schema)
532+
}
533+
525534
// filterNonTableParameters filters out TABLE mode parameters
526535
// TABLE parameters are output columns in RETURNS TABLE() and shouldn't be compared as input parameters
527536
func filterNonTableParameters(params []*ir.Parameter) []*ir.Parameter {

internal/diff/procedure.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func proceduresEqual(old, new *ir.Procedure) bool {
260260
if old.Name != new.Name {
261261
return false
262262
}
263-
if old.Definition != new.Definition {
263+
if !definitionsEqualIgnoringSchema(old.Definition, new.Definition, old.Schema) {
264264
return false
265265
}
266266
if old.Language != new.Language {
@@ -296,7 +296,7 @@ func proceduresEqualExceptComment(old, new *ir.Procedure) bool {
296296
if old.Name != new.Name {
297297
return false
298298
}
299-
if old.Definition != new.Definition {
299+
if !definitionsEqualIgnoringSchema(old.Definition, new.Definition, old.Schema) {
300300
return false
301301
}
302302
if old.Language != new.Language {

internal/postgres/desired_state.go

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,77 @@ func stripSchemaQualifications(sql string, schemaName string) string {
9696
return sql
9797
}
9898

99+
// Split SQL into dollar-quoted and non-dollar-quoted segments.
100+
// Schema qualifiers inside function/procedure bodies (dollar-quoted blocks)
101+
// must be preserved — the user may need them when search_path doesn't include
102+
// the function's schema (e.g., SET search_path = ''). (Issue #354)
103+
segments := splitDollarQuotedSegments(sql)
104+
var result strings.Builder
105+
result.Grow(len(sql))
106+
for _, seg := range segments {
107+
if seg.quoted {
108+
// Preserve dollar-quoted content as-is
109+
result.WriteString(seg.text)
110+
} else {
111+
result.WriteString(stripSchemaQualificationsFromText(seg.text, schemaName))
112+
}
113+
}
114+
return result.String()
115+
}
116+
117+
// dollarQuotedSegment represents a segment of SQL text, either inside or outside a dollar-quoted block.
118+
type dollarQuotedSegment struct {
119+
text string
120+
quoted bool // true if this segment is inside dollar quotes (including the delimiters)
121+
}
122+
123+
// splitDollarQuotedSegments splits SQL text into segments that are either inside or outside
124+
// dollar-quoted blocks ($$...$$, $tag$...$tag$, etc.). This allows callers to process
125+
// only the non-quoted parts while preserving function/procedure bodies verbatim.
126+
// dollarQuoteRe matches PostgreSQL dollar-quote tags: $$ or $identifier$ where the
127+
// identifier must start with a letter or underscore (not a digit). This avoids
128+
// false positives on $1, $2 etc. parameter references.
129+
var dollarQuoteRe = regexp.MustCompile(`\$(?:[a-zA-Z_][a-zA-Z0-9_]*)?\$`)
130+
131+
func splitDollarQuotedSegments(sql string) []dollarQuotedSegment {
132+
var segments []dollarQuotedSegment
133+
134+
pos := 0
135+
for pos < len(sql) {
136+
// Find the next dollar-quote opening tag
137+
loc := dollarQuoteRe.FindStringIndex(sql[pos:])
138+
if loc == nil {
139+
// No more dollar quotes — rest is unquoted
140+
segments = append(segments, dollarQuotedSegment{text: sql[pos:], quoted: false})
141+
break
142+
}
143+
144+
openStart := pos + loc[0]
145+
openEnd := pos + loc[1]
146+
tag := sql[openStart:openEnd]
147+
148+
// Add the unquoted segment before this tag
149+
if openStart > pos {
150+
segments = append(segments, dollarQuotedSegment{text: sql[pos:openStart], quoted: false})
151+
}
152+
153+
// Find the matching closing tag
154+
closeIdx := strings.Index(sql[openEnd:], tag)
155+
if closeIdx == -1 {
156+
// No closing tag — treat rest as quoted (unterminated)
157+
segments = append(segments, dollarQuotedSegment{text: sql[openStart:], quoted: true})
158+
pos = len(sql)
159+
} else {
160+
closeEnd := openEnd + closeIdx + len(tag)
161+
segments = append(segments, dollarQuotedSegment{text: sql[openStart:closeEnd], quoted: true})
162+
pos = closeEnd
163+
}
164+
}
165+
return segments
166+
}
167+
168+
// stripSchemaQualificationsFromText performs the actual schema qualification stripping on a text segment.
169+
func stripSchemaQualificationsFromText(text string, schemaName string) string {
99170
// Escape the schema name for use in regex
100171
escapedSchema := regexp.QuoteMeta(schemaName)
101172

@@ -133,7 +204,7 @@ func stripSchemaQualifications(sql string, schemaName string) string {
133204
pattern4 := fmt.Sprintf(`(?:^|[^"])%s\.([a-zA-Z_][a-zA-Z0-9_$]*)`, escapedSchema)
134205
re4 := regexp.MustCompile(pattern4)
135206

136-
result := sql
207+
result := text
137208
// Apply in order: quoted schema first to avoid double-matching
138209
result = re1.ReplaceAllString(result, "$1")
139210
result = re2.ReplaceAllString(result, "$1")

internal/postgres/desired_state_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,82 @@
11
package postgres
22

33
import (
4+
"reflect"
45
"testing"
56
)
67

8+
func TestSplitDollarQuotedSegments(t *testing.T) {
9+
tests := []struct {
10+
name string
11+
sql string
12+
expected []dollarQuotedSegment
13+
}{
14+
{
15+
name: "no dollar quotes",
16+
sql: "SELECT 1 FROM public.users;",
17+
expected: []dollarQuotedSegment{{text: "SELECT 1 FROM public.users;", quoted: false}},
18+
},
19+
{
20+
name: "simple dollar-quoted body",
21+
sql: "CREATE FUNCTION f() AS $$body$$ LANGUAGE sql;",
22+
expected: []dollarQuotedSegment{
23+
{text: "CREATE FUNCTION f() AS ", quoted: false},
24+
{text: "$$body$$", quoted: true},
25+
{text: " LANGUAGE sql;", quoted: false},
26+
},
27+
},
28+
{
29+
name: "named dollar-quote tag",
30+
sql: "AS $func$body$func$ LANGUAGE sql;",
31+
expected: []dollarQuotedSegment{
32+
{text: "AS ", quoted: false},
33+
{text: "$func$body$func$", quoted: true},
34+
{text: " LANGUAGE sql;", quoted: false},
35+
},
36+
},
37+
{
38+
name: "parameter references not treated as dollar quotes",
39+
sql: "SELECT $1 + $2 FROM t;",
40+
expected: []dollarQuotedSegment{
41+
{text: "SELECT $1 + $2 FROM t;", quoted: false},
42+
},
43+
},
44+
{
45+
name: "multiple dollar-quoted blocks",
46+
sql: "AS $$body1$$; AS $f$body2$f$;",
47+
expected: []dollarQuotedSegment{
48+
{text: "AS ", quoted: false},
49+
{text: "$$body1$$", quoted: true},
50+
{text: "; AS ", quoted: false},
51+
{text: "$f$body2$f$", quoted: true},
52+
{text: ";", quoted: false},
53+
},
54+
},
55+
{
56+
name: "unterminated dollar quote",
57+
sql: "AS $$body without end",
58+
expected: []dollarQuotedSegment{
59+
{text: "AS ", quoted: false},
60+
{text: "$$body without end", quoted: true},
61+
},
62+
},
63+
{
64+
name: "empty input",
65+
sql: "",
66+
expected: nil,
67+
},
68+
}
69+
70+
for _, tt := range tests {
71+
t.Run(tt.name, func(t *testing.T) {
72+
result := splitDollarQuotedSegments(tt.sql)
73+
if !reflect.DeepEqual(result, tt.expected) {
74+
t.Errorf("splitDollarQuotedSegments(%q)\n got: %+v\n want: %+v", tt.sql, result, tt.expected)
75+
}
76+
})
77+
}
78+
}
79+
780
func TestReplaceSchemaInSearchPath(t *testing.T) {
881
tests := []struct {
982
name string

ir/normalize.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ func normalizeView(view *View) {
280280

281281
// Strip same-schema qualifiers from view definition for consistent comparison.
282282
// This uses the same logic as function/procedure body normalization.
283-
view.Definition = stripSchemaPrefixFromBody(view.Definition, view.Schema)
283+
view.Definition = StripSchemaPrefixFromBody(view.Definition, view.Schema)
284284

285285
// Normalize triggers on the view (e.g., INSTEAD OF triggers)
286286
for _, trigger := range view.Triggers {
@@ -321,8 +321,10 @@ func normalizeFunction(function *Function) {
321321
}
322322
// Normalize function body to handle whitespace differences
323323
function.Definition = normalizeFunctionDefinition(function.Definition)
324-
// Strip current schema qualifier from function body for consistent unqualified output
325-
function.Definition = stripSchemaPrefixFromBody(function.Definition, function.Schema)
324+
// Note: We intentionally do NOT strip schema qualifiers from function bodies here.
325+
// Functions may have SET search_path that excludes their own schema, making
326+
// qualified references (e.g., public.test) necessary. Stripping is done at
327+
// comparison time in the diff package instead. (Issue #354)
326328
}
327329

328330
// normalizeFunctionDefinition normalizes function body whitespace
@@ -344,10 +346,10 @@ func normalizeFunctionDefinition(def string) string {
344346
return strings.Join(normalized, "\n")
345347
}
346348

347-
// stripSchemaPrefixFromBody removes the current schema qualifier from identifiers
349+
// StripSchemaPrefixFromBody removes the current schema qualifier from identifiers
348350
// in a function or procedure body. For example, "public.users" becomes "users".
349351
// It skips single-quoted string literals to avoid modifying string constants.
350-
func stripSchemaPrefixFromBody(body, schema string) string {
352+
func StripSchemaPrefixFromBody(body, schema string) string {
351353
if body == "" || schema == "" {
352354
return body
353355
}
@@ -445,8 +447,8 @@ func normalizeProcedure(procedure *Procedure) {
445447
}
446448
}
447449

448-
// Strip current schema qualifier from procedure body for consistent unqualified output
449-
procedure.Definition = stripSchemaPrefixFromBody(procedure.Definition, procedure.Schema)
450+
// Note: We intentionally do NOT strip schema qualifiers from procedure bodies here.
451+
// Same rationale as functions — see normalizeFunction. (Issue #354)
450452
}
451453

452454
// normalizeFunctionReturnType normalizes function return types, especially TABLE types

ir/normalize_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ func TestStripSchemaPrefixFromBody(t *testing.T) {
9393

9494
for _, tt := range tests {
9595
t.Run(tt.name, func(t *testing.T) {
96-
result := stripSchemaPrefixFromBody(tt.body, tt.schema)
96+
result := StripSchemaPrefixFromBody(tt.body, tt.schema)
9797
if result != tt.expected {
98-
t.Errorf("stripSchemaPrefixFromBody(%q, %q) = %q, want %q", tt.body, tt.schema, result, tt.expected)
98+
t.Errorf("StripSchemaPrefixFromBody(%q, %q) = %q, want %q", tt.body, tt.schema, result, tt.expected)
9999
}
100100
})
101101
}

testdata/diff/create_function/issue_354_empty_search_path/diff.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ VOLATILE
77
SET search_path = ''
88
AS $$
99
BEGIN
10-
INSERT INTO test (title) VALUES (p_title);
10+
INSERT INTO public.test (title) VALUES (p_title);
1111
END;
1212
$$;

testdata/diff/create_function/issue_354_empty_search_path/plan.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
{
1010
"steps": [
1111
{
12-
"sql": "CREATE OR REPLACE FUNCTION create_hello(\n p_title text\n)\nRETURNS void\nLANGUAGE plpgsql\nVOLATILE\nSET search_path = ''\nAS $$\nBEGIN\n INSERT INTO test (title) VALUES (p_title);\nEND;\n$$;",
12+
"sql": "CREATE OR REPLACE FUNCTION create_hello(\n p_title text\n)\nRETURNS void\nLANGUAGE plpgsql\nVOLATILE\nSET search_path = ''\nAS $$\nBEGIN\n INSERT INTO public.test (title) VALUES (p_title);\nEND;\n$$;",
1313
"type": "function",
1414
"operation": "create",
1515
"path": "public.create_hello"

testdata/diff/create_function/issue_354_empty_search_path/plan.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ VOLATILE
77
SET search_path = ''
88
AS $$
99
BEGIN
10-
INSERT INTO test (title) VALUES (p_title);
10+
INSERT INTO public.test (title) VALUES (p_title);
1111
END;
1212
$$;

testdata/diff/create_function/issue_354_empty_search_path/plan.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,6 @@ VOLATILE
1818
SET search_path = ''
1919
AS $$
2020
BEGIN
21-
INSERT INTO test (title) VALUES (p_title);
21+
INSERT INTO public.test (title) VALUES (p_title);
2222
END;
2323
$$;

0 commit comments

Comments
 (0)