[Snowflake] DDL clean up (#1059)

artie-labs · Nov 20, 2024 · 0c161d3 · 0c161d3
1 parent 01338f7
commit 0c161d3
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 30 deletions.
diff --git a/clients/snowflake/dialect/ddl.go b/clients/snowflake/dialect/ddl.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/artie-labs/transfer/lib/config/constants"
 	"github.com/artie-labs/transfer/lib/sql"
 )
 
@@ -21,18 +20,14 @@ func (SnowflakeDialect) BuildCreateTableQuery(tableID sql.TableIdentifier, tempo
 	}
 }
 
-func (sd SnowflakeDialect) BuildAddColumnQuery(tableID sql.TableIdentifier, sqlPart string) string {
-	return sd.buildAlterColumnQuery(tableID, constants.Add, sqlPart)
+func (SnowflakeDialect) BuildAddColumnQuery(tableID sql.TableIdentifier, sqlPart string) string {
+	return fmt.Sprintf("ALTER TABLE %s ADD COLUMN IF NOT EXISTS %s", tableID.FullyQualifiedName(), sqlPart)
 }
 
-func (sd SnowflakeDialect) BuildDropColumnQuery(tableID sql.TableIdentifier, colName string) string {
-	return sd.buildAlterColumnQuery(tableID, constants.Delete, colName)
+func (SnowflakeDialect) BuildDropColumnQuery(tableID sql.TableIdentifier, colName string) string {
+	return fmt.Sprintf("ALTER TABLE %s DROP COLUMN IF EXISTS %s", tableID.FullyQualifiedName(), colName)
 }
 
 func (SnowflakeDialect) BuildDescribeTableQuery(tableID sql.TableIdentifier) (string, []any, error) {
 	return fmt.Sprintf("DESC TABLE %s", tableID.FullyQualifiedName()), nil, nil
 }
-
-func (SnowflakeDialect) buildAlterColumnQuery(tableID sql.TableIdentifier, columnOp constants.ColumnOperation, colSQLPart string) string {
-	return fmt.Sprintf("ALTER TABLE %s %s COLUMN %s", tableID.FullyQualifiedName(), columnOp, colSQLPart)
-}
diff --git a/clients/snowflake/dialect/dialect.go b/clients/snowflake/dialect/dialect.go
@@ -12,17 +12,17 @@ import (
 
 type SnowflakeDialect struct{}
 
-func (sd SnowflakeDialect) QuoteIdentifier(identifier string) string {
+func (SnowflakeDialect) QuoteIdentifier(identifier string) string {
 	return fmt.Sprintf(`"%s"`, strings.ToUpper(identifier))
 }
 
 func (SnowflakeDialect) EscapeStruct(value string) string {
 	return sql.QuoteLiteral(value)
 }
 
-func (SnowflakeDialect) IsColumnAlreadyExistsErr(err error) bool {
-	// Snowflake doesn't have column mutations (IF NOT EXISTS)
-	return strings.Contains(err.Error(), "already exists")
+func (SnowflakeDialect) IsColumnAlreadyExistsErr(_ error) bool {
+	// We don't need this check as Snowflake DDLs are idempotent
+	return false
 }
 
 // IsTableDoesNotExistErr will check if the resulting error message looks like this

diff --git a/clients/snowflake/dialect/dialect_test.go b/clients/snowflake/dialect/dialect_test.go
@@ -18,17 +18,6 @@ func TestSnowflakeDialect_QuoteIdentifier(t *testing.T) {
 	assert.Equal(t, `"FOO"`, dialect.QuoteIdentifier("FOO"))
 }
 
-func TestSnowflakeDialect_IsColumnAlreadyExistsErr(t *testing.T) {
-	{
-		// Invalid error
-		assert.False(t, SnowflakeDialect{}.IsColumnAlreadyExistsErr(fmt.Errorf("hello there qux")))
-	}
-	{
-		// Valid
-		assert.True(t, SnowflakeDialect{}.IsColumnAlreadyExistsErr(fmt.Errorf("Column already exists")))
-	}
-}
-
 func TestSnowflakeDialect_IsTableDoesNotExistErr(t *testing.T) {
 	errToExpectation := map[error]bool{
 		nil: false,
@@ -62,7 +51,7 @@ func TestSnowflakeDialect_BuildAddColumnQuery(t *testing.T) {
 	fakeTableID.FullyQualifiedNameReturns("{TABLE}")
 
 	assert.Equal(t,
-		"ALTER TABLE {TABLE} add COLUMN {SQL_PART}",
+		"ALTER TABLE {TABLE} ADD COLUMN IF NOT EXISTS {SQL_PART}",
 		SnowflakeDialect{}.BuildAddColumnQuery(fakeTableID, "{SQL_PART}"),
 	)
 }
@@ -72,7 +61,7 @@ func TestSnowflakeDialect_BuildDropColumnQuery(t *testing.T) {
 	fakeTableID.FullyQualifiedNameReturns("{TABLE}")
 
 	assert.Equal(t,
-		"ALTER TABLE {TABLE} drop COLUMN {SQL_PART}",
+		"ALTER TABLE {TABLE} DROP COLUMN IF EXISTS {SQL_PART}",
 		SnowflakeDialect{}.BuildDropColumnQuery(fakeTableID, "{SQL_PART}"),
 	)
 }

diff --git a/lib/destination/ddl/ddl_sflk_test.go b/lib/destination/ddl/ddl_sflk_test.go
@@ -32,7 +32,7 @@ func (d *DDLTestSuite) TestAlterComplexObjects() {
 	assert.NoError(d.T(), shared.AlterTableAddColumns(context.Background(), d.snowflakeStagesStore, tc, config.SharedDestinationColumnSettings{}, tableID, cols))
 	for i := 0; i < len(cols); i++ {
 		_, execQuery, _ := d.fakeSnowflakeStagesStore.ExecContextArgsForCall(i)
-		assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s add COLUMN %s %s", `shop.public."COMPLEX_COLUMNS"`,
+		assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s ADD COLUMN IF NOT EXISTS %s %s", `shop.public."COMPLEX_COLUMNS"`,
 			d.snowflakeStagesStore.Dialect().QuoteIdentifier(cols[i].Name()),
 			d.snowflakeStagesStore.Dialect().DataTypeForKind(cols[i].KindDetails, false, config.SharedDestinationColumnSettings{})), execQuery)
 	}
@@ -134,9 +134,7 @@ func (d *DDLTestSuite) TestAlterTableDeleteDryRun() {
 		assert.Equal(d.T(), i+1, d.fakeSnowflakeStagesStore.ExecContextCallCount(), "tried to delete one column")
 
 		_, execArg, _ := d.fakeSnowflakeStagesStore.ExecContextArgsForCall(i)
-		assert.Equal(d.T(), execArg, fmt.Sprintf("ALTER TABLE %s %s COLUMN %s", `shop.public."USERS"`, constants.Delete,
-			d.snowflakeStagesStore.Dialect().QuoteIdentifier(cols[i].Name()),
-		))
+		assert.Equal(d.T(), execArg, fmt.Sprintf("ALTER TABLE %s DROP COLUMN IF EXISTS %s", `shop.public."USERS"`, d.snowflakeStagesStore.Dialect().QuoteIdentifier(cols[i].Name())))
 	}
 }