fix: varchar IN check constraints generating invalid SQL #273
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes handling of CHECK constraints with IN clauses on varchar columns to ensure idempotency during schema migrations. PostgreSQL stores these constraints using the = ANY (ARRAY[...]) syntax, and the previous code failed to properly normalize them back to the user's original IN (...) format.
Changes:
- Introduced
convertAnyArrayToSimpleIn()to strip type casts from array values for idempotent normalization - Fixed
findArrayClose()to return the position of]without requiring an immediate following) - Updated both conversion functions to properly handle optional array type casts when finding the closing parenthesis
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ir/normalize.go | Added convertAnyArrayToSimpleIn() function, updated findArrayClose() and convertAnyArrayToIn() to handle optional array type casts, improved documentation |
| testdata/diff/online/add_constraint/* | Added test case for varchar column with IN constraint |
| testdata/diff/migrate/v2/* | Updated expected output to show type casts stripped from IN clause values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When a varchar column has a CHECK constraint with IN (...), PostgreSQL stores it internally as `col::text = ANY (ARRAY['a'::varchar, ...]::text[])`. The old code failed to handle the `::text[]` cast at the end, producing invalid SQL. This fix: - Fixes findArrayClose() to handle optional array type casts like ::text[] - Updates convertAnyArrayToIn() to properly find closing paren after optional type casts - Preserves type information (e.g., ::character varying, ::text) in output Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| closeParenIdx := arrayEnd + 1 | ||
| for closeParenIdx < len(expr) && expr[closeParenIdx] != ')' { | ||
| closeParenIdx++ | ||
| } | ||
| if closeParenIdx >= len(expr) { | ||
| return expr // No closing paren found | ||
| } |
There was a problem hiding this comment.
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.
When user writes CHECK (status IN ('pending', ...)) on a varchar column,
PostgreSQL stores it with single casts: 'pending'::character varying.
But when pgschema generates CHECK (status::text IN ('pending'::character
varying, ...)), PostgreSQL stores it with double casts:
'pending'::character varying::text.
This caused idempotency failures on second apply because the normalized
forms differed.
Add two normalizations:
1. (column)::type → column::type (remove unnecessary parens)
2. ::character varying::text → ::character varying (normalize double casts)
Both forms now normalize to the same output while preserving type casts.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tianzhou
changed the title
1 participant
Fix #269
When a varchar column has a CHECK constraint with IN (...), PostgreSQL stores it internally as
col::text = ANY (ARRAY['a'::varchar, ...]::text[]). The old code failed to handle the::text[]cast at the end, producing invalid SQL.This fix: