Skip to content

feat: support range types (CREATE TYPE ... AS RANGE) - #684

Merged
tianzhou merged 8 commits into
mainfrom
feat/range-types
Nov 6, 2026
Merged

tianzhou merged 8 commits into
mainfrom
feat/range-types

Conversation

@tianzhou

@tianzhou tianzhou commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Phases 2 and 3 of the range type design, following the Phase 1 fixes in #683. A user-defined range type is now inspected, dumped, created, dropped, compared and replaced.

  • Inspector: new GetRangeTypesForSchema query over pg_range (subtype, operator class, collation, canonical, subtype_diff, multirange). Opclass and collation are captured only when they differ from the subtype's defaults, as pg_dump prints them. Type privilege queries include typtype = 'r', so GRANT USAGE ON TYPE on a range round-trips.
  • Rendering: CREATE TYPE ... AS RANGE (...) in pg_dump's option order. multirange_type_name is printed only when it is not the default, using a Go port of PostgreSQL's makeMultirangeTypeName.
  • Ordering: a range over a new enum or domain, and a composite attribute or domain over a range or its multirange, sort after what they need. A range whose subtype_diff is a new function is created after the functions phase, and tables using the range or its multirange wait for it.
  • Modified ranges: PostgreSQL has no ALTER TYPE for a range's definition, so a changed range is dropped and recreated as a single plan step (one "modify" in the plan).
  • Drop order fix: functions are dropped before types, so dropping a range (or a domain with a function CHECK) together with the function it references failed with a dependency error. Such functions now wait until after the types are dropped.

After review (216785b)

  • A replaced range is scheduled through the create phase's type buckets (after the drop phase, after a new subtype_diff function or subtype it needs), still as one plan step.
  • Deferral propagates to types built on a deferred range, to tables with array columns of it, and to functions whose signature uses it.
  • Types drop in reverse dependency order; a held-back subtype_diff function drops between the types that reference it and the types its signature uses, with its callees held too.
  • Inspector fix uncovered by that: GetFunctionDependencies keyed functions by identity arguments (with parameter names) while the Functions map is keyed by types, so any function with named parameters had no dependencies. It now uses oidvectortypes. No existing golden file changed.
  • Plan-time schema remapping covers SubtypeOpclass and Collation; the multirange-name port clips like the server.

After the second review (45c8f93)

  • Dropped types and held-back functions are ordered together by dependency, which also covers a range over a function-checked domain whose subtype_diff takes that domain.
  • A replaced range is dropped in the drop phase (inside that joint order, before an old subtype_diff or subtype that goes away) and created in the create phase. The plan reports one drop and one add of the type.
  • subtype_diff is matched by signature, so an overload taking the range itself is ordered as a dependent of the range.
  • subtype_opclass and collation in the target schema lose their prefix, like a column collation.

After the third review (9f205e1)

  • subtype_diff matching normalizes the subtype the way parameter types are normalized, so a timestamp (or varchar) range still finds its difference function.
  • A held-back function also drops before a type its SQL body or a parameter default names.

After the fourth and fifth reviews

  • Function-to-type dependencies come from the catalog: a new GetFunctionTypeDependencies query (pg_depend, pg_proc → pg_type) fills Function.TypeDependencies, which the joint drop order uses as certain edges. The textual body match was tried and removed: a literal or parameter named like a type makes a false edge, and two such hints can hide a real one. The field is kept out of the JSON fingerprint.

Not in scope

  • COMMENT ON TYPE is not generated for any type kind today (the IR captures the comment, no diff code emits it), so range comments are not round-tripped either. Noted in the design doc and the dump suite manifest; worth its own PR.
  • A range column removed from a retained table is dropped in the modify phase, after the replaced range's DROP TYPE, so that combination fails with PostgreSQL's dependency error.
  • A chain function → domain (CHECK) → function (taking the domain) → range (subtype_diff) creates the range before the second function; domains have the same limit, and it needs interleaved type/function scheduling in the create phase.
  • Grants on a replaced range are lost, as for a recreated function or view today (open follow-up from Function COMMENT is lost when a function is recreated via DROP + CREATE #677). The two review threads on it are left open as the pointer.

Test plan

  • go test ./internal/diff ./ir ./internal/...
  • go test ./cmd -run TestPlanAndApply (full suite, PostgreSQL 18)
  • create_type/, create_domain/, privilege/grant_type* applied for real after the drop-order fix
  • TestIncludeIntegration
  • PGSCHEMA_POSTGRES_VERSION=14 for create_type/ and the range dump suite
  • TestDumpCommand_Range on 18 and 14
  • full go test ./cmd/dump solo run (550s, all fixtures byte-identical)

🤖 Generated with Claude Code

Phases 2 and 3 of docs/plans/2026-11-05-range-types-design.md. A user-defined
range type is now inspected, dumped, created, dropped, compared and replaced.

Inspector: a new GetRangeTypesForSchema query joins pg_range to the subtype,
operator class, collation, canonical and subtype_diff functions, and the
multirange. Subtype follows the domain base-type convention; function
references follow the aggregate convention (bare in the type's own schema);
opclass and collation are empty when they are the subtype's defaults, as
pg_dump prints them. The type privilege queries include typtype 'r'.

Diff: CREATE TYPE ... AS RANGE in pg_dump's option order, printing
multirange_type_name only when it differs from a port of PostgreSQL's
makeMultirangeTypeName. A range over a new enum or domain, and a composite
or domain over a range or its multirange, sort after what they need. A range
whose subtype_diff is a new function is created after the functions phase,
and tables using the range or its multirange wait for it. A changed range
has no ALTER TYPE, so it is dropped and recreated as one plan step.
Dropping a range (or a domain) together with a function it references
previously failed because functions are dropped before types; such
functions now wait until after the types.

Tests: nine create_type cases, privilege/grant_type_range, an include
fixture, unit tests for the renderer, equality, sort edges and the
multirange name, and the regenerated testdata/dump/range golden file.
add_range_type joins the cross-version smoke set. COMMENT ON TYPE is not
generated for any type kind yet, so range comments are not round-tripped.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 6, 2026 03:05
@greptile-apps

greptile-apps Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 0/5

[Medium impact] The PR extends schema dumps and migration plans for range types.

Do not merge yet: valid range migrations can fail, and successful replacements can lose grants.

Findings

  1. P1 Range replacement runs too early ▶
  2. P1 Range grants disappear ▶
  3. P1 Types precede their range ▶
  4. P1 Functions precede their range ▶
  5. P1 Range tables start too early ▶
  6. P1 Temporary schema names escape ▶
  7. P1 Helpers drop before their callers ▶

Summary

Adds inspection, SQL rendering, comparison, and replacement for user-defined range types.

  • Schema dumps now include user-defined range definitions.
  • Migration plans put range types between their prerequisites and dependents.
  • Plans now include grants on range types.

Acknowledged limits:

  • tianzhou explicitly deferred COMMENT ON TYPE because no type kind currently emits it.
  • Canonical function creation is explicitly documented as outside the supported creation path.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Inspect current and desired schemas] --> B[Compare range definitions]
  B --> C[Replace changed ranges]
  C --> D[Drop removed objects]
  D --> E[Create early types]
  E --> F[Create functions]
  F --> G[Create deferred ranges]
  G --> H[Create dependent tables]
  H --> I[Apply changed grants]
Loading

Reviews (1) · Last reviewed commit: "feat: support range types (CREATE TYPE ...." · Reviewed by Greptile

Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/type.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread cmd/plan/plan.go Resolved
Comment thread internal/diff/diff.go Resolved

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.

🟡 Changes recommended

Critical dependency-ordering defects can cause schema changes to fail, while range handling can also lose privileges or restore incorrect multirange names.

9 open issues
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file
What changed in this PR

Adds user-defined range types to pgschema’s inspect, dump, plan, and apply workflow, building on the constructor-filtering and schema-qualification fixes in #683.

Changes:

  • Captures range definitions and privileges, renders range DDL, and detects definition changes for replacement.
  • Adds dependency ordering for range subtypes, supporting functions, and range/multirange consumers.
  • Expands migration fixtures, dump and include coverage, and documentation; type comments remain out of scope.
File Description
testutil/​skip_list.go Adds range creation to essential tests.
testdata/​include/​types/​price_range.sql Adds an includable range definition.
testdata/​include/​expected_full_schema.sql Updates the assembled schema expectation.
testdata/​dump/​range/​pgschema.sql Expects range DDL and dependency ordering.
testdata/​dump/​range/​manifest.json Documents dump behavior and limitations.
testdata/​diff/​privilege/​grant_type_range/​plan.txt Expects the range privilege summary.
testdata/​diff/​privilege/​grant_type_range/​plan.sql Expects range USAGE grant SQL.
testdata/​diff/​privilege/​grant_type_range/​plan.json Expects the structured grant plan.
testdata/​diff/​privilege/​grant_type_range/​old.sql Defines the privilege test baseline.
testdata/​diff/​privilege/​grant_type_range/​new.sql Adds the desired range grant.
testdata/​diff/​privilege/​grant_type_range/​diff.sql Expects the grant migration.
testdata/​diff/​create_type/​range_column_unchanged/​plan.txt Expects no detected changes.
testdata/​diff/​create_type/​range_column_unchanged/​plan.sql Expects no executable steps.
testdata/​diff/​create_type/​range_column_unchanged/​plan.json Expects an empty structured plan.
testdata/​diff/​create_type/​range_column_unchanged/​old.sql Defines existing range-backed columns.
testdata/​diff/​create_type/​range_column_unchanged/​new.sql Preserves the same range-backed schema.
testdata/​diff/​create_type/​range_column_unchanged/​diff.sql Expects an empty migration.
testdata/​diff/​create_type/​drop_range_with_subtype_diff/​plan.txt Expects the dependent-object drop summary.
testdata/​diff/​create_type/​drop_range_with_subtype_diff/​plan.sql Expects table, range, then function drops.
testdata/​diff/​create_type/​drop_range_with_subtype_diff/​plan.json Records ordered drop steps.
testdata/​diff/​create_type/​drop_range_with_subtype_diff/​old.sql Defines a range with a supporting function.
testdata/​diff/​create_type/​drop_range_with_subtype_diff/​new.sql Describes removal of all dependent objects.
testdata/​diff/​create_type/​drop_range_with_subtype_diff/​diff.sql Expects dependency-ordered drop SQL.
testdata/​diff/​create_type/​drop_range_type/​plan.txt Expects the range-removal summary.
testdata/​diff/​create_type/​drop_range_type/​plan.sql Expects table-before-range removal.
testdata/​diff/​create_type/​drop_range_type/​plan.json Records range-removal steps.
testdata/​diff/​create_type/​drop_range_type/​old.sql Defines range and multirange columns.
testdata/​diff/​create_type/​drop_range_type/​new.sql Describes removal of the table and range.
testdata/​diff/​create_type/​drop_range_type/​diff.sql Expects table and range drops.
testdata/​diff/​create_type/​alter_range_subtype/​plan.txt Expects one modified range.
testdata/​diff/​create_type/​alter_range_subtype/​plan.sql Expects range replacement SQL.
testdata/​diff/​create_type/​alter_range_subtype/​plan.json Records replacement steps.
testdata/​diff/​create_type/​alter_range_subtype/​old.sql Defines the original integer subtype.
testdata/​diff/​create_type/​alter_range_subtype/​new.sql Changes the subtype to bigint.
testdata/​diff/​create_type/​alter_range_subtype/​diff.sql Expects DROP and CREATE statements.
testdata/​diff/​create_type/​add_range_type/​plan.txt Expects the range-and-table creation summary.
testdata/​diff/​create_type/​add_range_type/​plan.sql Expects range-before-table creation.
testdata/​diff/​create_type/​add_range_type/​plan.json Records range and table creation steps.
testdata/​diff/​create_type/​add_range_type/​old.sql Establishes an empty baseline.
testdata/​diff/​create_type/​add_range_type/​new.sql Adds range columns and an exclusion constraint.
testdata/​diff/​create_type/​add_range_type/​diff.sql Expects range and table DDL.
testdata/​diff/​create_type/​add_range_subtype_diff/​plan.txt Expects the function-backed range summary.
testdata/​diff/​create_type/​add_range_subtype_diff/​plan.sql Expects function, range, then table creation.
testdata/​diff/​create_type/​add_range_subtype_diff/​plan.json Records dependency-ordered creation steps.
testdata/​diff/​create_type/​add_range_subtype_diff/​old.sql Establishes an empty baseline.
testdata/​diff/​create_type/​add_range_subtype_diff/​new.sql Adds a subtype-difference function and consumers.
testdata/​diff/​create_type/​add_range_subtype_diff/​diff.sql Expects function-backed range migration SQL.
testdata/​diff/​create_type/​add_range_options/​plan.txt Expects the range-options summary.
testdata/​diff/​create_type/​add_range_options/​plan.sql Expects optional range clauses.
testdata/​diff/​create_type/​add_range_options/​plan.json Records range-option creation steps.
testdata/​diff/​create_type/​add_range_options/​old.sql Establishes an empty baseline.
testdata/​diff/​create_type/​add_range_options/​new.sql Exercises naming, collation, and operator classes.
testdata/​diff/​create_type/​add_range_options/​diff.sql Expects canonical range-option rendering.
testdata/​diff/​create_type/​add_range_in_composite_and_domain/​plan.txt Expects the dependent-type creation summary.
testdata/​diff/​create_type/​add_range_in_composite_and_domain/​plan.sql Expects the range before dependent types.
testdata/​diff/​create_type/​add_range_in_composite_and_domain/​plan.json Records dependent-type creation order.
testdata/​diff/​create_type/​add_range_in_composite_and_domain/​old.sql Establishes an empty baseline.
testdata/​diff/​create_type/​add_range_in_composite_and_domain/​new.sql Adds composite and domain range consumers.
testdata/​diff/​create_type/​add_range_in_composite_and_domain/​diff.sql Expects dependency-ordered type DDL.
testdata/​diff/​create_type/​add_range_enum_subtype/​plan.txt Expects the enum-backed range summary.
testdata/​diff/​create_type/​add_range_enum_subtype/​plan.sql Expects enum-before-range creation.
testdata/​diff/​create_type/​add_range_enum_subtype/​plan.json Records enum and range creation steps.
testdata/​diff/​create_type/​add_range_enum_subtype/​old.sql Establishes an empty baseline.
testdata/​diff/​create_type/​add_range_enum_subtype/​new.sql Adds a range over a new enum.
testdata/​diff/​create_type/​add_range_enum_subtype/​diff.sql Expects enum-backed range DDL.
README.md Lists range type support.
ir/​queries/​queries.sql.go Adds generated range queries and privilege filters.
ir/​queries/​queries.sql Queries range metadata and includes range privileges.
ir/​ir.go Adds range kind and definition fields.
ir/​inspector.go Populates range objects from catalog results.
internal/​diff/​type.go Renders, compares, and replaces range types.
internal/​diff/​type_test.go Tests rendering, naming, equality, and ordering.
internal/​diff/​topological.go Adds subtype and multirange dependency edges.
internal/​diff/​diff.go Schedules range creation, replacement, and supporting-function drops.
docs/​syntax/​create_type.mdx Documents range syntax and replacement behavior.
docs/​plans/​2026-11-05-range-types-design.md Records implementation progress and deviations.
docs/​coding-agent.mdx Updates the supported type list.
docs/​blog/​supabase-declarative-schema-vs-pgschema.mdx Updates the range-support comparison.
cmd/​plan/​plan.go Normalizes range schema references during planning.
CLAUDE.md Updates the documented type capabilities.

Review effort: Balanced

Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/topological.go Resolved
Comment thread internal/diff/type.go Resolved
Comment thread internal/diff/type.go Resolved
…ents of a deferred range

Review of #684 (greptile, Copilot) found ordering gaps around range types:

- A replaced range was emitted before the drop phase, so a table using the
  old definition that is dropped in the same plan blocked DROP TYPE RESTRICT,
  and a new subtype_diff function did not exist yet for the CREATE. The
  replacement (still one DROP + CREATE step) now goes through the create
  phase's type buckets like a new range would.
- Types built on a deferred range (a composite attribute, a domain over the
  multirange, a range over a deferred domain), tables with array columns of
  it, and functions whose signature uses it were created before the range.
  Deferral now propagates through the type graph, deferred-type matching
  strips array and quote syntax, and signature-dependent functions move to
  the batch after the type.
- Types were dropped alphabetically. They now drop in reverse dependency
  order, and a held-back subtype_diff function drops between the types that
  reference it and the types its signature uses, with its callees held too.
  That exposed an inspector bug: GetFunctionDependencies keyed functions by
  identity arguments (with parameter names) while the Functions map is keyed
  by types, so named-parameter functions never had dependencies. It now uses
  oidvectortypes with the same same-schema stripping.
- Plan-time schema remapping covers SubtypeOpclass and Collation.
- defaultMultirangeTypeName clips the range name to NAMEDATALEN-12 bytes
  before appending _multirange, as the server does.

Grants on a replaced range are lost, as for recreated functions and views
(open follow-up from #677); not addressed here.

Co-Authored-By: Claude Fable 5.1 <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.

🟡 Changes recommended

Unresolved dependency-ordering defects can break range creation, replacement, and deletion, while overload matching and schema qualification can resolve references incorrectly.

3 open issues
2 previously missed issues

In code that hasn't changed since last review

Medium severity Match subtype-difference functions by identity when ordering drops

internal/​diff/​diff.go:3934

For a range r using diff(integer, integer), this name-only lookup also holds an unrelated overload diff(r) until after the range. Dropping all three objects then fails at DROP TYPE r RESTRICT because diff(r) still depends on it. Match the actual subtype-difference function identity, including argument types, so the consuming overload drops before the range.

Medium severity Apply schema-prefix stripping to opclass and collation references

internal/​diff/​type.go:323

Same-schema opclass and collation references bypass smart schema qualification here. The inspector always qualifies a custom collation, so a default dump of schema tenant emits an unqualified range but retains collation = tenant.my_coll. Reusing that dump in another schema either binds the original schema's collation or fails, unlike column collations, which strip the target prefix. Pass both options through stripSchemaPrefixMode, preserving genuinely cross-schema references and --qualify-schema behavior.

9 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/type.go Resolved
…nt across phases

Second review round on #684:

- Dropped types and the functions they hold back are now ordered together
  by dependency (orderTypeAndFunctionDrops): a type before the type it is
  built on, a type before the function it names, a function before the
  type its signature uses, a caller before its callee. This covers a range
  over a function-checked domain whose subtype_diff takes that domain
  (drop_range_over_checked_domain).
- A replaced range is no longer one DROP + CREATE step in the create
  phase. The old definition is dropped in the drop phase inside that joint
  order, so it precedes an old subtype_diff or subtype that goes away too,
  and the new one is created in the create phase through the usual buckets.
  The plan reports one drop and one add of the type, which is what runs
  (alter_range_drop_old_subtype_diff).
- A range's subtype_diff is matched by signature, not just name, so an
  overload taking the range itself is ordered as a dependent of the range
  (drop_range_subtype_diff_overload).
- subtype_opclass and collation in the target schema lose their prefix
  like a column collation, so a dump applies to another schema.

A chain of function -> domain (CHECK) -> function (taking the domain) ->
range (subtype_diff) still creates the range before the second function;
domains have the same limit, and it needs interleaved scheduling of types
and functions in the create phase. Recorded in the design doc.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@tianzhou

tianzhou commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Two notes from the second Copilot review had no thread; both are fixed in 45c8f93:

  • Match subtype_diff by identity: typeReferencesNewFunction now matches name(subtype, subtype) against signature keys in the function lookup, so an overload taking the range itself is treated as a dependent of the range and dropped before it (drop_range_subtype_diff_overload).
  • Strip the target-schema prefix from opclass and collation: both go through stripSchemaPrefixMode like a column collation, so a dump of schema tenant applies to another schema; pg_catalog and cross-schema references keep their prefix.

🤖 Addressed by Claude Code

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.

🟡 Changes recommended

Critical dependency-ordering and range-type handling defects can produce invalid dumps, failed applies, and silently omitted type changes.

2 open issues
4 previously missed issues

In code that hasn't changed since last review

High severity Hoisting ignores type dependencies in multi-file dumps

internal/​diff/​diff.go:2297

With an unrelated table plus time_diff, timerange, and shift_length(timerange), multi-file formatting hoists shift_length ahead of the range because it checks only table/view dependencies (internal/dump/formatter.go:178–183). Loading the dump then fails because the parameter type does not exist. Make hoisting preserve prerequisite type files and add a multi-file replay case with an unrelated early table.

High severity Domain CHECK overloads can create incorrect drop ordering

internal/​diff/​diff.go:3953

If domain positive checks retained is_positive(integer), dropping the domain and overload is_positive(positive) incorrectly holds the overload because domain matching uses only function names. The joint graph then contains edges in both directions, and its cycle fallback drops the domain first, failing because the overload still uses it. Use catalog-resolved CHECK/default function identities for holdback and joint drop edges (internal/diff/diff.go:4028–4029).

High severity Range-to-enum changes are silently omitted

internal/​diff/​type.go:386

Changing an otherwise unused span from a range to an enum, or vice versa, is silently ignored. Type comparison records a modification, but this filter excludes it and both modify generators skip changes between different kinds, so the resulting plan reports no changes. Include modifications involving a range on either side in the replacement path.

High severity Array-based domains are misidentified as array subtypes

ir/​queries/​queries.sql:2164

This branch also matches domains over arrays: PostgreSQL copies the base type's typcategory = 'A' into the domain, but the domain's typelem is zero. For CREATE DOMAIN endpoints AS integer[] followed by a range with subtype = endpoints, the element joins are NULL, so this expression returns NULL and inspection records an empty subtype. Dumps then emit an invalid subtype = option. Restrict this branch to actual array types and render an array-based domain by its qualified domain name; regenerate the sqlc output too.

3 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
…d functions before types their bodies name

Third review round on #684:

- A range's subtype_diff is matched by signature against the function
  lookup. Parameter types are normalized (timestamp without time zone ->
  timestamp, character varying -> varchar) while the subtype keeps the
  format_type spelling, so the match now normalizes the subtype the same
  way (ir.NormalizeTypeName). Without it a timestamp range was created
  before its new difference function (add_range_subtype_diff_timestamp).
- In the joint drop order a held-back function also precedes a type that
  its SQL body or a parameter default names, since PostgreSQL records that
  dependency for SQL-standard bodies and defaults
  (drop_range_subtype_diff_body_type).

Co-Authored-By: Claude Fable 5.1 <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.

🟡 Changes recommended

The dependency matcher creates a spurious function-to-range dependency, causing an incorrect drop order that makes DROP TYPE ... RESTRICT fail.

1 open issue
2 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

Comment thread internal/diff/diff.go Resolved
Fourth review round on #684: matching a held-back function's body text
against dropped type names can invent a function -> type edge (a parameter
or literal named like the type). Against the certain type -> function edge
of a range naming its subtype_diff that made a cycle, and the fallback
could drop the enum before the function.

Certain edges are added first (type structure, a type naming a function,
signatures, recorded function dependencies); a textual edge is added only
when it does not contradict them, so it can over-order a drop but never
reverse one (drop_range_subtype_diff_body_names_range).

Co-Authored-By: Claude Fable 5.1 <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.

🟡 Changes recommended

A false textual dependency hint can cause a real type dependency to be rejected as cyclic, leading to DROP TYPE executing while a function still depends on it and failing.

1 open issue
1 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

Comment thread internal/diff/diff.go Resolved
A function's body or parameter default naming a type was a textual hint,
added to the drop order only when it did not contradict an edge added
before it. That check also ran against earlier hints, so a false match
(a literal naming another range) sorted first could veto a true,
catalog-recorded dependency (a SQL-standard body casting to a range), and
DROP TYPE ran while the function still depended on it.

Read pg_depend from pg_proc to pg_type (signature, SQL-standard body,
parameter defaults; arrays as their element) into Function.TypeDependencies
with a new GetFunctionTypeDependencies query, and use it as a certain edge
in orderTypeAndFunctionDrops. The textual match is removed: for drops,
only a recorded dependency can make DROP ... RESTRICT fail, and the catalog
is the source of those. drop_two_ranges_cross_body covers the veto case.

Co-Authored-By: Claude Fable 5.1 <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.

🔵 Needs a closer look

Multiple critical gaps in type-transition handling and dependency ordering require human assessment because they can omit migrations or produce SQL that fails to execute.

No open issues 🎉

1 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

tianzhou and others added 2 commits November 5, 2026 22:29
…TypeDependencies from JSON

The plan.json was generated while Function.TypeDependencies was still
serialized; it is ordering data and now carries json:"-" so existing
fingerprints do not shift, which also changes this new case's hash.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Each diff case carries its own fixture, plan and apply run, so the twenty
range cases are now one transition that drops, replaces, adds and keeps
ranges at once, with the privilege grant folded in. Its diff.sql is the
specification for the whole drop order, the create order and the no-op
guard; the drop-order algorithm keeps its unit test in type_test.go.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@tianzhou
tianzhou merged commit 1c54bf4 into main Nov 6, 2026
1 check passed
@tianzhou
tianzhou deleted the feat/range-types branch November 8, 2026 06:28
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.

2 participants