Skip to content

fix: defer functions whose signature uses a deferred table's row type (#545) - #546

Merged
tianzhou merged 4 commits into
mainfrom
fix/issue-545-function-signature-table-dep
Aug 16, 2026
Merged

fix: defer functions whose signature uses a deferred table's row type (#545)#546
tianzhou merged 4 commits into
mainfrom
fix/issue-545-function-signature-table-dep

Conversation

@tianzhou

Copy link
Copy Markdown
Contributor

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 exist at apply time.

functionReferencesNewTable() (added for #530) only regex-scanned the function body for FROM/JOIN/INTO/etc. references, so it missed signature-only dependencies like:

CREATE FUNCTION pgledger_generate_id(prefix text) RETURNS text ...;
CREATE TABLE pgledger_accounts (id text PRIMARY KEY DEFAULT pgledger_generate_id('pgla'), ...);
CREATE FUNCTION pgledger_check_account_balance_constraints(account pgledger_accounts) RETURNS void ...;

The fix generalizes the existing functionReferencesNewView() signature check (return type + parameter types) into functionSignatureReferencesRelation() 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_chain case (same function → table → function chain family): added x_check(row_x x), whose body never references table x, so only the signature carries the dependency. Before the fix it was emitted before CREATE TABLE x; now it lands after.

PGSCHEMA_TEST_FILTER="dependency/issue_530_function_table_function_chain" go test ./internal/diff -run TestDiffFromFiles
PGSCHEMA_TEST_FILTER="dependency/issue_530_function_table_function_chain" go test ./cmd -run TestPlanAndApply

Also ran the full dependency/, create_function/, and create_view/ categories locally — all pass.

🤖 Generated with Claude Code

…#545)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 16, 2026 08:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-apps

greptile-apps Bot commented Aug 16, 2026

Copy link
Copy Markdown

Greptile Summary

The 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/5

The 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

Filename Overview
internal/diff/diff.go Adds shared signature relation matching, but classifies functions referencing both deferred-table buckets into a bucket emitted before the later tables.
testdata/diff/dependency/issue_530_function_table_function_chain/new.sql Adds a valid signature-only table row-type dependency fixture for the ordinary tablesWithDeps path.
testdata/diff/dependency/issue_530_function_table_function_chain/diff.sql Updates expected DDL to emit x_check after table x.
testdata/diff/dependency/issue_530_function_table_function_chain/plan.json Updates serialized plan output and version for the added function.
testdata/diff/dependency/issue_530_function_table_function_chain/plan.sql Updates SQL plan output with the correctly ordered regression function.
testdata/diff/dependency/issue_530_function_table_function_chain/plan.txt Updates human-readable plan counts and SQL for the added function.

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]
Loading

Reviews (1): Last reviewed commit: "fix: defer functions whose signature use..." | Re-trigger Greptile

Comment thread internal/diff/diff.go
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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Comment thread internal/diff/diff.go

// Check return type (e.g., "SETOF public.actor", "actor", "SETOF actor")
if fn.ReturnType != "" {
typeName := extractBaseTypeName(fn.ReturnType)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

tianzhou and others added 2 commits August 16, 2026 17:01
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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 as TABLE("a""b" uses), this extracts "b" uses as the type, so the deferred uses dependency is missed and the function can still be emitted before the table. Skip doubled quotes while locating the closing identifier quote (as splitColumnNameAndType already does in ir/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. extractBaseTypeName removes every ", so a parameter or return type such as "a""b" becomes ab, while buildTableLookup stores the actual table name as a"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) {

@tianzhou
tianzhou merged commit 843aac0 into main Aug 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep create statements order if ORM schema export is used as desired schema (bugfix plan for pgledger)

2 participants