From 939190135c8df0a905778c58cc6b8a75c35a7c47 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Wed, 4 Dec 2024 11:23:57 +0000 Subject: [PATCH 1/2] Make changing column collation unrepresentable Ensure that a SQL statement of the form: ```sql ALTER TABLE foo ALTER COLUMN a SET DATA TYPE text COLLATE "en_US" ``` is converted to raw SQL instead of an `OpAlterColumn` operation. The change of collation is not currently represntable by an `OpAlterColumn` operation. --- pkg/sql2pgroll/alter_table.go | 14 ++++++++++++++ pkg/sql2pgroll/alter_table_test.go | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index c5b12b5b..f5989c76 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -78,6 +78,10 @@ func convertAlterTableAlterColumnType(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTa return nil, fmt.Errorf("expected column definition, got %T", cmd.GetDef().Node) } + if !canConvertColumnForSetDataType(node.ColumnDef) { + return nil, nil + } + return &migrations.OpAlterColumn{ Table: stmt.GetRelation().GetRelname(), Column: cmd.GetName(), @@ -170,6 +174,16 @@ func canConvertUniqueConstraint(constraint *pgq.Constraint) bool { return true } +// canConvertColumnForSetDataType checks if `column` can be faithfully +// converted as part of an OpAlterColumn operation to set a new type for the +// column. +func canConvertColumnForSetDataType(column *pgq.ColumnDef) bool { + if column.GetCollClause() != nil { + return false + } + return true +} + func ptr[T any](x T) *T { return &x } diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 666661db..028e1f8b 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -67,6 +67,10 @@ func TestUnconvertableAlterTableAddConstraintStatements(t *testing.T) { "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a) INCLUDE (b)", "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a) WITH (fillfactor=70)", "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a) USING INDEX TABLESPACE baz", + + // Altering a column to change its collation is not representable by an + // `OpAlterColumn` operation + `ALTER TABLE foo ALTER COLUMN a SET DATA TYPE text COLLATE "en_US"`, } for _, sql := range tests { From 648b8f3400b7d159eb4f1780050c60144d476e57 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Wed, 4 Dec 2024 11:31:50 +0000 Subject: [PATCH 2/2] Make changing type `USING` unrepresentable Ensure that a SQL statement of the form: ```sql ALTER TABLE foo ALTER COLUMN a SET DATA TYPE text USING 'foo' ``` is converted to raw SQL instead of an `OpAlterColumn` operation. The `USING` clause is not currently represntable by an `OpAlterColumn` operation. --- pkg/sql2pgroll/alter_table.go | 3 +++ pkg/sql2pgroll/alter_table_test.go | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index f5989c76..6d155138 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -181,6 +181,9 @@ func canConvertColumnForSetDataType(column *pgq.ColumnDef) bool { if column.GetCollClause() != nil { return false } + if column.GetRawDefault() != nil { + return false + } return true } diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 028e1f8b..a0280b9c 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -68,9 +68,10 @@ func TestUnconvertableAlterTableAddConstraintStatements(t *testing.T) { "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a) WITH (fillfactor=70)", "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a) USING INDEX TABLESPACE baz", - // Altering a column to change its collation is not representable by an - // `OpAlterColumn` operation + // COLLATE and USING clauses are not representable by `OpAlterColumn` + // operations when changing data type. `ALTER TABLE foo ALTER COLUMN a SET DATA TYPE text COLLATE "en_US"`, + "ALTER TABLE foo ALTER COLUMN a SET DATA TYPE text USING 'foo'", } for _, sql := range tests {