fix: defer functions whose signature uses a deferred table's row type (#545) - #546
Conversation
…#545) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes migration ordering when a table is deferred (due to function-referenced defaults/generated expressions/CHECKs) by also deferring any newly-created functions whose signature (return type or parameter types) references that table’s implicit composite row type—preventing type "<table>" does not exist failures at apply time.
Changes:
- Generalizes the existing view-signature dependency logic into
functionSignatureReferencesRelation()and reuses it for both views and deferred tables. - Extends
functionReferencesNewTable()to consider function signature dependencies in addition to body SQL table references. - Updates the existing dependency regression test case to cover a signature-only table dependency (
x_check(row_x x)).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/diff/diff.go | Adds a shared signature dependency helper and uses it to correctly defer functions that depend on deferred table/view composite types. |
| testdata/diff/dependency/issue_530_function_table_function_chain/new.sql | Adds a signature-only dependency function to reproduce #545 behavior. |
| testdata/diff/dependency/issue_530_function_table_function_chain/diff.sql | Updates expected migration SQL ordering to place x_check after CREATE TABLE x. |
| testdata/diff/dependency/issue_530_function_table_function_chain/plan.sql | Updates expected planned SQL output for the new function and corrected ordering. |
| testdata/diff/dependency/issue_530_function_table_function_chain/plan.txt | Updates expected human-readable plan output (counts + ordering). |
| testdata/diff/dependency/issue_530_function_table_function_chain/plan.json | Updates expected JSON plan output (includes x_check, version bump). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Greptile SummaryThe PR generalizes function-signature relation detection so functions using deferred tables' implicit composite types are emitted later, and adds a signature-only dependency regression fixture. The common deferred-function bucket still precedes one of the table buckets included by its lookup.
Confidence Score: 4/5The PR should not merge until functions referencing tablesAfterTableDomains are emitted after those tables. The signature check correctly fixes the tested deferred-table path, but its lookup also contains a later table bucket, allowing generated migrations to create a function before its required composite type exists. Files Needing Attention: internal/diff/diff.go Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[New base table] --> B[Domain based on base row type]
B --> C[Table using deferred domain]
C --> D[tablesAfterTableDomains]
E[Function signature uses table C row type] --> F[functionsWithTableDeps]
F --> G[CREATE FUNCTION]
D --> H[CREATE TABLE C]
G -->|currently emitted first| I[Missing composite type error]
H --> J[Composite type becomes available]
Reviews (1): Last reviewed commit: "fix: defer functions whose signature use..." | Re-trigger Greptile |
A function whose signature or body references a table in tablesAfterTableDomains was emitted in the functionsWithTableDeps batch, which comes before that final table batch, so CREATE FUNCTION failed with 'type ... does not exist'. Split out a functionsAfterAllTables batch emitted after tablesAfterTableDomains. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||
| // Check return type (e.g., "SETOF public.actor", "actor", "SETOF actor") | ||
| if fn.ReturnType != "" { | ||
| typeName := extractBaseTypeName(fn.ReturnType) |
There was a problem hiding this comment.
Confirmed and fixed in 055f9d0. Reproduced with a new test case (dependency/issue_545_returns_table_ref): a function declared RETURNS TABLE(r x) where x is a deferred table was emitted before the table, since the output columns only appear in pg_get_function_result as TABLE(r x) and extractBaseTypeName left that expression unparsed. functionSignatureReferencesRelation now parses the TABLE(...) column list (paren- and quote-aware for types like numeric(10,2) and quoted identifiers) and checks each output column type.
pg_get_function_arguments excludes TABLE-mode output columns, and pg_get_function_result renders them as 'TABLE(r uses)', which extractBaseTypeName left unparsed. A function declared RETURNS TABLE(r deferred_table) was therefore emitted before the table existed. Parse the TABLE(...) column list and check each output column type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The TABLE(r uses) output-column reference is now exercised against the same last-batch table as the parameter-type reference, covering both the TABLE(...) parsing and the functionsAfterAllTables deferral in one case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
internal/diff/diff.go:2548
- The quoted output-column parser treats the first
"as the terminator, but PostgreSQL escapes an embedded quote as"". For a valid result such asTABLE("a""b" uses), this extracts"b" usesas the type, so the deferredusesdependency is missed and the function can still be emitted before the table. Skip doubled quotes while locating the closing identifier quote (assplitColumnNameAndTypealready does inir/normalize.go:503-513).
if strings.HasPrefix(col, `"`) {
if end := strings.Index(col[1:], `"`); end >= 0 {
typeExpr = col[end+2:]
internal/diff/diff.go:2818
- This signature check still misses valid deferred table names containing an escaped quote.
extractBaseTypeNameremoves every", so a parameter or return type such as"a""b"becomesab, whilebuildTableLookupstores the actual table name asa"b; the keys can never match and the function remains in the early batch. Parse/unescape each quoted identifier component while preserving embedded quotes instead of deleting all quote characters.
// A table also defines an implicit composite row type, so a function using
// it as a parameter or return type must be created after the table.
if functionSignatureReferencesRelation(fn, newTables) {
Summary
When a table is deferred to the second table batch (because a column DEFAULT / generated expression / CHECK references a newly created function), any function using that table's implicit composite row type in its signature (parameter or return type) was still emitted in the first function batch — before the table exists — causing
type "<table>" does not existat apply time.functionReferencesNewTable()(added for #530) only regex-scanned the function body forFROM/JOIN/INTO/etc. references, so it missed signature-only dependencies like:The fix generalizes the existing
functionReferencesNewView()signature check (return type + parameter types) intofunctionSignatureReferencesRelation()and applies it to deferred tables as well — PostgreSQL exposes both tables and views as composite types, so the same rule applies.Fixes #545
Test plan
Folded the reproducing scenario into the existing
testdata/diff/dependency/issue_530_function_table_function_chaincase (same function → table → function chain family): addedx_check(row_x x), whose body never references tablex, so only the signature carries the dependency. Before the fix it was emitted beforeCREATE TABLE x; now it lands after.Also ran the full
dependency/,create_function/, andcreate_view/categories locally — all pass.🤖 Generated with Claude Code