fix: defer functions that reference new tables to after table creation (#530) - #531
Conversation
#530) When a function depends on a table that itself depends on another function (the chain fn1 -> table -> fn2), fn2 was incorrectly created in the first function batch before the table it queries exists. This caused a "relation does not exist" error. Split functionsWithoutViewDeps into those that reference tables being created later (tablesWithDeps) and those that do not. Table-dependent functions are now created after all tables exist, preserving the correct dependency chain.
Greptile SummaryThis PR splits newly added functions into pre-table and post-table batches so functions querying dependency-bearing new tables are emitted after those tables.
Confidence Score: 4/5The PR should not merge until incidental table-name matches can no longer defer functions required by the tables created ahead of them. Scanning raw function definitions treats comments, literals, and aliases as table dependencies, allowing the new batching logic to emit a table before a function required by its default or CHECK expression. Files Needing Attention: internal/diff/diff.go Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
F[New function]
T[New table in tablesWithDeps]
Scan{Function definition contains table name?}
Early[Create function before tablesWithDeps]
CreateTable[Create tablesWithDeps]
Late[Create deferred function]
F --> Scan
Scan -- No --> Early
Early --> CreateTable
Scan -- Yes --> CreateTable
CreateTable --> Late
Reviews (1): Last reviewed commit: "fix: defer functions that reference new ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Fixes migration statement ordering for schemas with a dependency chain function → table → function, ensuring functions that query newly created (function-dependent) tables are emitted after those tables are created, preventing relation does not exist errors during apply.
Changes:
- Split “functions without view deps” into two batches: those safe to create early vs. those referencing newly-created function-dependent tables (issue #530).
- Added
functionReferencesNewTablehelper leveraging existing identifier-matching logic. - Added a new file-based diff fixture to validate the corrected ordering for the minimal reproduction.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/diff/diff.go | Adjusts generateCreateSQL ordering to defer table-referencing functions until after creation of function-dependent tables; adds helper for detecting table references. |
| testdata/diff/dependency/issue_530_function_table_function_chain/old.sql | Adds empty “old schema” fixture for regression coverage. |
| testdata/diff/dependency/issue_530_function_table_function_chain/new.sql | Adds “new schema” fixture reproducing the function→table→function dependency chain. |
| testdata/diff/dependency/issue_530_function_table_function_chain/diff.sql | Adds expected migration output verifying function/table/function ordering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use a regex matching FROM/JOIN/INTO/UPDATE/DELETE TABLE patterns to avoid false positives from table names appearing in comments, string literals, or aliases within function bodies. This addresses the concern that a function whose body incidentally contains a table name (e.g., in a comment) could be incorrectly deferred past a table that depends on that function.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (2)
internal/diff/diff.go:2637
tableRefPatternmatches bareINTO, which can also occur in PL/pgSQL asSELECT ... INTO var. That can incorrectly classifyvaras a table reference and defer the function unnecessarily (potentially breaking ordering for tables/domains that depend on that function). Restrict the pattern toINSERT INTO(and optionallyMERGE INTO) to reduce false positives.
var tableRefPattern = regexp.MustCompile(
`(?i)(?:FROM|JOIN|INTO|UPDATE(?:\s+ONLY)?|DELETE\s+FROM|TABLE)\s+` +
`([a-z_][a-z0-9_$]*(?:\.[a-z_][a-z0-9_$]*)*)`,
)
internal/diff/diff.go:2643
- The doc comment says this approach is "avoiding false positives from comments, literals, and aliases", but the regex will still match
FROM ...text inside SQL comments or string literals (it’s a heuristic, not a parser). Please adjust the comment to avoid overstating the behavior.
// functionReferencesNewTable determines if a function body references any newly
// added table that will be created after the first function batch (tablesWithDeps).
// It looks for table names in SQL table-reference contexts (FROM, JOIN,
// INSERT INTO, UPDATE, DELETE FROM) rather than scanning the entire body,
// avoiding false positives from comments, literals, and aliases.
Problem
When a schema has the dependency chain function → table → function, pgschema creates all functions before all tables. This means a function that queries a new table fails with
relation does not existbecause the table hasn't been created yet.Minimal reproduction
The dependency graph is:
random_id→x→x_is_flaggedThe current
generateCreateSQLorder groups all functions first:random_id()✓x_is_flagged()✗ (tablexdoesn't exist yet)xFix
Split
functionsWithoutViewDepsinto those that reference tables intablesWithDepsand those that don't. Functions that query new tables are deferred to after all tables are created, preserving the correct dependency order.New order:
random_id()(no table deps)x(depends onrandom_id)x_is_flagged()(depends onx— created after all tables)Changes
internal/diff/diff.go: AddedfunctionReferencesNewTablehelper using the existingcontainsIdentifierfromview.go. ModifiedgenerateCreateSQLto split functions by table dependencies and emit table-dependent functions aftertablesWithDeps.testdata/diff/dependency/issue_530_function_table_function_chain/validates the correct ordering.Closes #530