fix: domain CHECK constraint ordering with function dependencies#261
Merged
tianzhou merged 4 commits intopgplex:mainfrom Jan 27, 2026
Merged
fix: domain CHECK constraint ordering with function dependencies#261tianzhou merged 4 commits intopgplex:mainfrom
tianzhou merged 4 commits intopgplex:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes migration ordering and comparison for PostgreSQL domains whose CHECK constraints depend on functions, ensuring both domains and tables using them are created in a dependency-safe order and compared without spurious diffs.
Changes:
- Extend
generateCreateSQLto treat domains with function dependencies similarly to function-dependent tables: create functions first, then dependent domains, then tables that use those domains. - Add domain-aware dependency helpers (
domainReferencesNewFunction,tableUsesDeferredDomain) and wire them into the create ordering. - Normalize domain CHECK constraint expressions to strip same-schema function qualifiers, and add regression test fixtures for function-dependent domains and domain-using tables.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
testdata/diff/create_domain/domain_function_table_dependency/plan.txt |
Asserts plan summary and execution order for function → domain → table chain, verifying correct creation sequencing. |
testdata/diff/create_domain/domain_function_table_dependency/plan.sql |
Captures expected SQL ordering: create function, then domain with CHECK, then table using that domain. |
testdata/diff/create_domain/domain_function_table_dependency/plan.json |
Encodes the same scenario in JSON plan form, exercising step typing and ordering for function, domain, and table. |
testdata/diff/create_domain/domain_function_table_dependency/old.sql |
Defines the empty starting schema for the function → domain → table dependency test. |
testdata/diff/create_domain/domain_function_table_dependency/new.sql |
Defines desired schema with a validating function, a domain using it in CHECK, and a table using the domain. |
testdata/diff/create_domain/domain_function_table_dependency/diff.sql |
Expected migration SQL used by diff tests to validate the correct statement ordering. |
testdata/diff/create_domain/domain_function_check_dependency/plan.txt |
Asserts plan summary and ordering for a function-dependent domain without tables, ensuring domains wait for functions. |
testdata/diff/create_domain/domain_function_check_dependency/plan.sql |
Expected SQL for creating the validating function and dependent domain in correct order. |
testdata/diff/create_domain/domain_function_check_dependency/plan.json |
JSON representation of the function + domain plan, verifying step metadata for domains with function CHECKs. |
testdata/diff/create_domain/domain_function_check_dependency/old.sql |
Empty starting schema for the function-dependent domain-only test case. |
testdata/diff/create_domain/domain_function_check_dependency/new.sql |
Desired schema with a validating function and a domain whose CHECK constraint calls it. |
testdata/diff/create_domain/domain_function_check_dependency/diff.sql |
Expected migration SQL for the function + domain test used by file-based diff tests. |
ir/normalize.go |
Enhances domain normalization to accept the domain schema and strip same-schema function qualifiers in CHECK expressions, mirroring policy expression normalization and preventing spurious diffs. |
internal/diff/diff.go |
Adjusts create-time ordering to (a) partition types into domains with/without new-function dependencies, (b) track deferred domains, (c) classify tables by function/deferred-domain usage, and (d) introduce tableUsesDeferredDomain and domainReferencesNewFunction helpers used by generateCreateSQL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #254
split out from #255
Problem
@alecthomas explained the problem quite well in #254 so I won't repeat that here.
Fix
Extended the existing function dependency handling pattern to cover domains. The codebase already splits tables into those with/without function dependencies and creates them at appropriate points relative to function creation. This change applies the same approach to domains:
Domain constraint expression normalization
While testing the ordering fix, discovered that PostgreSQL always returns fully-qualified function names in constraint definitions (e.g.,
public.validate_id(VALUE)), but source SQL typically uses unqualified names. This caused constraint definitions to not match on re-comparison, producing spurious diffs.Fix: Strip same-schema prefixes from function calls in domain CHECK constraints, matching the existing pattern in
normalizePolicyExpression(added for Issue #220). This ensures consistent comparison regardless of whether the source SQL includes schema qualification.Alternative considered: Normalize expressions to always use fully-qualified names — would require adding schema prefixes to source expressions.