fix: order multi-file dump includes by dependency, not directory (#580)

[tianzhou]

Summary

dump --multi-file wrote its \i includes in a fixed directory sequence (functions/ before tables/ before views/), discarding the cross-category dependency order the diff package had already computed for the single-file dump. Any function whose signature uses a table's or view's row type landed before that relation, so the generated main.sql failed to apply with type "..." does not exist.

The include order now follows each file's first appearance in the diff sequence, which is already topologically ordered. One adjustment is needed: a table's triggers, policies, and deferred constraints are bundled into the table's file even though the diff emits them after functions, so functions whose signature references no relation are hoisted ahead of the first table file. Functions that do reference a relation's row type keep the diff's placement, after that relation. The relation check reuses the diff package's existing functionSignatureReferencesRelation through a small exported wrapper rather than duplicating it.

This also fixes the domain-with-function-CHECK case (domains/ was emitted before functions/) as a side effect of following the diff order.

Known gaps left for follow-up PRs, all of which also fail in single-file dump and plan today:

• Aggregate whose input or state type is a view row type is emitted before the view (and before its own transition function) by the diff package.
• SQL-language function whose body queries a view is emitted before the view; the diff's view dependency check is signature-only.
• A function used by a policy on one table while taking another table's row type cannot be satisfied by file-level ordering while those statements stay bundled in the table file.

Addresses the multi-file include ordering in #580. The aggregate-to-view and SQL-body-to-view cases from that issue are diff-package ordering bugs and will be fixed in a follow-up PR, so #580 stays open until then.
Fixes #579

Test plan

• New TestDumpCommand_Issue580MultiFileIncludeOrder in cmd/dump/multifile_integration_test.go covers both dump --multi-file orders \i includes by fixed object-category buckets, not by dependency #580 variants (table row type parameter, view row type return), the type does not exist #579 shape, trigger and policy functions that must stay ahead of their tables, and a domain CHECK function. It asserts the pairwise include order and replays the multi-file output into an empty schema. Fails before the fix on three files, passes after.
• Existing TestCreateMultiFileOutput, TestMultiFileIncludeOrderDeterministic, TestDumpFormatterHelpers, TestIncludeIntegration, and TestIgnore* pass.

go test ./cmd/dump -run 'TestDumpCommand_Issue580MultiFileIncludeOrder|TestCreateMultiFileOutput|TestMultiFileIncludeOrderDeterministic' -v

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR changes multi-file dump generation so main.sql follows dependency-oriented first appearance rather than a fixed directory sequence, and exposes relation-signature detection for the formatter.

• Groups statements by output file while retaining their first position in the diff sequence.
• Hoists functions considered independent of table and view row types ahead of table files.
• Adds an integration test covering relation-typed functions, trigger and policy helpers, and domain-check dependencies.
• Updates modular-schema documentation to describe dependency-based include ordering.

Confidence Score: 4/5

The PR is not yet safe to merge because multi-file dumps can move body-dependent functions before tables that must exist when those functions are created.

The new hoisting predicate models only relation types in function signatures, while the diff's established ordering also accounts for table references in function bodies; discarding that dependency can make generated dumps fail to replay.

Files Needing Attention: internal/dump/formatter.go

Important Files Changed

Filename	Overview
internal/dump/formatter.go	Replaces directory ordering with first-appearance ordering, but its signature-only hoisting can move body-dependent functions ahead of required tables.
internal/diff/diff.go	Exposes existing relation lookup and function-signature dependency detection through small wrappers.
cmd/dump/multifile_integration_test.go	Adds end-to-end replay coverage for several cross-category dependencies, but not scalar-signature functions whose bodies query deferred tables.
docs/workflow/modular-schema-files.mdx	Updates documentation to describe dependency-based include ordering.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  D[Topologically ordered diff steps] --> G[Group statements by object file]
  G --> F[Record each file's first appearance]
  F --> H{Function signature references relation?}
  H -->|No| B[Hoist before first table]
  H -->|Yes| K[Keep diff position]
  B --> M[Write main.sql includes]
  K --> M

Loading

_{Reviews (1): Last reviewed commit: "fix: order multi-file dump includes by d..." | Re-trigger Greptile}

[Copilot]

🟡 Changes recommended

The function “hoisting” rule in multi-file ordering can override diff-intended ordering for functions that depend on relations via their bodies (not signatures), potentially reintroducing replay failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes dump --multi-file producing a main.sql that could not be replayed due to \i include lines being ordered by fixed directory buckets instead of dependency order (notably functions/aggregates that depend on table/view composite row types).

Changes:

• Reworks multi-file dump include ordering to follow the diff package’s dependency/topological sequence, rather than a fixed directory order.
• Exposes small helper wrappers in internal/diff to reuse existing “function signature references relation” logic from the dump formatter.
• Adds an integration test to validate correct cross-category ordering and replayability of the generated multi-file output; updates docs to reflect the new behavior.

File summaries

File	Description
internal/dump/formatter.go	Changes multi-file grouping and computes main.sql include order based on diff sequence, with a function-hoisting adjustment.
internal/diff/diff.go	Exports wrappers to build a relation lookup and to test whether a function signature references a relation.
cmd/dump/multifile_integration_test.go	Adds an integration test that asserts include ordering and replays the dumped files into an empty schema.
docs/workflow/modular-schema-files.mdx	Updates documentation to describe dependency-based include ordering for multi-file dumps.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Aggregate-to-view and SQL-function-body-to-view dependencies remain incorrectly ordered despite being included in issue #580.

Review details

Suppressed comments (1)

internal/dump/formatter.go:121

• The baseline is not dependency-ordered for two view cases described in #580. generateCreateAggregatesSQL runs at internal/diff/diff.go:2009 before views at line 2060, and functionReferencesNewView only checks signatures, so an aggregate using a view row type—or an SQL function whose body queries a view—still appears before views/<view>.sql; this formatter preserves that invalid order. Please add these relation dependencies to the diff ordering and cover both replay cases.

	// Compute the include order. The diff package already emits statements in
	// dependency order, so the first appearance of each file is the baseline.

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[tianzhou]

Re the review note on aggregate-to-view and SQL-function-body-to-view ordering: confirmed, both are real, but they are diff-package ordering bugs rather than formatter bugs. I reproduced each in single-file dump, where they fail the same way (type v does not exist for an aggregate over a view row type, relation "v" does not exist for a SQL function body querying a view), so plan/apply against an empty database hit them too. The aggregate case is also emitted before its own transition function when that function takes the view row type. The multi-file formatter preserves whatever order the diff produces, so it inherits the fix once the diff is corrected.

These will be handled in a follow-up PR in internal/diff: scanning SQL-language function bodies for view references in functionReferencesNewView, and deferring aggregates whose signature or state type names a new view until after the view-dependent functions. This PR is scoped to the multi-file include ordering. I've updated the description so it no longer auto-closes #580; that issue stays open for the follow-up.