Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 49 additions & 9 deletions ir/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,35 @@ func normalizeCheckClause(checkClause string) string {

// applyLegacyCheckNormalizations applies the existing normalization patterns
func applyLegacyCheckNormalizations(clause string) string {
// Convert PostgreSQL's "= ANY (ARRAY[...])" format to "IN (...)" format
// Normalize unnecessary parentheses around simple identifiers before type casts.
// PostgreSQL sometimes stores "(column)::type" but the generated SQL uses "column::type".
// These are semantically equivalent, so normalize to the simpler form for idempotency.
// Pattern: (identifier)::type → identifier::type
// Examples:
// - "(status)::text" → "status::text"
// - "((name))::varchar" → "name::varchar"
parenCastRe := regexp.MustCompile(`\(([a-zA-Z_][a-zA-Z0-9_]*)\)::`)
for {
newClause := parenCastRe.ReplaceAllString(clause, `$1::`)
if newClause == clause {
break
}
clause = newClause
}

// Normalize redundant double type casts.
// When pgschema generates CHECK (status::text IN ('value'::character varying, ...)),
// PostgreSQL stores it with double casts: 'value'::character varying::text
// But when the user writes CHECK (status IN ('value', ...)), PostgreSQL stores
// just 'value'::character varying with ::text[] on the whole array.
// Normalize the double cast to single cast for idempotent comparison.
// Pattern: ::character varying::text → ::character varying
// Pattern: ::varchar::text → ::varchar
doubleCastRe := regexp.MustCompile(`::(character varying|varchar)::text\b`)
clause = doubleCastRe.ReplaceAllString(clause, "::$1")

// Convert PostgreSQL's "= ANY (ARRAY[...])" format to "IN (...)" format.
// Type casts are preserved to maintain accuracy with PostgreSQL's internal representation.
if strings.Contains(clause, "= ANY (ARRAY[") {
return convertAnyArrayToIn(clause)
}
Expand Down Expand Up @@ -1072,6 +1100,7 @@ func IsTextLikeType(typeName string) bool {
// - "status = ANY (ARRAY['active'::public.status_type])" → "status IN ('active'::public.status_type)"
// - "gender = ANY (ARRAY['M'::text, 'F'::text])" → "gender IN ('M'::text, 'F'::text)"
// - "(col = ANY (ARRAY[...])) AND (other)" → "(col IN (...)) AND (other)"
// - "col::text = ANY (ARRAY['a'::varchar]::text[])" → "col::text IN ('a'::varchar)"
func convertAnyArrayToIn(expr string) string {
const marker = " = ANY (ARRAY["
idx := strings.Index(expr, marker)
Expand All @@ -1082,16 +1111,28 @@ func convertAnyArrayToIn(expr string) string {
// Extract the part before the marker (column name with possible leading content)
prefix := expr[:idx]

// Find the closing "])" for ARRAY[...] starting after the marker
// Find the closing "]" for ARRAY[...] starting after the marker
startIdx := idx + len(marker)
arrayEnd := findArrayClose(expr, startIdx)
if arrayEnd == -1 {
return expr
}

// Extract array contents and any trailing expression
// Extract array contents
arrayContents := expr[startIdx:arrayEnd]
suffix := expr[arrayEnd+2:] // skip "])"

// Find the closing ")" for ANY(...), which may be after an optional type cast like "::text[]"
// Pattern after "]": optional "::type[]" followed by ")"
closeParenIdx := arrayEnd + 1
for closeParenIdx < len(expr) && expr[closeParenIdx] != ')' {
closeParenIdx++
}
if closeParenIdx >= len(expr) {
return expr // No closing paren found
}
Comment on lines +1126 to +1132
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simple loop to find the closing parenthesis for ANY(...) is too naive and will fail if the array type cast contains parentheses. For example, with input like 'col = ANY (ARRAY['a'::varchar(10)]::varchar(10)[])', this code would incorrectly find the ')' inside 'varchar(10)' instead of the one that closes 'ANY(...)'.

A more robust approach would be to properly parse the optional type cast (which follows the pattern '::typename' or '::typename[]' where typename can include parentheses for types like varchar(10)), or to track parenthesis nesting depth similar to how findArrayClose tracks bracket depth.

Copilot uses AI. Check for mistakes.

// Everything after the closing ")" is the suffix
suffix := expr[closeParenIdx+1:]

// Split values and preserve them as-is, including all type casts
values := strings.Split(arrayContents, ", ")
Expand All @@ -1105,8 +1146,9 @@ func convertAnyArrayToIn(expr string) string {
return fmt.Sprintf("%s IN (%s)%s", prefix, strings.Join(cleanValues, ", "), suffix)
}

// findArrayClose finds the position of the closing "])" for an ARRAY literal,
// findArrayClose finds the position of the closing "]" for an ARRAY literal,
// handling nested brackets and quoted strings properly.
// Returns the position of the "]" that closes the ARRAY.
func findArrayClose(expr string, startIdx int) int {
bracketDepth := 1 // We're already inside ARRAY[
inQuote := false
Expand Down Expand Up @@ -1134,10 +1176,8 @@ func findArrayClose(expr string, startIdx int) int {
case ']':
bracketDepth--
if bracketDepth == 0 {
// Found the closing ], verify next char is )
if i+1 < len(expr) && expr[i+1] == ')' {
return i // Return position of ]
}
// Found the closing ] for the ARRAY
return i
}
}
}
Expand Down
41 changes: 41 additions & 0 deletions ir/normalize_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package ir

import (
"testing"
)

func TestNormalizeCheckClause(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "varchar IN with ::text cast - user form (has extra parens around column)",
input: "CHECK ((status)::text = ANY (ARRAY['pending'::character varying, 'shipped'::character varying, 'delivered'::character varying]::text[]))",
expected: "CHECK (status::text IN ('pending'::character varying, 'shipped'::character varying, 'delivered'::character varying))",
},
{
name: "varchar IN without explicit cast - user form (no extra parens)",
input: "CHECK (status::text = ANY (ARRAY['pending'::character varying, 'shipped'::character varying, 'delivered'::character varying]::text[]))",
expected: "CHECK (status::text IN ('pending'::character varying, 'shipped'::character varying, 'delivered'::character varying))",
},
{
name: "varchar IN with double cast - applied form (pgschema-generated SQL stored by PostgreSQL)",
input: "CHECK (status::text = ANY (ARRAY['pending'::character varying::text, 'shipped'::character varying::text, 'delivered'::character varying::text]))",
expected: "CHECK (status::text IN ('pending'::character varying, 'shipped'::character varying, 'delivered'::character varying))",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := normalizeCheckClause(tt.input)
t.Logf("Input: %s", tt.input)
t.Logf("Output: %s", result)
t.Logf("Expected: %s", tt.expected)
if result != tt.expected {
t.Errorf("normalizeCheckClause() = %v, want %v", result, tt.expected)
}
})
}
}
2 changes: 2 additions & 0 deletions testdata/diff/online/add_constraint/diff.sql
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
ALTER TABLE orders
ADD CONSTRAINT check_amount_positive CHECK (amount > 0::numeric);
ALTER TABLE orders
ADD CONSTRAINT check_valid_status CHECK (status::text IN ('pending'::character varying, 'shipped'::character varying, 'delivered'::character varying));
4 changes: 3 additions & 1 deletion testdata/diff/online/add_constraint/new.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ CREATE TABLE public.orders (
id integer NOT NULL,
customer_id integer NOT NULL,
amount numeric(10,2),
status varchar(10) NOT NULL,
created_at timestamp with time zone DEFAULT now() NOT NULL,
CONSTRAINT check_amount_positive CHECK (amount > 0)
CONSTRAINT check_amount_positive CHECK (amount > 0),
CONSTRAINT check_valid_status CHECK (status IN ('pending', 'shipped', 'delivered'))
);
1 change: 1 addition & 0 deletions testdata/diff/online/add_constraint/old.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ CREATE TABLE public.orders (
id integer NOT NULL,
customer_id integer NOT NULL,
amount numeric(10,2),
status varchar(10) NOT NULL,
created_at timestamp with time zone DEFAULT now() NOT NULL
);
14 changes: 13 additions & 1 deletion testdata/diff/online/add_constraint/plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"pgschema_version": "1.6.1",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "563f1deff597a3640689858d007209bb87e776772a637d6ae4dc151558f8af38"
"hash": "a3bcb075d28a8e3584a89f8644ecdc54b7f0861216ae959392e9078128545620"
},
"groups": [
{
Expand All @@ -19,6 +19,18 @@
"type": "table.constraint",
"operation": "create",
"path": "public.orders.check_amount_positive"
},
{
"sql": "ALTER TABLE orders\nADD CONSTRAINT check_valid_status CHECK (status::text IN ('pending'::character varying, 'shipped'::character varying, 'delivered'::character varying)) NOT VALID;",
"type": "table.constraint",
"operation": "create",
"path": "public.orders.check_valid_status"
},
{
"sql": "ALTER TABLE orders VALIDATE CONSTRAINT check_valid_status;",
"type": "table.constraint",
"operation": "create",
"path": "public.orders.check_valid_status"
}
]
}
Expand Down
5 changes: 5 additions & 0 deletions testdata/diff/online/add_constraint/plan.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@ ALTER TABLE orders
ADD CONSTRAINT check_amount_positive CHECK (amount > 0::numeric) NOT VALID;

ALTER TABLE orders VALIDATE CONSTRAINT check_amount_positive;

ALTER TABLE orders
ADD CONSTRAINT check_valid_status CHECK (status::text IN ('pending'::character varying, 'shipped'::character varying, 'delivered'::character varying)) NOT VALID;

ALTER TABLE orders VALIDATE CONSTRAINT check_valid_status;
6 changes: 6 additions & 0 deletions testdata/diff/online/add_constraint/plan.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Summary by type:
Tables:
~ orders
+ check_amount_positive (constraint)
+ check_valid_status (constraint)

DDL to be executed:
--------------------------------------------------
Expand All @@ -14,3 +15,8 @@ ALTER TABLE orders
ADD CONSTRAINT check_amount_positive CHECK (amount > 0::numeric) NOT VALID;

ALTER TABLE orders VALIDATE CONSTRAINT check_amount_positive;

ALTER TABLE orders
ADD CONSTRAINT check_valid_status CHECK (status::text IN ('pending'::character varying, 'shipped'::character varying, 'delivered'::character varying)) NOT VALID;

ALTER TABLE orders VALIDATE CONSTRAINT check_valid_status;
Loading