Revert "fix: extension qualifier (#146)" #147
Conversation
This reverts commit e99d0e5.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the type qualification logic for custom types in PostgreSQL schema diffs. The main change simplifies type qualification rules by using a schema-matching approach instead of extension detection.
- Removes extension-based type detection logic and the
pg_dependjoin - Simplifies type qualification to only check if the type's schema matches the table's schema
- Updates test fixtures to use composite types and enums instead of extension types and domains
- Removes the
addedTypeNamestracking mechanism from table modification logic
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ir/queries/queries.sql | Simplifies the type qualification logic in SQL queries to use schema matching instead of extension detection |
| ir/queries/queries.sql.go | Generated Go code reflecting the simplified queries |
| internal/diff/diff.go | Removes addedTypeNames map tracking for type qualification |
| internal/diff/table.go | Removes conditional type qualification logic based on addedTypeNames |
| testdata/diff/create_table/add_column_custom_type/*.sql | Updates test fixtures to use composite and enum types instead of extension types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WHEN dt.typtype = 'b' THEN | ||
| -- Base types: qualify if not in pg_catalog or table's schema | ||
| CASE | ||
| WHEN dn.nspname = 'pg_catalog' THEN c.udt_name | ||
| WHEN dep.objid IS NOT NULL THEN dt.typname -- Extension type: unqualified | ||
| WHEN dn.nspname LIKE 'pgschema_tmp_%' THEN 'public.' || dt.typname -- Temp schema: re-qualify to public | ||
| WHEN dn.nspname = c.table_schema THEN dt.typname | ||
| ELSE dn.nspname || '.' || dt.typname | ||
| END |
There was a problem hiding this comment.
The base type qualification logic (typtype = 'b') is missing from the generated code in queries.sql.go. The generated file still uses the old single CASE statement for domains, enums, and composite types but doesn't include the base type handling. This inconsistency could lead to incorrect type qualification for base types.
| -- - Enums (status_enum) should be schema-qualified | ||
| -- New state: Add columns using custom types from setup.sql | ||
| -- Types (email_address, user_status) are created in setup.sql | ||
| -- Use unqualified names - types will be resolved via setup.sql context |
There was a problem hiding this comment.
The comment on lines 1-3 is outdated and contradicts the actual behavior. The comment states 'Use unqualified names - types will be resolved via setup.sql context', but with the new logic, types are qualified based on whether their schema matches the table's schema. Since both types are in the public schema (same as the table), they remain unqualified, but this is due to schema matching, not setup.sql context resolution.
| -- Use unqualified names - types will be resolved via setup.sql context | |
| -- Unqualified names are used because the types' schema matches the table's schema (public) |
| -- Extension types (citext) should be unqualified (search_path resolution) | ||
| -- Custom domains and enums should be schema-qualified (public.*) | ||
| -- Setup: Create types in public schema | ||
| -- This simulates extension types like citext installed in public schema |
There was a problem hiding this comment.
The comment 'This simulates extension types like citext installed in public schema' is misleading. The types being created (composite type and enum) are not extension types and don't behave like extension types. This comment should be updated to reflect that these are regular user-defined types used to test schema-based qualification logic.
| -- This simulates extension types like citext installed in public schema | |
| -- These are regular user-defined types (composite and enum) created in the public schema to test schema-based qualification logic. |
1 participant
This reverts commit e99d0e5.
Needs to find a better way to #144