From 3972d471b696b98a0fcbae33f7eade47ad7d99c1 Mon Sep 17 00:00:00 2001 From: dilame Date: Tue, 26 May 2026 02:53:50 +0800 Subject: [PATCH] Respect table persistence drift Constraint: PostgreSQL stores LOGGED/UNLOGGED table persistence in pg_class.relpersistence.\nRejected: Recreate tables for persistence-only changes | ALTER TABLE SET LOGGED/UNLOGGED is the direct PostgreSQL operation and keeps the diff narrow.\nConfidence: high\nScope-risk: narrow\nDirective: Preserve table persistence in both schema extraction and create/alter DDL generation.\nTested: PATH="/Applications/Postgres.app/Contents/Versions/16/bin:/opt/local/bin:/opt/local/sbin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/usr/local/go/bin:/Users/bowzee/.volta/bin:/Users/bowzee/.codex/tmp/arg0/codex-arg0ugL248:/opt/homebrew/share/google-cloud-sdk/bin:/Users/bowzee/perl5/bin:/Users/bowzee/.antigravity/antigravity/bin:/opt/homebrew/opt/libpq/bin:/Users/bowzee/.local/bin:/opt/local/bin:/opt/local/sbin:/Users/bowzee/.cargo/bin:/Users/bowzee/Library/Application Support/JetBrains/Toolbox/scripts:/Applications/Codex.app/Contents/Resources" go test ./...\nNot-tested: Production migration run --- .../table_cases_test.go | 46 +++++++++++++++++++ internal/queries/queries.sql | 1 + internal/queries/queries.sql.go | 3 ++ internal/schema/schema.go | 2 + internal/schema/schema_test.go | 4 +- pkg/diff/sql_generator.go | 27 ++++++++++- 6 files changed, 80 insertions(+), 3 deletions(-) diff --git a/internal/migration_acceptance_tests/table_cases_test.go b/internal/migration_acceptance_tests/table_cases_test.go index d502ffe..b15a783 100644 --- a/internal/migration_acceptance_tests/table_cases_test.go +++ b/internal/migration_acceptance_tests/table_cases_test.go @@ -98,6 +98,52 @@ var tableAcceptanceTestCases = []acceptanceTestCase{ `, }, }, + { + name: "Set table unlogged", + oldSchemaDDL: []string{ + ` + CREATE TABLE foobar( + id INT PRIMARY KEY + ); + `, + }, + newSchemaDDL: []string{ + ` + CREATE UNLOGGED TABLE foobar( + id INT PRIMARY KEY + ); + `, + }, + expectedPlanDDL: []string{ + `ALTER TABLE "public"."foobar" SET UNLOGGED`, + }, + expectedHazardTypes: []diff.MigrationHazardType{ + diff.MigrationHazardTypeAcquiresAccessExclusiveLock, + }, + }, + { + name: "Set table logged", + oldSchemaDDL: []string{ + ` + CREATE UNLOGGED TABLE foobar( + id INT PRIMARY KEY + ); + `, + }, + newSchemaDDL: []string{ + ` + CREATE TABLE foobar( + id INT PRIMARY KEY + ); + `, + }, + expectedPlanDDL: []string{ + `ALTER TABLE "public"."foobar" SET LOGGED`, + }, + expectedHazardTypes: []diff.MigrationHazardType{ + diff.MigrationHazardTypeAcquiresAccessExclusiveLock, + }, + }, { name: "Create table with RLS enabled", oldSchemaDDL: nil, diff --git a/internal/queries/queries.sql b/internal/queries/queries.sql index d8397c8..191b559 100644 --- a/internal/queries/queries.sql +++ b/internal/queries/queries.sql @@ -20,6 +20,7 @@ SELECT c.oid, c.relname::TEXT AS table_name, table_namespace.nspname::TEXT AS table_schema_name, + c.relpersistence = 'u' AS is_unlogged, c.relreplident::TEXT AS replica_identity, c.relrowsecurity AS rls_enabled, c.relforcerowsecurity AS rls_forced, diff --git a/internal/queries/queries.sql.go b/internal/queries/queries.sql.go index 4315102..605dab3 100644 --- a/internal/queries/queries.sql.go +++ b/internal/queries/queries.sql.go @@ -1089,6 +1089,7 @@ SELECT c.oid, c.relname::TEXT AS table_name, table_namespace.nspname::TEXT AS table_schema_name, + c.relpersistence = 'u' AS is_unlogged, c.relreplident::TEXT AS replica_identity, c.relrowsecurity AS rls_enabled, c.relforcerowsecurity AS rls_forced, @@ -1136,6 +1137,7 @@ type GetTablesRow struct { Oid interface{} TableName string TableSchemaName string + IsUnlogged bool ReplicaIdentity string RlsEnabled bool RlsForced bool @@ -1158,6 +1160,7 @@ func (q *Queries) GetTables(ctx context.Context) ([]GetTablesRow, error) { &i.Oid, &i.TableName, &i.TableSchemaName, + &i.IsUnlogged, &i.ReplicaIdentity, &i.RlsEnabled, &i.RlsForced, diff --git a/internal/schema/schema.go b/internal/schema/schema.go index b3945d5..3d4710a 100644 --- a/internal/schema/schema.go +++ b/internal/schema/schema.go @@ -218,6 +218,7 @@ type Table struct { CheckConstraints []CheckConstraint Policies []Policy Privileges []TablePrivilege + IsUnlogged bool ReplicaIdentity ReplicaIdentity RLSEnabled bool RLSForced bool @@ -1056,6 +1057,7 @@ func (s *schemaFetcher) buildTable( CheckConstraints: checkConsByTable[schemaQualifiedName.GetFQEscapedName()], Policies: policiesByTable[schemaQualifiedName.GetFQEscapedName()], Privileges: privilegesByTable[schemaQualifiedName.GetFQEscapedName()], + IsUnlogged: table.IsUnlogged, ReplicaIdentity: ReplicaIdentity(table.ReplicaIdentity), RLSEnabled: table.RlsEnabled, RLSForced: table.RlsForced, diff --git a/internal/schema/schema_test.go b/internal/schema/schema_test.go index c73bb17..e82edd3 100644 --- a/internal/schema/schema_test.go +++ b/internal/schema/schema_test.go @@ -239,7 +239,7 @@ var ( GRANT SELECT ON schema_2.foo TO some_role_1; GRANT INSERT ON schema_2.foo TO some_role_2 WITH GRANT OPTION; `}, - expectedHash: "4c2174e2cac3956b", + expectedHash: "63bdda1e60bbc81", expectedSchema: Schema{ NamedSchemas: []NamedSchema{ {Name: "public"}, @@ -591,7 +591,7 @@ var ( ALTER TABLE foo_fk_1 ADD CONSTRAINT foo_fk_1_fk FOREIGN KEY (author, content) REFERENCES foo_1 (author, content) NOT VALID; `}, - expectedHash: "32c5a9c52dcfb15e", + expectedHash: "528eed3f5ac35bd1", expectedSchema: Schema{ NamedSchemas: []NamedSchema{ {Name: "public"}, diff --git a/pkg/diff/sql_generator.go b/pkg/diff/sql_generator.go index 6c31e22..6552c74 100644 --- a/pkg/diff/sql_generator.go +++ b/pkg/diff/sql_generator.go @@ -828,7 +828,12 @@ func (t *tableSQLVertexGenerator) Add(table schema.Table) ([]Statement, error) { columnDefs = append(columnDefs, "\t"+columnDef) } createTableSb := strings.Builder{} - createTableSb.WriteString(fmt.Sprintf("CREATE TABLE %s (\n%s\n)", + tableKind := "TABLE" + if table.IsUnlogged { + tableKind = "UNLOGGED TABLE" + } + createTableSb.WriteString(fmt.Sprintf("CREATE %s %s (\n%s\n)", + tableKind, table.GetFQEscapedName(), strings.Join(columnDefs, ",\n"), )) @@ -954,6 +959,10 @@ func (t *tableSQLVertexGenerator) Alter(diff tableDiff) ([]Statement, error) { stmts = append(stmts, alterBaseTableStmts...) } + if diff.old.IsUnlogged != diff.new.IsUnlogged { + stmts = append(stmts, alterTablePersistenceStatement(diff.new.SchemaQualifiedName, diff.new.IsUnlogged)) + } + if diff.old.ReplicaIdentity != diff.new.ReplicaIdentity { alterReplicaIdentityStmt, err := alterReplicaIdentityStatement(diff.new.SchemaQualifiedName, diff.new.ReplicaIdentity) if err != nil { @@ -1060,6 +1069,22 @@ func (t *tableSQLVertexGenerator) alterBaseTable(diff tableDiff) ([]Statement, e return stmts, nil } +func alterTablePersistenceStatement(table schema.SchemaQualifiedName, isUnlogged bool) Statement { + persistence := "LOGGED" + if isUnlogged { + persistence = "UNLOGGED" + } + return Statement{ + DDL: fmt.Sprintf("%s SET %s", alterTablePrefix(table), persistence), + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + Hazards: []MigrationHazard{{ + Type: MigrationHazardTypeAcquiresAccessExclusiveLock, + Message: "Changing table persistence requires an ACCESS EXCLUSIVE lock and rewrites the table", + }}, + } +} + func (t *tableSQLVertexGenerator) alterPartition(diff tableDiff) ([]Statement, error) { if diff.old.ForValues != diff.new.ForValues { return nil, fmt.Errorf("altering partition FOR VALUES: %w", ErrNotImplemented)