From 72acd29a41c186c44bfb21c468d5e3461e527f0a Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Wed, 28 Jan 2026 10:10:26 -0800 Subject: [PATCH 1/2] fix: incorrect parentheses in partial index WHERE clause with IN and AND (#264) The convertAnyArrayToIn() function assumed ARRAY[...] was at the end of the expression. When additional conditions followed (like AND ...), the simple TrimSuffix approach failed, leaving stray brackets and parens. Fixed by properly parsing to find the matching ]) closure, handling nested parentheses and quoted strings. Co-Authored-By: Claude Opus 4.5 --- ir/normalize.go | 77 +++++++++++++++---- .../diff/online/add_partial_index/diff.sql | 2 +- .../diff/online/add_partial_index/new.sql | 3 +- .../diff/online/add_partial_index/old.sql | 1 + .../diff/online/add_partial_index/plan.json | 4 +- .../diff/online/add_partial_index/plan.sql | 2 +- .../diff/online/add_partial_index/plan.txt | 2 +- 7 files changed, 70 insertions(+), 21 deletions(-) diff --git a/ir/normalize.go b/ir/normalize.go index 62e045af..d8744f84 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -1071,35 +1071,82 @@ func IsTextLikeType(typeName string) bool { // Example transformations: // - "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)" func convertAnyArrayToIn(expr string) string { - if !strings.Contains(expr, "= ANY (ARRAY[") { + const marker = " = ANY (ARRAY[" + idx := strings.Index(expr, marker) + if idx == -1 { return expr } - // Extract the column name and values - parts := strings.Split(expr, " = ANY (ARRAY[") - if len(parts) != 2 { + // Extract the part before the marker (column name with possible leading content) + prefix := expr[:idx] + + // Find the closing "])" for ARRAY[...] starting after the marker + startIdx := idx + len(marker) + arrayEnd := findArrayClose(expr, startIdx) + if arrayEnd == -1 { return expr } - columnName := strings.TrimSpace(parts[0]) - - // Remove the closing parentheses and brackets - valuesPart := parts[1] - valuesPart = strings.TrimSuffix(valuesPart, "])") - valuesPart = strings.TrimSuffix(valuesPart, "])) ") - valuesPart = strings.TrimSuffix(valuesPart, "]))") - valuesPart = strings.TrimSuffix(valuesPart, "])") + // Extract array contents and any trailing expression + arrayContents := expr[startIdx:arrayEnd] + suffix := expr[arrayEnd+2:] // skip "])" // Split values and preserve them as-is, including all type casts - values := strings.Split(valuesPart, ", ") + values := strings.Split(arrayContents, ", ") var cleanValues []string for _, val := range values { val = strings.TrimSpace(val) cleanValues = append(cleanValues, val) } - // Return converted format: "column IN ('val1'::type, 'val2'::type)" - return fmt.Sprintf("%s IN (%s)", columnName, strings.Join(cleanValues, ", ")) + // Return converted format: "prefix IN (values)suffix" + return fmt.Sprintf("%s IN (%s)%s", prefix, strings.Join(cleanValues, ", "), suffix) +} + +// findArrayClose finds the position of the closing "])" for an ARRAY literal, +// handling nested parentheses and quoted strings properly. +func findArrayClose(expr string, startIdx int) int { + bracketDepth := 1 // We're already inside ARRAY[ + parenDepth := 0 + inQuote := false + + for i := startIdx; i < len(expr); i++ { + ch := expr[i] + + if inQuote { + if ch == '\'' { + // Check for escaped quote '' + if i+1 < len(expr) && expr[i+1] == '\'' { + i++ // Skip escaped quote + continue + } + inQuote = false + } + continue + } + + switch ch { + case '\'': + inQuote = true + case '[': + bracketDepth++ + 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 ] + } + } + case '(': + parenDepth++ + case ')': + parenDepth-- + } + } + + return -1 // Not found } diff --git a/testdata/diff/online/add_partial_index/diff.sql b/testdata/diff/online/add_partial_index/diff.sql index 6cf158fb..77965216 100644 --- a/testdata/diff/online/add_partial_index/diff.sql +++ b/testdata/diff/online/add_partial_index/diff.sql @@ -1 +1 @@ -CREATE INDEX IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status); +CREATE INDEX IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE (status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status)) AND (is_active IS NOT NULL); diff --git a/testdata/diff/online/add_partial_index/new.sql b/testdata/diff/online/add_partial_index/new.sql index 8f9a0f30..c47e65bd 100644 --- a/testdata/diff/online/add_partial_index/new.sql +++ b/testdata/diff/online/add_partial_index/new.sql @@ -7,8 +7,9 @@ CREATE TABLE public.orders ( order_date date, total_amount numeric(10,2), payment_status text, + is_active boolean, created_at timestamp with time zone, updated_at timestamp with time zone ); -CREATE INDEX idx_active_orders_customer_date ON public.orders USING btree (customer_id, order_date DESC, total_amount) WHERE status IN ('pending'::order_status, 'processing'::order_status, 'confirmed'::order_status); \ No newline at end of file +CREATE INDEX idx_active_orders_customer_date ON public.orders USING btree (customer_id, order_date DESC, total_amount) WHERE (status IN ('pending'::order_status, 'processing'::order_status, 'confirmed'::order_status)) AND (is_active IS NOT NULL); \ No newline at end of file diff --git a/testdata/diff/online/add_partial_index/old.sql b/testdata/diff/online/add_partial_index/old.sql index 4928b62c..9b2e6131 100644 --- a/testdata/diff/online/add_partial_index/old.sql +++ b/testdata/diff/online/add_partial_index/old.sql @@ -7,6 +7,7 @@ CREATE TABLE public.orders ( order_date date, total_amount numeric(10,2), payment_status text, + is_active boolean, created_at timestamp with time zone, updated_at timestamp with time zone ); \ No newline at end of file diff --git a/testdata/diff/online/add_partial_index/plan.json b/testdata/diff/online/add_partial_index/plan.json index 312cdeae..de8bb746 100644 --- a/testdata/diff/online/add_partial_index/plan.json +++ b/testdata/diff/online/add_partial_index/plan.json @@ -3,13 +3,13 @@ "pgschema_version": "1.6.1", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "40ed646f28de84540200a1acd21fc416700f08c6779bb8f105b423be70a5df8c" + "hash": "d3818157c6994cfe669982e5174d7e36ada8c106bb8d340c392c7cbe64ebc135" }, "groups": [ { "steps": [ { - "sql": "CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status);", + "sql": "CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE (status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status)) AND (is_active IS NOT NULL);", "type": "table.index", "operation": "create", "path": "public.orders.idx_active_orders_customer_date" diff --git a/testdata/diff/online/add_partial_index/plan.sql b/testdata/diff/online/add_partial_index/plan.sql index 99e4ca03..9d90c515 100644 --- a/testdata/diff/online/add_partial_index/plan.sql +++ b/testdata/diff/online/add_partial_index/plan.sql @@ -1,4 +1,4 @@ -CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status); +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE (status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status)) AND (is_active IS NOT NULL); -- pgschema:wait SELECT diff --git a/testdata/diff/online/add_partial_index/plan.txt b/testdata/diff/online/add_partial_index/plan.txt index f29ad623..110e9170 100644 --- a/testdata/diff/online/add_partial_index/plan.txt +++ b/testdata/diff/online/add_partial_index/plan.txt @@ -11,7 +11,7 @@ DDL to be executed: -------------------------------------------------- -- Transaction Group #1 -CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status); +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_active_orders_customer_date ON orders (customer_id, order_date DESC, total_amount) WHERE (status IN ('pending'::public.order_status, 'processing'::public.order_status, 'confirmed'::public.order_status)) AND (is_active IS NOT NULL); -- Transaction Group #2 -- pgschema:wait From 117abc1e3dc0641210ff34887140ab5cac59af20 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Wed, 28 Jan 2026 10:19:25 -0800 Subject: [PATCH 2/2] refactor: remove unused parenDepth in findArrayClose Address Copilot review feedback - the parenDepth variable was tracked but never used in the matching logic. Removed it and updated the doc comment to accurately describe what the function does. Co-Authored-By: Claude Opus 4.5 --- ir/normalize.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/ir/normalize.go b/ir/normalize.go index d8744f84..c7d8f964 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -1106,10 +1106,9 @@ func convertAnyArrayToIn(expr string) string { } // findArrayClose finds the position of the closing "])" for an ARRAY literal, -// handling nested parentheses and quoted strings properly. +// handling nested brackets and quoted strings properly. func findArrayClose(expr string, startIdx int) int { bracketDepth := 1 // We're already inside ARRAY[ - parenDepth := 0 inQuote := false for i := startIdx; i < len(expr); i++ { @@ -1140,10 +1139,6 @@ func findArrayClose(expr string, startIdx int) int { return i // Return position of ] } } - case '(': - parenDepth++ - case ')': - parenDepth-- } }