fix: normalize partial index IN-list predicates to avoid perpetual rebuild (#473)

[tianzhou]

Summary

A partial index whose WHERE predicate uses an IN-list on a varchar column produced a perpetual spurious diff — plan scheduled a concurrent rebuild on every run even when nothing changed.

Root cause. PostgreSQL stores the same predicate two semantically-identical but textually-different ways depending on how it reaches the catalog:

Source	pg_get_expr(indpred)
WHERE status IN ('new','acknowledged') (user's original DDL)	... = ANY ((ARRAY['new'::varchar, ...])::text[]) — array-level cast
The dump predicate re-parsed by embedded PG (desired state)	... = ANY (ARRAY[('new'::varchar)::text, ...]) — element-level cast

convertAnyArrayToIn in ir/normalize.go only matched the element-level form — its marker required = ANY (ARRAY[, not the wrapped = ANY ((ARRAY[. So current state (array-level) was left untouched while desired state (element-level) was rewritten to IN (...), the two Where strings never matched, and indexesStructurallyEqual reported an endless rebuild. Sibling of #200/#265.

Fix. Make convertAnyArrayToIn robust to both forms:

• skip the optional wrapping paren + whole-array ::text[] cast, and
• strip the redundant per-element ::text cast (('x'::varchar)::text and 'x'::varchar::text)

so both forms collapse to the same canonical col IN ('x'::varchar, ...). The existing CHECK-constraint normalization (which shares this function) already collapsed casts via its own preprocessing and is unaffected.

Fixes #473

Test plan

Added testdata/diff/create_index/issue_473_partial_index_in_list/ where old.sql uses the natural IN-list syntax (array-level cast) and new.sql uses the dump-rendered form (element-level cast). Before the fix this produced a spurious DROP INDEX + CREATE INDEX; after the fix the diff is empty.

PGSCHEMA_TEST_FILTER="create_index/issue_473_partial_index_in_list" go test -v ./internal/diff -run TestDiffFromFiles
PGSCHEMA_TEST_FILTER="create_index/issue_473_partial_index_in_list" go test -v ./cmd -run TestPlanAndApply

Verified end-to-end against live PostgreSQL 17: dump then plan against the same DB now reports No changes detected on the first plan. Re-ran the impacted diff categories (create_index, create_table, online, create_policy, create_view, migrate) and the full ir unit suite — all green.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a non-converging diff for partial indexes whose predicates use IN (...) on varchar columns by making predicate normalization canonicalize both catalog-rendered = ANY ((ARRAY[...])::text[]) and dump-reparsed = ANY (ARRAY[...]) forms to the same IN (...) representation.

Changes:

• Enhanced convertAnyArrayToIn to handle optional wrapping parentheses + whole-array casts and to strip redundant per-element ::text casts on varchar literals.
• Added a regression test case (issue_473_partial_index_in_list) whose old/new SQL differs only in these equivalent predicate renderings, asserting the resulting plan/diff is empty.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ir/normalize.go	Makes = ANY (ARRAY...) → IN (...) normalization robust to both array-level and element-level cast shapes for stable comparisons.
testdata/diff/create_index/issue_473_partial_index_in_list/old.sql	Repro “old” schema using a natural IN (...) partial-index predicate.
testdata/diff/create_index/issue_473_partial_index_in_list/new.sql	Repro “new” schema using the dump-rendered = ANY ((ARRAY...)::text[]) form.
testdata/diff/create_index/issue_473_partial_index_in_list/plan.txt	Asserts the plan output is “No changes detected.”
testdata/diff/create_index/issue_473_partial_index_in_list/plan.json	Asserts the JSON plan has no groups (null) for this scenario.
testdata/diff/create_index/issue_473_partial_index_in_list/plan.sql	Empty plan SQL for the no-op diff case.
testdata/diff/create_index/issue_473_partial_index_in_list/diff.sql	Empty diff SQL for the no-op diff case.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR fixes a perpetual spurious diff on partial indexes whose WHERE predicate uses an IN-list on a varchar column. The root cause is that PostgreSQL represents the same predicate in two textually distinct forms depending on how it reaches the catalog — an array-level cast = ANY ((ARRAY[...])::text[]) vs. an element-level cast = ANY (ARRAY[('x'::varchar)::text, ...]) — and the old convertAnyArrayToIn only normalised the element-level form.

• ir/normalize.go: convertAnyArrayToIn is made aware of the optional wrapping-paren form, and a new stripRedundantTextCast helper strips the redundant ::text cast so both representations collapse to the same canonical col IN ('x'::character varying, ...) string.
• testdata/diff/create_index/issue_473_partial_index_in_list/: Integration test data confirms that the natural IN (...) syntax and the dump-rendered form now produce an empty diff.

Confidence Score: 4/5

Safe to merge; the fix is tightly scoped to the two known predicate representations and has no effect on existing normalisation paths.

The algorithm correctly handles both the array-level-wrapped and element-level-cast forms, and the existing check-constraint tests continue to exercise the shared code path. The two observations — no direct unit tests for the new helpers, and the inner-paren forward scan being theoretically stoppable by a ')' in a future cast syntax — are minor concerns rather than present defects.

ir/normalize.go — specifically the arrayWrapped close-paren scan and the new stripRedundantTextCast helper, which lack dedicated unit tests.

Important Files Changed

Filename	Overview
ir/normalize.go	Core fix: convertAnyArrayToIn broadened to handle the wrapped array form; new stripRedundantTextCast helper added. Logic is correct for the described cases; no unit tests added for the new code paths.
testdata/diff/create_index/issue_473_partial_index_in_list/old.sql	Correctly defines the natural IN-list form whose catalog representation is the array-level cast form.
testdata/diff/create_index/issue_473_partial_index_in_list/new.sql	Correctly defines the dump-rendered array-level cast form; when applied to embedded PG it produces the element-level cast on inspection, exercising the new normalization path.
testdata/diff/create_index/issue_473_partial_index_in_list/plan.json	Plan output shows null groups, confirming no migration steps are generated — correct for an empty diff.
testdata/diff/create_index/issue_473_partial_index_in_list/diff.sql	Empty file confirming the normalized forms of old and new are identical — the expected outcome for the fixed bug.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["convertAnyArrayToIn(expr)"] --> B{"Find ' = ANY ('"}
    B -- "not found" --> Z["return expr unchanged"]
    B -- "found" --> C{"expr[pos] == '('?"}
    C -- "yes" --> D["arrayWrapped = true, pos++"]
    C -- "no" --> E{"HasPrefix ARRAY[?"}
    D --> E
    E -- "no" --> Z
    E -- "yes" --> F["findArrayClose to arrayEnd"]
    F --> G{"arrayEnd == -1?"}
    G -- "yes" --> Z
    G -- "no" --> H["Scan forward for first ')'"]
    H --> I{"arrayWrapped?"}
    I -- "no" --> K["closeParenIdx = outer ')'"]
    I -- "yes" --> J["Skip inner ')' and cast, scan for outer ')'"]
    J --> K
    K --> L["Split arrayContents by ', '"]
    L --> M["stripRedundantTextCast each element"]
    M --> N["Return prefix IN values suffix"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["convertAnyArrayToIn(expr)"] --> B{"Find ' = ANY ('"}
    B -- "not found" --> Z["return expr unchanged"]
    B -- "found" --> C{"expr[pos] == '('?"}
    C -- "yes" --> D["arrayWrapped = true, pos++"]
    C -- "no" --> E{"HasPrefix ARRAY[?"}
    D --> E
    E -- "no" --> Z
    E -- "yes" --> F["findArrayClose to arrayEnd"]
    F --> G{"arrayEnd == -1?"}
    G -- "yes" --> Z
    G -- "no" --> H["Scan forward for first ')'"]
    H --> I{"arrayWrapped?"}
    I -- "no" --> K["closeParenIdx = outer ')'"]
    I -- "yes" --> J["Skip inner ')' and cast, scan for outer ')'"]
    J --> K
    K --> L["Split arrayContents by ', '"]
    L --> M["stripRedundantTextCast each element"]
    M --> N["Return prefix IN values suffix"]

Loading

_{Reviews (1): Last reviewed commit: "fix: normalize partial index IN-list pre..." | Re-trigger Greptile}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.