From 4527ae567c92ac3939d8a6909dfec77d250b9732 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 10 Dec 2024 09:52:12 +0100 Subject: [PATCH 1/6] Format imports --- pkg/sql2pgroll/alter_table.go | 1 + pkg/sql2pgroll/alter_table_test.go | 1 + pkg/sql2pgroll/convert.go | 1 + pkg/sql2pgroll/create_table.go | 1 + pkg/sql2pgroll/create_table_test.go | 1 + pkg/sql2pgroll/rename.go | 1 + pkg/sql2pgroll/rename_test.go | 1 + 7 files changed, 7 insertions(+) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 6d155138..eea25fe9 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -6,6 +6,7 @@ import ( "fmt" pgq "github.com/pganalyze/pg_query_go/v6" + "github.com/xataio/pgroll/pkg/migrations" ) diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index a6c89cd7..1aaab2ca 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/xataio/pgroll/pkg/migrations" "github.com/xataio/pgroll/pkg/sql2pgroll" "github.com/xataio/pgroll/pkg/sql2pgroll/expect" diff --git a/pkg/sql2pgroll/convert.go b/pkg/sql2pgroll/convert.go index 663e5b0c..40ca0264 100644 --- a/pkg/sql2pgroll/convert.go +++ b/pkg/sql2pgroll/convert.go @@ -6,6 +6,7 @@ import ( "fmt" pgq "github.com/pganalyze/pg_query_go/v6" + "github.com/xataio/pgroll/pkg/migrations" ) diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 089b8d4a..5724465a 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -4,6 +4,7 @@ package sql2pgroll import ( pgq "github.com/pganalyze/pg_query_go/v6" + "github.com/xataio/pgroll/pkg/migrations" ) diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 5636f489..acc980b0 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/xataio/pgroll/pkg/migrations" "github.com/xataio/pgroll/pkg/sql2pgroll" "github.com/xataio/pgroll/pkg/sql2pgroll/expect" diff --git a/pkg/sql2pgroll/rename.go b/pkg/sql2pgroll/rename.go index 2b47c792..6fbd596a 100644 --- a/pkg/sql2pgroll/rename.go +++ b/pkg/sql2pgroll/rename.go @@ -4,6 +4,7 @@ package sql2pgroll import ( pgq "github.com/pganalyze/pg_query_go/v6" + "github.com/xataio/pgroll/pkg/migrations" ) diff --git a/pkg/sql2pgroll/rename_test.go b/pkg/sql2pgroll/rename_test.go index e3888922..bcfbb18f 100644 --- a/pkg/sql2pgroll/rename_test.go +++ b/pkg/sql2pgroll/rename_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/xataio/pgroll/pkg/migrations" "github.com/xataio/pgroll/pkg/sql2pgroll" "github.com/xataio/pgroll/pkg/sql2pgroll/expect" From 08b744619da0c7a221a6c6a69593e574d32a1f06 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 10 Dec 2024 11:32:15 +0100 Subject: [PATCH 2/6] Handle simple DROP COLUMN statements --- pkg/sql2pgroll/alter_table.go | 15 +++++++++++++++ pkg/sql2pgroll/alter_table_test.go | 4 ++++ pkg/sql2pgroll/expect/drop_column.go | 12 ++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 pkg/sql2pgroll/expect/drop_column.go diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index eea25fe9..00d99637 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -36,6 +36,8 @@ func convertAlterTableStmt(stmt *pgq.AlterTableStmt) (migrations.Operations, err op, err = convertAlterTableAlterColumnType(stmt, alterTableCmd) case pgq.AlterTableType_AT_AddConstraint: op, err = convertAlterTableAddConstraint(stmt, alterTableCmd) + case pgq.AlterTableType_AT_DropColumn: + op, err = convertAlterTableDropColumn(stmt, alterTableCmd) } if err != nil { @@ -156,6 +158,19 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * }, nil } +// convertAlterTableDropColumn converts SQL statements like: +// +// `ALTER TABLE foo DROP COLUMN bar +// +// to an OpDropColumn operation. +func convertAlterTableDropColumn(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { + return &migrations.OpDropColumn{ + Table: stmt.GetRelation().GetRelname(), + Column: cmd.GetName(), + Down: PlaceHolderSQL, + }, nil +} + // canConvertUniqueConstraint checks if the unique constraint `constraint` can // be faithfully converted to an OpCreateConstraint operation without losing // information. diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 1aaab2ca..30aba44f 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -44,6 +44,10 @@ func TestConvertAlterTableStatements(t *testing.T) { sql: "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a, b)", expectedOp: expect.CreateConstraintOp2, }, + { + sql: "ALTER TABLE foo DROP COLUMN bar", + expectedOp: expect.DropColumnOp1, + }, } for _, tc := range tests { diff --git a/pkg/sql2pgroll/expect/drop_column.go b/pkg/sql2pgroll/expect/drop_column.go new file mode 100644 index 00000000..7d269e3a --- /dev/null +++ b/pkg/sql2pgroll/expect/drop_column.go @@ -0,0 +1,12 @@ +package expect + +import ( + "github.com/xataio/pgroll/pkg/migrations" + "github.com/xataio/pgroll/pkg/sql2pgroll" +) + +var DropColumnOp1 = &migrations.OpDropColumn{ + Table: "foo", + Column: "bar", + Down: sql2pgroll.PlaceHolderSQL, +} From 6346c9c1bb02aae4bf3b1b09037bcc9aec228481 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 10 Dec 2024 12:29:16 +0100 Subject: [PATCH 3/6] Fall back to raw SQL for unrepresentable options --- pkg/sql2pgroll/alter_table.go | 13 +++++++++++++ pkg/sql2pgroll/alter_table_test.go | 8 ++++++++ 2 files changed, 21 insertions(+) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 00d99637..58dc9c8c 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -164,6 +164,19 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * // // to an OpDropColumn operation. func convertAlterTableDropColumn(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { + switch cmd.Behavior { + case pgq.DropBehavior_DROP_RESTRICT, pgq.DropBehavior_DROP_BEHAVIOR_UNDEFINED: + // Supported + case pgq.DropBehavior_DROP_CASCADE: + // Fall back to SQL + return nil, nil + } + + // IF EXISTS not supported + if cmd.MissingOk { + return nil, nil + } + return &migrations.OpDropColumn{ Table: stmt.GetRelation().GetRelname(), Column: cmd.GetName(), diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 30aba44f..415dbab3 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -48,6 +48,10 @@ func TestConvertAlterTableStatements(t *testing.T) { sql: "ALTER TABLE foo DROP COLUMN bar", expectedOp: expect.DropColumnOp1, }, + { + sql: "ALTER TABLE foo DROP COLUMN bar RESTRICT ", + expectedOp: expect.DropColumnOp1, + }, } for _, tc := range tests { @@ -77,6 +81,10 @@ func TestUnconvertableAlterTableAddConstraintStatements(t *testing.T) { // 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'", + + // CASCADE and IF EXISTS clauses are not represented by OpDropColumn + "ALTER TABLE foo DROP COLUMN bar CASCADE", + "ALTER TABLE foo DROP COLUMN IF EXISTS bar", } for _, sql := range tests { From 42f101088f02bd20101361abf0ae1c7d03f1e5ed Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 10 Dec 2024 12:47:53 +0100 Subject: [PATCH 4/6] Rename test --- pkg/sql2pgroll/alter_table_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 415dbab3..144b220b 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -66,7 +66,7 @@ func TestConvertAlterTableStatements(t *testing.T) { } } -func TestUnconvertableAlterTableAddConstraintStatements(t *testing.T) { +func TestUnconvertableAlterTableStatements(t *testing.T) { t.Parallel() tests := []string{ From 1b63f22d224056a6cff21640dec63c71650d1309 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 10 Dec 2024 12:51:37 +0100 Subject: [PATCH 5/6] Break our check conversion code --- pkg/sql2pgroll/alter_table.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 58dc9c8c..915429ea 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -164,16 +164,7 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * // // to an OpDropColumn operation. func convertAlterTableDropColumn(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { - switch cmd.Behavior { - case pgq.DropBehavior_DROP_RESTRICT, pgq.DropBehavior_DROP_BEHAVIOR_UNDEFINED: - // Supported - case pgq.DropBehavior_DROP_CASCADE: - // Fall back to SQL - return nil, nil - } - - // IF EXISTS not supported - if cmd.MissingOk { + if !canConvertDropColumn(cmd) { return nil, nil } @@ -184,6 +175,17 @@ func convertAlterTableDropColumn(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCm }, nil } +// canConvertDropColumn checks whether we can convert the command without losing any information. +func canConvertDropColumn(cmd *pgq.AlterTableCmd) bool { + if cmd.MissingOk { + return false + } + if cmd.Behavior == pgq.DropBehavior_DROP_CASCADE { + return false + } + return true +} + // canConvertUniqueConstraint checks if the unique constraint `constraint` can // be faithfully converted to an OpCreateConstraint operation without losing // information. From f4a5ba28776eeec6123e5f5d5538b349a43270a3 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 10 Dec 2024 14:00:04 +0100 Subject: [PATCH 6/6] Add license header --- pkg/sql2pgroll/expect/drop_column.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/sql2pgroll/expect/drop_column.go b/pkg/sql2pgroll/expect/drop_column.go index 7d269e3a..3d57c842 100644 --- a/pkg/sql2pgroll/expect/drop_column.go +++ b/pkg/sql2pgroll/expect/drop_column.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 + package expect import (