From 4bfd93c698b8c27671f2d698a2b1907e8f499a40 Mon Sep 17 00:00:00 2001 From: Nathan Villaescusa Date: Sun, 28 Apr 2024 22:27:23 -0700 Subject: [PATCH] Kill uppercase escape name --- clients/bigquery/bigquery.go | 19 ++-- clients/mssql/staging.go | 16 ++- clients/mssql/store.go | 4 - clients/mssql/tableid.go | 2 +- clients/redshift/redshift.go | 4 - clients/redshift/staging.go | 16 ++- clients/redshift/tableid.go | 2 +- clients/shared/append.go | 16 ++- clients/shared/merge.go | 19 ++-- clients/shared/table_config_test.go | 1 - clients/shared/utils.go | 2 +- clients/snowflake/snowflake.go | 8 +- clients/snowflake/staging.go | 18 ++-- clients/snowflake/tableid.go | 2 +- clients/snowflake/writes.go | 2 +- lib/destination/ddl/ddl.go | 9 +- lib/destination/ddl/ddl_alter_delete_test.go | 14 --- lib/destination/ddl/ddl_bq_test.go | 70 ++++++------- lib/destination/ddl/ddl_create_table_test.go | 30 +++--- lib/destination/ddl/ddl_sflk_test.go | 47 ++++----- lib/destination/ddl/ddl_temp_test.go | 62 +++++------ lib/destination/dml/merge.go | 23 ++-- lib/destination/dml/merge_bigquery_test.go | 27 +++-- lib/destination/dml/merge_mssql_test.go | 16 ++- lib/destination/dml/merge_parts_test.go | 23 ++-- lib/destination/dml/merge_test.go | 82 +++++++-------- lib/destination/dml/merge_valid_test.go | 29 ++---- lib/destination/dwh.go | 1 - lib/optimization/table_data.go | 4 +- lib/sql/escape.go | 17 +-- lib/sql/escape_test.go | 104 ++++++++----------- lib/typing/columns/columns.go | 12 +-- lib/typing/columns/columns_test.go | 16 +-- lib/typing/columns/wrapper.go | 4 +- lib/typing/columns/wrapper_test.go | 8 +- 35 files changed, 299 insertions(+), 430 deletions(-) diff --git a/clients/bigquery/bigquery.go b/clients/bigquery/bigquery.go index 3c649f56d..7fa32d31d 100644 --- a/clients/bigquery/bigquery.go +++ b/clients/bigquery/bigquery.go @@ -39,14 +39,13 @@ type Store struct { func (s *Store) PrepareTemporaryTable(tableData *optimization.TableData, tableConfig *types.DwhTableConfig, tempTableID types.TableIdentifier, _ types.AdditionalSettings, createTempTable bool) error { if createTempTable { tempAlterTableArgs := ddl.AlterTableArgs{ - Dwh: s, - Tc: tableConfig, - TableID: tempTableID, - CreateTable: true, - TemporaryTable: true, - ColumnOp: constants.Add, - UppercaseEscNames: ptr.ToBool(s.ShouldUppercaseEscapedNames()), - Mode: tableData.Mode(), + Dwh: s, + Tc: tableConfig, + TableID: tempTableID, + CreateTable: true, + TemporaryTable: true, + ColumnOp: constants.Add, + Mode: tableData.Mode(), } if err := tempAlterTableArgs.AlterTable(tableData.ReadOnlyInMemoryCols().GetColumns()...); err != nil { @@ -110,10 +109,6 @@ func (s *Store) Label() constants.DestinationKind { return constants.BigQuery } -func (s *Store) ShouldUppercaseEscapedNames() bool { - return false -} - func (s *Store) GetClient(ctx context.Context) *bigquery.Client { client, err := bigquery.NewClient(ctx, s.config.BigQuery.ProjectID) if err != nil { diff --git a/clients/mssql/staging.go b/clients/mssql/staging.go index f373fbfb6..c70813d1c 100644 --- a/clients/mssql/staging.go +++ b/clients/mssql/staging.go @@ -9,20 +9,18 @@ import ( "github.com/artie-labs/transfer/lib/destination/ddl" "github.com/artie-labs/transfer/lib/destination/types" "github.com/artie-labs/transfer/lib/optimization" - "github.com/artie-labs/transfer/lib/ptr" ) func (s *Store) PrepareTemporaryTable(tableData *optimization.TableData, tableConfig *types.DwhTableConfig, tempTableID types.TableIdentifier, _ types.AdditionalSettings, createTempTable bool) error { if createTempTable { tempAlterTableArgs := ddl.AlterTableArgs{ - Dwh: s, - Tc: tableConfig, - TableID: tempTableID, - CreateTable: true, - TemporaryTable: true, - ColumnOp: constants.Add, - UppercaseEscNames: ptr.ToBool(s.ShouldUppercaseEscapedNames()), - Mode: tableData.Mode(), + Dwh: s, + Tc: tableConfig, + TableID: tempTableID, + CreateTable: true, + TemporaryTable: true, + ColumnOp: constants.Add, + Mode: tableData.Mode(), } if err := tempAlterTableArgs.AlterTable(tableData.ReadOnlyInMemoryCols().GetColumns()...); err != nil { diff --git a/clients/mssql/store.go b/clients/mssql/store.go index ffd5725c5..1f5acadcf 100644 --- a/clients/mssql/store.go +++ b/clients/mssql/store.go @@ -34,10 +34,6 @@ func (s *Store) Label() constants.DestinationKind { return constants.MSSQL } -func (s *Store) ShouldUppercaseEscapedNames() bool { - return false -} - func (s *Store) Merge(tableData *optimization.TableData) error { return shared.Merge(s, tableData, s.config, types.MergeOpts{}) } diff --git a/clients/mssql/tableid.go b/clients/mssql/tableid.go index 09b4ee8d2..1c74597f4 100644 --- a/clients/mssql/tableid.go +++ b/clients/mssql/tableid.go @@ -33,6 +33,6 @@ func (ti TableIdentifier) FullyQualifiedName() string { return fmt.Sprintf( "%s.%s", ti.schema, - sql.EscapeName(ti.table, false, constants.MSSQL), + sql.EscapeName(ti.table, constants.MSSQL), ) } diff --git a/clients/redshift/redshift.go b/clients/redshift/redshift.go index 34acb73f4..1a801c9fc 100644 --- a/clients/redshift/redshift.go +++ b/clients/redshift/redshift.go @@ -45,10 +45,6 @@ func (s *Store) Label() constants.DestinationKind { return constants.Redshift } -func (s *Store) ShouldUppercaseEscapedNames() bool { - return false -} - func (s *Store) GetTableConfig(tableData *optimization.TableData) (*types.DwhTableConfig, error) { const ( describeNameCol = "column_name" diff --git a/clients/redshift/staging.go b/clients/redshift/staging.go index bb6614baa..5c562034c 100644 --- a/clients/redshift/staging.go +++ b/clients/redshift/staging.go @@ -12,21 +12,19 @@ import ( "github.com/artie-labs/transfer/lib/destination/ddl" "github.com/artie-labs/transfer/lib/destination/types" "github.com/artie-labs/transfer/lib/optimization" - "github.com/artie-labs/transfer/lib/ptr" "github.com/artie-labs/transfer/lib/s3lib" ) func (s *Store) PrepareTemporaryTable(tableData *optimization.TableData, tableConfig *types.DwhTableConfig, tempTableID types.TableIdentifier, _ types.AdditionalSettings, _ bool) error { // Redshift always creates a temporary table. tempAlterTableArgs := ddl.AlterTableArgs{ - Dwh: s, - Tc: tableConfig, - TableID: tempTableID, - CreateTable: true, - TemporaryTable: true, - ColumnOp: constants.Add, - UppercaseEscNames: ptr.ToBool(s.ShouldUppercaseEscapedNames()), - Mode: tableData.Mode(), + Dwh: s, + Tc: tableConfig, + TableID: tempTableID, + CreateTable: true, + TemporaryTable: true, + ColumnOp: constants.Add, + Mode: tableData.Mode(), } if err := tempAlterTableArgs.AlterTable(tableData.ReadOnlyInMemoryCols().GetColumns()...); err != nil { diff --git a/clients/redshift/tableid.go b/clients/redshift/tableid.go index aea58d7e3..3ae2d7e68 100644 --- a/clients/redshift/tableid.go +++ b/clients/redshift/tableid.go @@ -35,6 +35,6 @@ func (ti TableIdentifier) FullyQualifiedName() string { return fmt.Sprintf( "%s.%s", ti.schema, - sql.EscapeName(ti.table, false, constants.Redshift), + sql.EscapeName(ti.table, constants.Redshift), ) } diff --git a/clients/shared/append.go b/clients/shared/append.go index 21c755194..c4a2af112 100644 --- a/clients/shared/append.go +++ b/clients/shared/append.go @@ -8,7 +8,6 @@ import ( "github.com/artie-labs/transfer/lib/destination/ddl" "github.com/artie-labs/transfer/lib/destination/types" "github.com/artie-labs/transfer/lib/optimization" - "github.com/artie-labs/transfer/lib/ptr" "github.com/artie-labs/transfer/lib/typing/columns" ) @@ -29,14 +28,13 @@ func Append(dwh destination.DataWarehouse, tableData *optimization.TableData, op tableData.TopicConfig().IncludeDatabaseUpdatedAt, tableData.Mode()) createAlterTableArgs := ddl.AlterTableArgs{ - Dwh: dwh, - Tc: tableConfig, - TableID: tableID, - CreateTable: tableConfig.CreateTable(), - ColumnOp: constants.Add, - CdcTime: tableData.LatestCDCTs, - UppercaseEscNames: ptr.ToBool(dwh.ShouldUppercaseEscapedNames()), - Mode: tableData.Mode(), + Dwh: dwh, + Tc: tableConfig, + TableID: tableID, + CreateTable: tableConfig.CreateTable(), + ColumnOp: constants.Add, + CdcTime: tableData.LatestCDCTs, + Mode: tableData.Mode(), } // Keys that exist in CDC stream, but not in DWH diff --git a/clients/shared/merge.go b/clients/shared/merge.go index b9247faa6..6522ae9bc 100644 --- a/clients/shared/merge.go +++ b/clients/shared/merge.go @@ -35,14 +35,13 @@ func Merge(dwh destination.DataWarehouse, tableData *optimization.TableData, cfg tableID := dwh.IdentifierFor(tableData.TopicConfig(), tableData.Name()) createAlterTableArgs := ddl.AlterTableArgs{ - Dwh: dwh, - Tc: tableConfig, - TableID: tableID, - CreateTable: tableConfig.CreateTable(), - ColumnOp: constants.Add, - CdcTime: tableData.LatestCDCTs, - UppercaseEscNames: ptr.ToBool(dwh.ShouldUppercaseEscapedNames()), - Mode: tableData.Mode(), + Dwh: dwh, + Tc: tableConfig, + TableID: tableID, + CreateTable: tableConfig.CreateTable(), + ColumnOp: constants.Add, + CdcTime: tableData.LatestCDCTs, + Mode: tableData.Mode(), } // Columns that are missing in DWH, but exist in our CDC stream. @@ -60,7 +59,6 @@ func Merge(dwh destination.DataWarehouse, tableData *optimization.TableData, cfg ColumnOp: constants.Delete, ContainOtherOperations: tableData.ContainOtherOperations(), CdcTime: tableData.LatestCDCTs, - UppercaseEscNames: ptr.ToBool(dwh.ShouldUppercaseEscapedNames()), Mode: tableData.Mode(), } @@ -122,11 +120,10 @@ func Merge(dwh destination.DataWarehouse, tableData *optimization.TableData, cfg TableID: tableID, SubQuery: subQuery, IdempotentKey: tableData.TopicConfig().IdempotentKey, - PrimaryKeys: tableData.PrimaryKeys(dwh.ShouldUppercaseEscapedNames(), dwh.Label()), + PrimaryKeys: tableData.PrimaryKeys(dwh.Label()), Columns: tableData.ReadOnlyInMemoryCols(), SoftDelete: tableData.TopicConfig().SoftDelete, DestKind: dwh.Label(), - UppercaseEscNames: ptr.ToBool(dwh.ShouldUppercaseEscapedNames()), ContainsHardDeletes: ptr.ToBool(tableData.ContainsHardDeletes()), } diff --git a/clients/shared/table_config_test.go b/clients/shared/table_config_test.go index 1d8b26d81..3c8061bb5 100644 --- a/clients/shared/table_config_test.go +++ b/clients/shared/table_config_test.go @@ -76,7 +76,6 @@ func (MockDWH) PrepareTemporaryTable(tableData *optimization.TableData, tableCon func (MockDWH) IdentifierFor(topicConfig kafkalib.TopicConfig, name string) types.TableIdentifier { panic("not implemented") } -func (MockDWH) ShouldUppercaseEscapedNames() bool { return true } type MockTableIdentifier struct{ fqName string } diff --git a/clients/shared/utils.go b/clients/shared/utils.go index b3b236604..c5a89b272 100644 --- a/clients/shared/utils.go +++ b/clients/shared/utils.go @@ -30,7 +30,7 @@ func BackfillColumn(cfg config.Config, dwh destination.DataWarehouse, column col return fmt.Errorf("failed to escape default value: %w", err) } - escapedCol := column.Name(dwh.ShouldUppercaseEscapedNames(), dwh.Label()) + escapedCol := column.Name(dwh.Label()) // TODO: This is added because `default` is not technically a column that requires escaping, but it is required when it's in the where clause. // Once we escape everything by default, we can remove this patch of code. diff --git a/clients/snowflake/snowflake.go b/clients/snowflake/snowflake.go index ff85f71b4..76eef7572 100644 --- a/clients/snowflake/snowflake.go +++ b/clients/snowflake/snowflake.go @@ -79,10 +79,6 @@ func (s *Store) Label() constants.DestinationKind { return constants.Snowflake } -func (s *Store) ShouldUppercaseEscapedNames() bool { - return true -} - func (s *Store) GetConfigMap() *types.DwhToTablesConfigMap { if s == nil { return nil @@ -133,7 +129,7 @@ func (s *Store) generateDedupeQueries(tableID, stagingTableID types.TableIdentif var primaryKeysEscaped []string for _, pk := range primaryKeys { pkCol := columns.NewColumn(pk, typing.Invalid) - primaryKeysEscaped = append(primaryKeysEscaped, pkCol.Name(s.ShouldUppercaseEscapedNames(), s.Label())) + primaryKeysEscaped = append(primaryKeysEscaped, pkCol.Name(s.Label())) } orderColsToIterate := primaryKeysEscaped @@ -146,7 +142,7 @@ func (s *Store) generateDedupeQueries(tableID, stagingTableID types.TableIdentif orderByCols = append(orderByCols, fmt.Sprintf("%s ASC", pk)) } - temporaryTableName := sql.EscapeName(stagingTableID.Table(), s.ShouldUppercaseEscapedNames(), s.Label()) + temporaryTableName := sql.EscapeName(stagingTableID.Table(), s.Label()) var parts []string parts = append(parts, fmt.Sprintf("CREATE OR REPLACE TRANSIENT TABLE %s AS (SELECT * FROM %s QUALIFY ROW_NUMBER() OVER (PARTITION BY by %s ORDER BY %s) = 2)", temporaryTableName, diff --git a/clients/snowflake/staging.go b/clients/snowflake/staging.go index 91e07e286..729c7074f 100644 --- a/clients/snowflake/staging.go +++ b/clients/snowflake/staging.go @@ -12,7 +12,6 @@ import ( "github.com/artie-labs/transfer/lib/destination/ddl" "github.com/artie-labs/transfer/lib/destination/types" "github.com/artie-labs/transfer/lib/optimization" - "github.com/artie-labs/transfer/lib/ptr" "github.com/artie-labs/transfer/lib/typing" "github.com/artie-labs/transfer/lib/typing/columns" "github.com/artie-labs/transfer/lib/typing/values" @@ -49,14 +48,13 @@ func castColValStaging(colVal any, colKind columns.Column, additionalDateFmts [] func (s *Store) PrepareTemporaryTable(tableData *optimization.TableData, tableConfig *types.DwhTableConfig, tempTableID types.TableIdentifier, additionalSettings types.AdditionalSettings, createTempTable bool) error { if createTempTable { tempAlterTableArgs := ddl.AlterTableArgs{ - Dwh: s, - Tc: tableConfig, - TableID: tempTableID, - CreateTable: true, - TemporaryTable: true, - ColumnOp: constants.Add, - UppercaseEscNames: ptr.ToBool(s.ShouldUppercaseEscapedNames()), - Mode: tableData.Mode(), + Dwh: s, + Tc: tableConfig, + TableID: tempTableID, + CreateTable: true, + TemporaryTable: true, + ColumnOp: constants.Add, + Mode: tableData.Mode(), } if err := tempAlterTableArgs.AlterTable(tableData.ReadOnlyInMemoryCols().GetColumns()...); err != nil { @@ -85,7 +83,7 @@ func (s *Store) PrepareTemporaryTable(tableData *optimization.TableData, tableCo // COPY the CSV file (in Snowflake) into a table copyCommand := fmt.Sprintf("COPY INTO %s (%s) FROM (SELECT %s FROM @%s)", tempTableID.FullyQualifiedName(), - strings.Join(tableData.ReadOnlyInMemoryCols().GetEscapedColumnsToUpdate(s.ShouldUppercaseEscapedNames(), s.Label()), ","), + strings.Join(tableData.ReadOnlyInMemoryCols().GetEscapedColumnsToUpdate(s.Label()), ","), escapeColumns(tableData.ReadOnlyInMemoryCols(), ","), addPrefixToTableName(tempTableID, "%")) if additionalSettings.AdditionalCopyClause != "" { diff --git a/clients/snowflake/tableid.go b/clients/snowflake/tableid.go index 057129194..b69140a80 100644 --- a/clients/snowflake/tableid.go +++ b/clients/snowflake/tableid.go @@ -43,6 +43,6 @@ func (ti TableIdentifier) FullyQualifiedName() string { "%s.%s.%s", ti.database, ti.schema, - sql.EscapeName(ti.table, true, constants.Snowflake), + sql.EscapeName(ti.table, constants.Snowflake), ) } diff --git a/clients/snowflake/writes.go b/clients/snowflake/writes.go index 84f39f612..4d5abc747 100644 --- a/clients/snowflake/writes.go +++ b/clients/snowflake/writes.go @@ -55,7 +55,7 @@ func (s *Store) Merge(tableData *optimization.TableData) error { var additionalEqualityStrings []string if len(tableData.TopicConfig().AdditionalMergePredicates) > 0 { for _, additionalMergePredicate := range tableData.TopicConfig().AdditionalMergePredicates { - mergePredicateCol := sql.EscapeName(additionalMergePredicate.PartitionField, s.ShouldUppercaseEscapedNames(), s.Label()) + mergePredicateCol := sql.EscapeName(additionalMergePredicate.PartitionField, s.Label()) additionalEqualityStrings = append(additionalEqualityStrings, fmt.Sprintf("c.%s = cc.%s", mergePredicateCol, mergePredicateCol)) } } diff --git a/lib/destination/ddl/ddl.go b/lib/destination/ddl/ddl.go index 723238ac6..a702a4901 100644 --- a/lib/destination/ddl/ddl.go +++ b/lib/destination/ddl/ddl.go @@ -44,7 +44,6 @@ type AlterTableArgs struct { TableID types.TableIdentifier CreateTable bool TemporaryTable bool - UppercaseEscNames *bool ColumnOp constants.ColumnOperation Mode config.Mode @@ -69,10 +68,6 @@ func (a AlterTableArgs) Validate() error { } } - if a.UppercaseEscNames == nil { - return fmt.Errorf("uppercaseEscNames cannot be nil") - } - return nil } @@ -104,7 +99,7 @@ func (a AlterTableArgs) AlterTable(cols ...columns.Column) error { mutateCol = append(mutateCol, col) switch a.ColumnOp { case constants.Add: - colName := col.Name(*a.UppercaseEscNames, a.Dwh.Label()) + colName := col.Name(a.Dwh.Label()) if col.PrimaryKey() && a.Mode != config.History { // Don't create a PK for history mode because it's append-only, so the primary key should not be enforced. @@ -113,7 +108,7 @@ func (a AlterTableArgs) AlterTable(cols ...columns.Column) error { colSQLParts = append(colSQLParts, fmt.Sprintf(`%s %s`, colName, typing.KindToDWHType(col.KindDetails, a.Dwh.Label(), col.PrimaryKey()))) case constants.Delete: - colSQLParts = append(colSQLParts, col.Name(*a.UppercaseEscNames, a.Dwh.Label())) + colSQLParts = append(colSQLParts, col.Name(a.Dwh.Label())) } } diff --git a/lib/destination/ddl/ddl_alter_delete_test.go b/lib/destination/ddl/ddl_alter_delete_test.go index e3b47ff12..1f0f82d2a 100644 --- a/lib/destination/ddl/ddl_alter_delete_test.go +++ b/lib/destination/ddl/ddl_alter_delete_test.go @@ -6,8 +6,6 @@ import ( "github.com/artie-labs/transfer/lib/config" - "github.com/artie-labs/transfer/lib/ptr" - "github.com/artie-labs/transfer/lib/config/constants" "github.com/artie-labs/transfer/lib/destination/ddl" "github.com/artie-labs/transfer/lib/destination/types" @@ -68,7 +66,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -89,7 +86,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -111,7 +107,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -146,7 +141,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: false, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -167,7 +161,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: false, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -188,7 +181,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: false, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -225,7 +217,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -242,7 +233,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -259,7 +249,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -284,7 +273,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts.Add(2 * constants.DeletionConfidencePadding), - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -299,7 +287,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts.Add(2 * constants.DeletionConfidencePadding), - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -314,7 +301,6 @@ func (d *DDLTestSuite) TestAlterDelete_Complete() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: ts.Add(2 * constants.DeletionConfidencePadding), - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } diff --git a/lib/destination/ddl/ddl_bq_test.go b/lib/destination/ddl/ddl_bq_test.go index 62c6a19bc..fb0c4974e 100644 --- a/lib/destination/ddl/ddl_bq_test.go +++ b/lib/destination/ddl/ddl_bq_test.go @@ -9,8 +9,6 @@ import ( "github.com/artie-labs/transfer/clients/bigquery" "github.com/artie-labs/transfer/lib/config" - "github.com/artie-labs/transfer/lib/ptr" - "github.com/artie-labs/transfer/lib/typing/columns" "github.com/stretchr/testify/assert" @@ -61,7 +59,6 @@ func (d *DDLTestSuite) TestAlterTableDropColumnsBigQuery() { ColumnOp: constants.Delete, ContainOtherOperations: true, CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -84,13 +81,12 @@ func (d *DDLTestSuite) TestAlterTableDropColumnsBigQuery() { ColumnOp: constants.Delete, ContainOtherOperations: true, CdcTime: ts.Add(2 * constants.DeletionConfidencePadding), - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(column)) query, _ := d.fakeBigQueryStore.ExecArgsForCall(callIdx) - assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s drop COLUMN %s", fqName, column.Name(false, d.bigQueryStore.Label())), query) + assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s drop COLUMN %s", fqName, column.Name(d.bigQueryStore.Label())), query) callIdx += 1 } @@ -134,21 +130,20 @@ func (d *DDLTestSuite) TestAlterTableAddColumns() { tc := d.bigQueryStore.GetConfigMap().TableConfig(tableID) for name, kind := range newCols { alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.bigQueryStore, - Tc: tc, - TableID: tableID, - CreateTable: tc.CreateTable(), - ColumnOp: constants.Add, - CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.bigQueryStore, + Tc: tc, + TableID: tableID, + CreateTable: tc.CreateTable(), + ColumnOp: constants.Add, + CdcTime: ts, + Mode: config.Replication, } col := columns.NewColumn(name, kind) assert.NoError(d.T(), alterTableArgs.AlterTable(col)) query, _ := d.fakeBigQueryStore.ExecArgsForCall(callIdx) - assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s %s COLUMN %s %s", fqName, constants.Add, col.Name(false, d.bigQueryStore.Label()), + assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s %s COLUMN %s %s", fqName, constants.Add, col.Name(d.bigQueryStore.Label()), typing.KindToDWHType(kind, d.bigQueryStore.Label(), false)), query) callIdx += 1 } @@ -196,19 +191,18 @@ func (d *DDLTestSuite) TestAlterTableAddColumnsSomeAlreadyExist() { // BQ returning the same error because the column already exists. d.fakeBigQueryStore.ExecReturnsOnCall(0, sqlResult, errors.New("Column already exists: _string at [1:39]")) alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.bigQueryStore, - Tc: tc, - TableID: tableID, - CreateTable: tc.CreateTable(), - ColumnOp: constants.Add, - CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.bigQueryStore, + Tc: tc, + TableID: tableID, + CreateTable: tc.CreateTable(), + ColumnOp: constants.Add, + CdcTime: ts, + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(column)) query, _ := d.fakeBigQueryStore.ExecArgsForCall(callIdx) - assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s %s COLUMN %s %s", fqName, constants.Add, column.Name(false, d.bigQueryStore.Label()), + assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s %s COLUMN %s %s", fqName, constants.Add, column.Name(d.bigQueryStore.Label()), typing.KindToDWHType(column.KindDetails, d.bigQueryStore.Label(), false)), query) callIdx += 1 } @@ -250,14 +244,13 @@ func (d *DDLTestSuite) TestAlterTableDropColumnsBigQuerySafety() { assert.Equal(d.T(), 0, len(d.bigQueryStore.GetConfigMap().TableConfig(tableID).ReadOnlyColumnsToDelete()), d.bigQueryStore.GetConfigMap().TableConfig(tableID).ReadOnlyColumnsToDelete()) for _, column := range cols.GetColumns() { alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.bigQueryStore, - Tc: tc, - TableID: tableID, - CreateTable: tc.CreateTable(), - ColumnOp: constants.Delete, - CdcTime: ts, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.bigQueryStore, + Tc: tc, + TableID: tableID, + CreateTable: tc.CreateTable(), + ColumnOp: constants.Delete, + CdcTime: ts, + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(column)) } @@ -268,14 +261,13 @@ func (d *DDLTestSuite) TestAlterTableDropColumnsBigQuerySafety() { // Now try to delete again and with an increased TS. It should now be all deleted. for _, column := range cols.GetColumns() { alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.bigQueryStore, - Tc: tc, - TableID: tableID, - CreateTable: tc.CreateTable(), - ColumnOp: constants.Delete, - CdcTime: ts.Add(2 * constants.DeletionConfidencePadding), - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.bigQueryStore, + Tc: tc, + TableID: tableID, + CreateTable: tc.CreateTable(), + ColumnOp: constants.Delete, + CdcTime: ts.Add(2 * constants.DeletionConfidencePadding), + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(column)) diff --git a/lib/destination/ddl/ddl_create_table_test.go b/lib/destination/ddl/ddl_create_table_test.go index a8e07dfea..5dabd6f08 100644 --- a/lib/destination/ddl/ddl_create_table_test.go +++ b/lib/destination/ddl/ddl_create_table_test.go @@ -8,8 +8,6 @@ import ( "github.com/artie-labs/transfer/clients/snowflake" "github.com/artie-labs/transfer/lib/config" - "github.com/artie-labs/transfer/lib/ptr" - "github.com/artie-labs/transfer/lib/typing/columns" "github.com/artie-labs/transfer/lib/config/constants" @@ -53,13 +51,12 @@ func (d *DDLTestSuite) Test_CreateTable() { }, } { alterTableArgs := ddl.AlterTableArgs{ - Dwh: dwhTc._dwh, - Tc: dwhTc._tableConfig, - TableID: dwhTc._tableID, - CreateTable: dwhTc._tableConfig.CreateTable(), - ColumnOp: constants.Add, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: dwhTc._dwh, + Tc: dwhTc._tableConfig, + TableID: dwhTc._tableID, + CreateTable: dwhTc._tableConfig.CreateTable(), + ColumnOp: constants.Add, + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(columns.NewColumn("name", typing.String))) @@ -119,14 +116,13 @@ func (d *DDLTestSuite) TestCreateTable() { tc := d.snowflakeStagesStore.GetConfigMap().TableConfig(tableID) alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.snowflakeStagesStore, - Tc: tc, - TableID: tableID, - CreateTable: tc.CreateTable(), - ColumnOp: constants.Add, - CdcTime: time.Now().UTC(), - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.snowflakeStagesStore, + Tc: tc, + TableID: tableID, + CreateTable: tc.CreateTable(), + ColumnOp: constants.Add, + CdcTime: time.Now().UTC(), + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(testCase.cols...), testCase.name) diff --git a/lib/destination/ddl/ddl_sflk_test.go b/lib/destination/ddl/ddl_sflk_test.go index 2305929d2..fe0c81d3a 100644 --- a/lib/destination/ddl/ddl_sflk_test.go +++ b/lib/destination/ddl/ddl_sflk_test.go @@ -8,8 +8,6 @@ import ( "github.com/artie-labs/transfer/clients/snowflake" "github.com/artie-labs/transfer/lib/config" - "github.com/artie-labs/transfer/lib/ptr" - "github.com/artie-labs/transfer/lib/typing/columns" "github.com/stretchr/testify/assert" @@ -34,20 +32,19 @@ func (d *DDLTestSuite) TestAlterComplexObjects() { tc := d.snowflakeStagesStore.GetConfigMap().TableConfig(tableID) alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.snowflakeStagesStore, - Tc: tc, - TableID: tableID, - ColumnOp: constants.Add, - CdcTime: time.Now().UTC(), - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.snowflakeStagesStore, + Tc: tc, + TableID: tableID, + ColumnOp: constants.Add, + CdcTime: time.Now().UTC(), + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(cols...)) for i := 0; i < len(cols); i++ { execQuery, _ := d.fakeSnowflakeStagesStore.ExecArgsForCall(i) assert.Equal(d.T(), fmt.Sprintf("ALTER TABLE %s add COLUMN %s %s", `shop.public."COMPLEX_COLUMNS"`, - cols[i].Name(false, d.snowflakeStagesStore.Label()), + cols[i].Name(d.snowflakeStagesStore.Label()), typing.KindToDWHType(cols[i].KindDetails, d.snowflakeStagesStore.Label(), false)), execQuery) } @@ -68,13 +65,12 @@ func (d *DDLTestSuite) TestAlterIdempotency() { d.fakeSnowflakeStagesStore.ExecReturns(nil, errors.New("column 'order_name' already exists")) alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.snowflakeStagesStore, - Tc: tc, - TableID: tableID, - ColumnOp: constants.Add, - CdcTime: time.Now().UTC(), - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.snowflakeStagesStore, + Tc: tc, + TableID: tableID, + ColumnOp: constants.Add, + CdcTime: time.Now().UTC(), + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(cols...)) @@ -98,13 +94,12 @@ func (d *DDLTestSuite) TestAlterTableAdd() { tc := d.snowflakeStagesStore.GetConfigMap().TableConfig(tableID) alterTableArgs := ddl.AlterTableArgs{ - Dwh: d.snowflakeStagesStore, - Tc: tc, - TableID: tableID, - ColumnOp: constants.Add, - CdcTime: time.Now().UTC(), - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.snowflakeStagesStore, + Tc: tc, + TableID: tableID, + ColumnOp: constants.Add, + CdcTime: time.Now().UTC(), + Mode: config.Replication, } assert.NoError(d.T(), alterTableArgs.AlterTable(cols...)) @@ -146,7 +141,6 @@ func (d *DDLTestSuite) TestAlterTableDeleteDryRun() { ContainOtherOperations: true, ColumnOp: constants.Delete, CdcTime: time.Now().UTC(), - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } @@ -180,7 +174,7 @@ func (d *DDLTestSuite) TestAlterTableDeleteDryRun() { execArg, _ := d.fakeSnowflakeStagesStore.ExecArgsForCall(i) assert.Equal(d.T(), execArg, fmt.Sprintf("ALTER TABLE %s %s COLUMN %s", `shop.public."USERS"`, constants.Delete, - cols[i].Name(false, d.snowflakeStagesStore.Label()))) + cols[i].Name(d.snowflakeStagesStore.Label()))) } } @@ -211,7 +205,6 @@ func (d *DDLTestSuite) TestAlterTableDelete() { ColumnOp: constants.Delete, ContainOtherOperations: true, CdcTime: time.Now(), - UppercaseEscNames: ptr.ToBool(false), Mode: config.Replication, } diff --git a/lib/destination/ddl/ddl_temp_test.go b/lib/destination/ddl/ddl_temp_test.go index e04e30f08..99a5e257b 100644 --- a/lib/destination/ddl/ddl_temp_test.go +++ b/lib/destination/ddl/ddl_temp_test.go @@ -7,8 +7,6 @@ import ( "github.com/artie-labs/transfer/clients/snowflake" "github.com/artie-labs/transfer/lib/config" - "github.com/artie-labs/transfer/lib/ptr" - "github.com/artie-labs/transfer/lib/typing/columns" "github.com/artie-labs/transfer/lib/config/constants" @@ -20,10 +18,9 @@ import ( func (d *DDLTestSuite) TestValidate_AlterTableArgs() { a := &ddl.AlterTableArgs{ - ColumnOp: constants.Delete, - CreateTable: true, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + ColumnOp: constants.Delete, + CreateTable: true, + Mode: config.Replication, } assert.Contains(d.T(), a.Validate().Error(), "incompatible operation - cannot drop columns and create table at the same time") @@ -39,15 +36,14 @@ func (d *DDLTestSuite) TestCreateTemporaryTable_Errors() { d.snowflakeStagesStore.GetConfigMap().AddTableToConfig(tableID, types.NewDwhTableConfig(&columns.Columns{}, nil, true, true)) snowflakeTc := d.snowflakeStagesStore.GetConfigMap().TableConfig(tableID) args := ddl.AlterTableArgs{ - Dwh: d.snowflakeStagesStore, - Tc: snowflakeTc, - TableID: tableID, - CreateTable: true, - TemporaryTable: true, - ColumnOp: constants.Add, - CdcTime: time.Time{}, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.snowflakeStagesStore, + Tc: snowflakeTc, + TableID: tableID, + CreateTable: true, + TemporaryTable: true, + ColumnOp: constants.Add, + CdcTime: time.Time{}, + Mode: config.Replication, } // No columns. @@ -74,15 +70,14 @@ func (d *DDLTestSuite) TestCreateTemporaryTable() { d.snowflakeStagesStore.GetConfigMap().AddTableToConfig(tableID, types.NewDwhTableConfig(&columns.Columns{}, nil, true, true)) sflkStageTc := d.snowflakeStagesStore.GetConfigMap().TableConfig(tableID) args := ddl.AlterTableArgs{ - Dwh: d.snowflakeStagesStore, - Tc: sflkStageTc, - TableID: tableID, - CreateTable: true, - TemporaryTable: true, - ColumnOp: constants.Add, - CdcTime: time.Time{}, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.snowflakeStagesStore, + Tc: sflkStageTc, + TableID: tableID, + CreateTable: true, + TemporaryTable: true, + ColumnOp: constants.Add, + CdcTime: time.Time{}, + Mode: config.Replication, } assert.NoError(d.T(), args.AlterTable(columns.NewColumn("foo", typing.String), columns.NewColumn("bar", typing.Float), columns.NewColumn("start", typing.String))) @@ -91,7 +86,7 @@ func (d *DDLTestSuite) TestCreateTemporaryTable() { assert.Contains(d.T(), query, - `CREATE TABLE IF NOT EXISTS db.schema."TEMPTABLENAME" (foo string,bar float,"start" string) STAGE_COPY_OPTIONS = ( PURGE = TRUE ) STAGE_FILE_FORMAT = ( TYPE = 'csv' FIELD_DELIMITER= '\t' FIELD_OPTIONALLY_ENCLOSED_BY='"' NULL_IF='\\N' EMPTY_FIELD_AS_NULL=FALSE)`, + `CREATE TABLE IF NOT EXISTS db.schema."TEMPTABLENAME" (foo string,bar float,"START" string) STAGE_COPY_OPTIONS = ( PURGE = TRUE ) STAGE_FILE_FORMAT = ( TYPE = 'csv' FIELD_DELIMITER= '\t' FIELD_OPTIONALLY_ENCLOSED_BY='"' NULL_IF='\\N' EMPTY_FIELD_AS_NULL=FALSE)`, query) } { @@ -100,15 +95,14 @@ func (d *DDLTestSuite) TestCreateTemporaryTable() { d.bigQueryStore.GetConfigMap().AddTableToConfig(tableID, types.NewDwhTableConfig(&columns.Columns{}, nil, true, true)) bqTc := d.bigQueryStore.GetConfigMap().TableConfig(tableID) args := ddl.AlterTableArgs{ - Dwh: d.bigQueryStore, - Tc: bqTc, - TableID: tableID, - CreateTable: true, - TemporaryTable: true, - ColumnOp: constants.Add, - CdcTime: time.Time{}, - UppercaseEscNames: ptr.ToBool(false), - Mode: config.Replication, + Dwh: d.bigQueryStore, + Tc: bqTc, + TableID: tableID, + CreateTable: true, + TemporaryTable: true, + ColumnOp: constants.Add, + CdcTime: time.Time{}, + Mode: config.Replication, } assert.NoError(d.T(), args.AlterTable(columns.NewColumn("foo", typing.String), columns.NewColumn("bar", typing.Float), columns.NewColumn("select", typing.String))) diff --git a/lib/destination/dml/merge.go b/lib/destination/dml/merge.go index 0a551beb0..66d221dff 100644 --- a/lib/destination/dml/merge.go +++ b/lib/destination/dml/merge.go @@ -29,7 +29,6 @@ type MergeArgument struct { // ContainsHardDeletes is only used for Redshift and MergeStatementParts, // where we do not issue a DELETE statement if there are no hard deletes in the batch ContainsHardDeletes *bool - UppercaseEscNames *bool } func (m *MergeArgument) Valid() error { @@ -53,10 +52,6 @@ func (m *MergeArgument) Valid() error { return fmt.Errorf("subQuery cannot be empty") } - if m.UppercaseEscNames == nil { - return fmt.Errorf("uppercaseEscNames cannot be nil") - } - if !constants.IsValidDestination(m.DestKind) { return fmt.Errorf("invalid destination: %s", m.DestKind) } @@ -97,7 +92,7 @@ func (m *MergeArgument) GetParts() ([]string, error) { equalitySQLParts = append(equalitySQLParts, equalitySQL) } - cols := m.Columns.GetEscapedColumnsToUpdate(*m.UppercaseEscNames, m.DestKind) + cols := m.Columns.GetEscapedColumnsToUpdate(m.DestKind) if m.SoftDelete { return []string{ @@ -118,7 +113,7 @@ func (m *MergeArgument) GetParts() ([]string, error) { // UPDATE fmt.Sprintf(`UPDATE %s as c SET %s FROM %s as cc WHERE %s%s;`, // UPDATE table set col1 = cc. col1 - m.TableID.FullyQualifiedName(), m.Columns.UpdateQuery(m.DestKind, *m.UppercaseEscNames, false), + m.TableID.FullyQualifiedName(), m.Columns.UpdateQuery(m.DestKind, false), // FROM table (temp) WHERE join on PK(s) m.SubQuery, strings.Join(equalitySQLParts, " and "), idempotentClause, ), @@ -162,7 +157,7 @@ func (m *MergeArgument) GetParts() ([]string, error) { // UPDATE fmt.Sprintf(`UPDATE %s as c SET %s FROM %s as cc WHERE %s%s AND COALESCE(cc.%s, false) = false;`, // UPDATE table set col1 = cc. col1 - m.TableID.FullyQualifiedName(), m.Columns.UpdateQuery(m.DestKind, *m.UppercaseEscNames, true), + m.TableID.FullyQualifiedName(), m.Columns.UpdateQuery(m.DestKind, true), // FROM staging WHERE join on PK(s) m.SubQuery, strings.Join(equalitySQLParts, " and "), idempotentClause, constants.DeleteColumnMarker, ), @@ -229,7 +224,7 @@ func (m *MergeArgument) GetStatement() (string, error) { equalitySQLParts = append(equalitySQLParts, m.AdditionalEqualityStrings...) } - cols := m.Columns.GetEscapedColumnsToUpdate(*m.UppercaseEscNames, m.DestKind) + cols := m.Columns.GetEscapedColumnsToUpdate(m.DestKind) if m.SoftDelete { return fmt.Sprintf(` @@ -238,7 +233,7 @@ WHEN MATCHED %sTHEN UPDATE SET %s WHEN NOT MATCHED AND IFNULL(cc.%s, false) = false THEN INSERT (%s) VALUES (%s);`, m.TableID.FullyQualifiedName(), subQuery, strings.Join(equalitySQLParts, " and "), // Update + Soft Deletion - idempotentClause, m.Columns.UpdateQuery(m.DestKind, *m.UppercaseEscNames, false), + idempotentClause, m.Columns.UpdateQuery(m.DestKind, false), // Insert constants.DeleteColumnMarker, strings.Join(cols, ","), array.StringsJoinAddPrefix(array.StringsJoinAddPrefixArgs{ @@ -271,7 +266,7 @@ WHEN NOT MATCHED AND IFNULL(cc.%s, false) = false THEN INSERT (%s) VALUES (%s);` // Delete constants.DeleteColumnMarker, // Update - constants.DeleteColumnMarker, idempotentClause, m.Columns.UpdateQuery(m.DestKind, *m.UppercaseEscNames, true), + constants.DeleteColumnMarker, idempotentClause, m.Columns.UpdateQuery(m.DestKind, true), // Insert constants.DeleteColumnMarker, strings.Join(cols, ","), array.StringsJoinAddPrefix(array.StringsJoinAddPrefixArgs{ @@ -298,7 +293,7 @@ func (m *MergeArgument) GetMSSQLStatement() (string, error) { equalitySQLParts = append(equalitySQLParts, equalitySQL) } - cols := m.Columns.GetEscapedColumnsToUpdate(*m.UppercaseEscNames, m.DestKind) + cols := m.Columns.GetEscapedColumnsToUpdate(m.DestKind) if m.SoftDelete { return fmt.Sprintf(` @@ -308,7 +303,7 @@ WHEN MATCHED %sTHEN UPDATE SET %s WHEN NOT MATCHED AND COALESCE(cc.%s, 0) = 0 THEN INSERT (%s) VALUES (%s);`, m.TableID.FullyQualifiedName(), m.SubQuery, strings.Join(equalitySQLParts, " and "), // Update + Soft Deletion - idempotentClause, m.Columns.UpdateQuery(m.DestKind, *m.UppercaseEscNames, false), + idempotentClause, m.Columns.UpdateQuery(m.DestKind, false), // Insert constants.DeleteColumnMarker, strings.Join(cols, ","), array.StringsJoinAddPrefix(array.StringsJoinAddPrefixArgs{ @@ -342,7 +337,7 @@ WHEN NOT MATCHED AND COALESCE(cc.%s, 1) = 0 THEN INSERT (%s) VALUES (%s);`, // Delete constants.DeleteColumnMarker, // Update - constants.DeleteColumnMarker, idempotentClause, m.Columns.UpdateQuery(m.DestKind, *m.UppercaseEscNames, true), + constants.DeleteColumnMarker, idempotentClause, m.Columns.UpdateQuery(m.DestKind, true), // Insert constants.DeleteColumnMarker, strings.Join(cols, ","), array.StringsJoinAddPrefix(array.StringsJoinAddPrefixArgs{ diff --git a/lib/destination/dml/merge_bigquery_test.go b/lib/destination/dml/merge_bigquery_test.go index 6fb44d488..0ca3eb0a1 100644 --- a/lib/destination/dml/merge_bigquery_test.go +++ b/lib/destination/dml/merge_bigquery_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/artie-labs/transfer/lib/config/constants" - "github.com/artie-labs/transfer/lib/ptr" "github.com/artie-labs/transfer/lib/typing" "github.com/artie-labs/transfer/lib/typing/columns" "github.com/stretchr/testify/assert" @@ -17,13 +16,12 @@ func TestMergeStatement_TempTable(t *testing.T) { cols.AddColumn(columns.NewColumn(constants.DeleteColumnMarker, typing.Boolean)) mergeArg := &MergeArgument{ - TableID: MockTableIdentifier{"customers.orders"}, - SubQuery: "customers.orders_tmp", - PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("order_id", typing.Invalid), false, constants.BigQuery)}, - Columns: &cols, - DestKind: constants.BigQuery, - SoftDelete: false, - UppercaseEscNames: ptr.ToBool(false), + TableID: MockTableIdentifier{"customers.orders"}, + SubQuery: "customers.orders_tmp", + PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("order_id", typing.Invalid), constants.BigQuery)}, + Columns: &cols, + DestKind: constants.BigQuery, + SoftDelete: false, } mergeSQL, err := mergeArg.GetStatement() @@ -39,13 +37,12 @@ func TestMergeStatement_JSONKey(t *testing.T) { cols.AddColumn(columns.NewColumn(constants.DeleteColumnMarker, typing.Boolean)) mergeArg := &MergeArgument{ - TableID: MockTableIdentifier{"customers.orders"}, - SubQuery: "customers.orders_tmp", - PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("order_oid", typing.Invalid), false, constants.BigQuery)}, - Columns: &cols, - DestKind: constants.BigQuery, - SoftDelete: false, - UppercaseEscNames: ptr.ToBool(false), + TableID: MockTableIdentifier{"customers.orders"}, + SubQuery: "customers.orders_tmp", + PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("order_oid", typing.Invalid), constants.BigQuery)}, + Columns: &cols, + DestKind: constants.BigQuery, + SoftDelete: false, } mergeSQL, err := mergeArg.GetStatement() diff --git a/lib/destination/dml/merge_mssql_test.go b/lib/destination/dml/merge_mssql_test.go index 385d4ace5..50e419c7f 100644 --- a/lib/destination/dml/merge_mssql_test.go +++ b/lib/destination/dml/merge_mssql_test.go @@ -7,7 +7,6 @@ import ( "time" "github.com/artie-labs/transfer/lib/config/constants" - "github.com/artie-labs/transfer/lib/ptr" "github.com/artie-labs/transfer/lib/typing" "github.com/artie-labs/transfer/lib/typing/columns" "github.com/stretchr/testify/assert" @@ -41,14 +40,13 @@ func Test_GetMSSQLStatement(t *testing.T) { strings.Join(cols, ","), strings.Join(tableValues, ","), "_tbl", strings.Join(cols, ",")) mergeArg := MergeArgument{ - TableID: MockTableIdentifier{fqTable}, - SubQuery: subQuery, - IdempotentKey: "", - PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), false, constants.MSSQL)}, - Columns: &_cols, - DestKind: constants.MSSQL, - SoftDelete: false, - UppercaseEscNames: ptr.ToBool(false), + TableID: MockTableIdentifier{fqTable}, + SubQuery: subQuery, + IdempotentKey: "", + PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), constants.MSSQL)}, + Columns: &_cols, + DestKind: constants.MSSQL, + SoftDelete: false, } mergeSQL, err := mergeArg.GetMSSQLStatement() diff --git a/lib/destination/dml/merge_parts_test.go b/lib/destination/dml/merge_parts_test.go index 23f07bb94..bf618fa5b 100644 --- a/lib/destination/dml/merge_parts_test.go +++ b/lib/destination/dml/merge_parts_test.go @@ -31,7 +31,7 @@ type result struct { // getBasicColumnsForTest - will return you all the columns within `result` that are needed for tests. // * In here, we'll return if compositeKey=false - id (pk), email, first_name, last_name, created_at, toast_text (TOAST-able) // * Else if compositeKey=true - id(pk), email (pk), first_name, last_name, created_at, toast_text (TOAST-able) -func getBasicColumnsForTest(compositeKey bool, uppercaseEscNames bool) result { +func getBasicColumnsForTest(compositeKey bool) result { idCol := columns.NewColumn("id", typing.Float) emailCol := columns.NewColumn("email", typing.String) textToastCol := columns.NewColumn("toast_text", typing.String) @@ -47,10 +47,10 @@ func getBasicColumnsForTest(compositeKey bool, uppercaseEscNames bool) result { cols.AddColumn(columns.NewColumn(constants.DeleteColumnMarker, typing.Boolean)) var pks []columns.Wrapper - pks = append(pks, columns.NewWrapper(idCol, uppercaseEscNames, constants.Redshift)) + pks = append(pks, columns.NewWrapper(idCol, constants.Redshift)) if compositeKey { - pks = append(pks, columns.NewWrapper(emailCol, uppercaseEscNames, constants.Redshift)) + pks = append(pks, columns.NewWrapper(emailCol, constants.Redshift)) } return result{ @@ -65,7 +65,7 @@ func TestMergeStatementParts_SkipDelete(t *testing.T) { // 2. There are 3 SQL queries (INSERT, UPDATE and DELETE) fqTableName := "public.tableName" tempTableName := "public.tableName__temp" - res := getBasicColumnsForTest(false, false) + res := getBasicColumnsForTest(false) mergeArg := &MergeArgument{ TableID: MockTableIdentifier{fqTableName}, SubQuery: tempTableName, @@ -73,7 +73,6 @@ func TestMergeStatementParts_SkipDelete(t *testing.T) { Columns: &res.ColumnsToTypes, DestKind: constants.Redshift, ContainsHardDeletes: ptr.ToBool(false), - UppercaseEscNames: ptr.ToBool(false), } parts, err := mergeArg.GetParts() @@ -92,7 +91,7 @@ func TestMergeStatementParts_SkipDelete(t *testing.T) { func TestMergeStatementPartsSoftDelete(t *testing.T) { fqTableName := "public.tableName" tempTableName := "public.tableName__temp" - res := getBasicColumnsForTest(false, false) + res := getBasicColumnsForTest(false) mergeArg := &MergeArgument{ TableID: MockTableIdentifier{fqTableName}, SubQuery: tempTableName, @@ -100,7 +99,6 @@ func TestMergeStatementPartsSoftDelete(t *testing.T) { Columns: &res.ColumnsToTypes, DestKind: constants.Redshift, SoftDelete: true, - UppercaseEscNames: ptr.ToBool(false), ContainsHardDeletes: ptr.ToBool(false), } @@ -132,7 +130,7 @@ func TestMergeStatementPartsSoftDelete(t *testing.T) { func TestMergeStatementPartsSoftDeleteComposite(t *testing.T) { fqTableName := "public.tableName" tempTableName := "public.tableName__temp" - res := getBasicColumnsForTest(true, false) + res := getBasicColumnsForTest(true) mergeArg := &MergeArgument{ TableID: MockTableIdentifier{fqTableName}, SubQuery: tempTableName, @@ -140,7 +138,6 @@ func TestMergeStatementPartsSoftDeleteComposite(t *testing.T) { Columns: &res.ColumnsToTypes, DestKind: constants.Redshift, SoftDelete: true, - UppercaseEscNames: ptr.ToBool(false), ContainsHardDeletes: ptr.ToBool(false), } @@ -175,7 +172,7 @@ func TestMergeStatementParts(t *testing.T) { // 2. There are 3 SQL queries (INSERT, UPDATE and DELETE) fqTableName := "public.tableName" tempTableName := "public.tableName__temp" - res := getBasicColumnsForTest(false, false) + res := getBasicColumnsForTest(false) mergeArg := &MergeArgument{ TableID: MockTableIdentifier{fqTableName}, SubQuery: tempTableName, @@ -183,7 +180,6 @@ func TestMergeStatementParts(t *testing.T) { Columns: &res.ColumnsToTypes, DestKind: constants.Redshift, ContainsHardDeletes: ptr.ToBool(true), - UppercaseEscNames: ptr.ToBool(false), } parts, err := mergeArg.GetParts() @@ -210,7 +206,6 @@ func TestMergeStatementParts(t *testing.T) { DestKind: constants.Redshift, IdempotentKey: "created_at", ContainsHardDeletes: ptr.ToBool(true), - UppercaseEscNames: ptr.ToBool(false), } parts, err = mergeArg.GetParts() @@ -233,7 +228,7 @@ func TestMergeStatementParts(t *testing.T) { func TestMergeStatementPartsCompositeKey(t *testing.T) { fqTableName := "public.tableName" tempTableName := "public.tableName__temp" - res := getBasicColumnsForTest(true, false) + res := getBasicColumnsForTest(true) mergeArg := &MergeArgument{ TableID: MockTableIdentifier{fqTableName}, SubQuery: tempTableName, @@ -241,7 +236,6 @@ func TestMergeStatementPartsCompositeKey(t *testing.T) { Columns: &res.ColumnsToTypes, DestKind: constants.Redshift, ContainsHardDeletes: ptr.ToBool(true), - UppercaseEscNames: ptr.ToBool(false), } parts, err := mergeArg.GetParts() @@ -268,7 +262,6 @@ func TestMergeStatementPartsCompositeKey(t *testing.T) { DestKind: constants.Redshift, ContainsHardDeletes: ptr.ToBool(true), IdempotentKey: "created_at", - UppercaseEscNames: ptr.ToBool(false), } parts, err = mergeArg.GetParts() diff --git a/lib/destination/dml/merge_test.go b/lib/destination/dml/merge_test.go index 962902109..6a465b4c2 100644 --- a/lib/destination/dml/merge_test.go +++ b/lib/destination/dml/merge_test.go @@ -10,7 +10,6 @@ import ( "github.com/artie-labs/transfer/lib/config/constants" "github.com/artie-labs/transfer/lib/destination/types" - "github.com/artie-labs/transfer/lib/ptr" "github.com/artie-labs/transfer/lib/typing" "github.com/artie-labs/transfer/lib/typing/columns" ) @@ -58,14 +57,13 @@ func TestMergeStatementSoftDelete(t *testing.T) { for _, idempotentKey := range []string{"", "updated_at"} { mergeArg := MergeArgument{ - TableID: MockTableIdentifier{fqTable}, - SubQuery: subQuery, - IdempotentKey: idempotentKey, - PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), false, constants.Snowflake)}, - Columns: &_cols, - DestKind: constants.Snowflake, - SoftDelete: true, - UppercaseEscNames: ptr.ToBool(false), + TableID: MockTableIdentifier{fqTable}, + SubQuery: subQuery, + IdempotentKey: idempotentKey, + PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), constants.Snowflake)}, + Columns: &_cols, + DestKind: constants.Snowflake, + SoftDelete: true, } mergeSQL, err := mergeArg.GetStatement() @@ -107,14 +105,13 @@ func TestMergeStatement(t *testing.T) { strings.Join(cols, ","), strings.Join(tableValues, ","), "_tbl", strings.Join(cols, ",")) mergeArg := MergeArgument{ - TableID: MockTableIdentifier{fqTable}, - SubQuery: subQuery, - IdempotentKey: "", - PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), false, constants.Snowflake)}, - Columns: &_cols, - DestKind: constants.Snowflake, - SoftDelete: false, - UppercaseEscNames: ptr.ToBool(false), + TableID: MockTableIdentifier{fqTable}, + SubQuery: subQuery, + IdempotentKey: "", + PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), constants.Snowflake)}, + Columns: &_cols, + DestKind: constants.Snowflake, + SoftDelete: false, } mergeSQL, err := mergeArg.GetStatement() @@ -125,10 +122,10 @@ func TestMergeStatement(t *testing.T) { assert.Contains(t, mergeSQL, "AS cc ON c.id = cc.id", mergeSQL) // Check setting for update - assert.Contains(t, mergeSQL, `SET id=cc.id,bar=cc.bar,updated_at=cc.updated_at,"start"=cc."start"`, mergeSQL) + assert.Contains(t, mergeSQL, `SET id=cc.id,bar=cc.bar,updated_at=cc.updated_at,"START"=cc."START"`, mergeSQL) // Check for INSERT - assert.Contains(t, mergeSQL, `id,bar,updated_at,"start"`, mergeSQL) - assert.Contains(t, mergeSQL, `cc.id,cc.bar,cc.updated_at,cc."start"`, mergeSQL) + assert.Contains(t, mergeSQL, `id,bar,updated_at,"START"`, mergeSQL) + assert.Contains(t, mergeSQL, `cc.id,cc.bar,cc.updated_at,cc."START"`, mergeSQL) } func TestMergeStatementIdempotentKey(t *testing.T) { @@ -155,14 +152,13 @@ func TestMergeStatementIdempotentKey(t *testing.T) { _cols.AddColumn(columns.NewColumn(constants.DeleteColumnMarker, typing.Boolean)) mergeArg := MergeArgument{ - TableID: MockTableIdentifier{fqTable}, - SubQuery: subQuery, - IdempotentKey: "updated_at", - PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), false, constants.Snowflake)}, - Columns: &_cols, - DestKind: constants.Snowflake, - SoftDelete: false, - UppercaseEscNames: ptr.ToBool(false), + TableID: MockTableIdentifier{fqTable}, + SubQuery: subQuery, + IdempotentKey: "updated_at", + PrimaryKeys: []columns.Wrapper{columns.NewWrapper(columns.NewColumn("id", typing.Invalid), constants.Snowflake)}, + Columns: &_cols, + DestKind: constants.Snowflake, + SoftDelete: false, } mergeSQL, err := mergeArg.GetStatement() @@ -201,13 +197,12 @@ func TestMergeStatementCompositeKey(t *testing.T) { SubQuery: subQuery, IdempotentKey: "updated_at", PrimaryKeys: []columns.Wrapper{ - columns.NewWrapper(columns.NewColumn("id", typing.Invalid), false, constants.Snowflake), - columns.NewWrapper(columns.NewColumn("another_id", typing.Invalid), false, constants.Snowflake), + columns.NewWrapper(columns.NewColumn("id", typing.Invalid), constants.Snowflake), + columns.NewWrapper(columns.NewColumn("another_id", typing.Invalid), constants.Snowflake), }, - Columns: &_cols, - DestKind: constants.Snowflake, - SoftDelete: false, - UppercaseEscNames: ptr.ToBool(false), + Columns: &_cols, + DestKind: constants.Snowflake, + SoftDelete: false, } mergeSQL, err := mergeArg.GetStatement() @@ -250,13 +245,12 @@ func TestMergeStatementEscapePrimaryKeys(t *testing.T) { SubQuery: subQuery, IdempotentKey: "", PrimaryKeys: []columns.Wrapper{ - columns.NewWrapper(columns.NewColumn("id", typing.Invalid), false, constants.Snowflake), - columns.NewWrapper(columns.NewColumn("group", typing.Invalid), false, constants.Snowflake), + columns.NewWrapper(columns.NewColumn("id", typing.Invalid), constants.Snowflake), + columns.NewWrapper(columns.NewColumn("group", typing.Invalid), constants.Snowflake), }, - Columns: &_cols, - DestKind: constants.Snowflake, - SoftDelete: false, - UppercaseEscNames: ptr.ToBool(false), + Columns: &_cols, + DestKind: constants.Snowflake, + SoftDelete: false, } mergeSQL, err := mergeArg.GetStatement() @@ -264,10 +258,10 @@ func TestMergeStatementEscapePrimaryKeys(t *testing.T) { assert.Contains(t, mergeSQL, fmt.Sprintf("MERGE INTO %s", fqTable), mergeSQL) assert.NotContains(t, mergeSQL, fmt.Sprintf("cc.%s >= c.%s", "updated_at", "updated_at"), fmt.Sprintf("Idempotency key: %s", mergeSQL)) // Check primary keys clause - assert.Contains(t, mergeSQL, `AS cc ON c.id = cc.id and c."group" = cc."group"`, mergeSQL) + assert.Contains(t, mergeSQL, `AS cc ON c.id = cc.id and c."GROUP" = cc."GROUP"`, mergeSQL) // Check setting for update - assert.Contains(t, mergeSQL, `SET id=cc.id,"group"=cc."group",updated_at=cc.updated_at,"start"=cc."start"`, mergeSQL) + assert.Contains(t, mergeSQL, `SET id=cc.id,"GROUP"=cc."GROUP",updated_at=cc.updated_at,"START"=cc."START"`, mergeSQL) // Check for INSERT - assert.Contains(t, mergeSQL, `id,"group",updated_at,"start"`, mergeSQL) - assert.Contains(t, mergeSQL, `cc.id,cc."group",cc.updated_at,cc."start"`, mergeSQL) + assert.Contains(t, mergeSQL, `id,"GROUP",updated_at,"START"`, mergeSQL) + assert.Contains(t, mergeSQL, `cc.id,cc."GROUP",cc.updated_at,cc."START"`, mergeSQL) } diff --git a/lib/destination/dml/merge_valid_test.go b/lib/destination/dml/merge_valid_test.go index e8223c97e..ec0aa742c 100644 --- a/lib/destination/dml/merge_valid_test.go +++ b/lib/destination/dml/merge_valid_test.go @@ -5,7 +5,6 @@ import ( "github.com/artie-labs/transfer/lib/config/constants" - "github.com/artie-labs/transfer/lib/ptr" "github.com/artie-labs/transfer/lib/typing" "github.com/artie-labs/transfer/lib/typing/columns" @@ -14,7 +13,7 @@ import ( func TestMergeArgument_Valid(t *testing.T) { primaryKeys := []columns.Wrapper{ - columns.NewWrapper(columns.NewColumn("id", typing.Integer), false, constants.Snowflake), + columns.NewWrapper(columns.NewColumn("id", typing.Integer), constants.Snowflake), } var cols columns.Columns @@ -70,35 +69,23 @@ func TestMergeArgument_Valid(t *testing.T) { expectedErr: "subQuery cannot be empty", }, { - name: "did not pass in uppercase esc col", + name: "missing dest kind", mergeArg: &MergeArgument{ PrimaryKeys: primaryKeys, Columns: &cols, - TableID: MockTableIdentifier{"schema.tableName"}, SubQuery: "schema.tableName", - }, - expectedErr: "uppercaseEscNames cannot be nil", - }, - { - name: "missing dest kind", - mergeArg: &MergeArgument{ - PrimaryKeys: primaryKeys, - Columns: &cols, - SubQuery: "schema.tableName", - TableID: MockTableIdentifier{"schema.tableName"}, - UppercaseEscNames: ptr.ToBool(false), + TableID: MockTableIdentifier{"schema.tableName"}, }, expectedErr: "invalid destination", }, { name: "everything exists", mergeArg: &MergeArgument{ - PrimaryKeys: primaryKeys, - Columns: &cols, - SubQuery: "schema.tableName", - TableID: MockTableIdentifier{"schema.tableName"}, - UppercaseEscNames: ptr.ToBool(false), - DestKind: constants.BigQuery, + PrimaryKeys: primaryKeys, + Columns: &cols, + SubQuery: "schema.tableName", + TableID: MockTableIdentifier{"schema.tableName"}, + DestKind: constants.BigQuery, }, }, } diff --git a/lib/destination/dwh.go b/lib/destination/dwh.go index 3a0acfa40..8ef6dc7d8 100644 --- a/lib/destination/dwh.go +++ b/lib/destination/dwh.go @@ -19,7 +19,6 @@ type DataWarehouse interface { Begin() (*sql.Tx, error) // Helper functions for merge - ShouldUppercaseEscapedNames() bool IsRetryableError(err error) bool IdentifierFor(topicConfig kafkalib.TopicConfig, table string) types.TableIdentifier GetTableConfig(tableData *optimization.TableData) (*types.DwhTableConfig, error) diff --git a/lib/optimization/table_data.go b/lib/optimization/table_data.go index 89b3eb264..1a6ad5b16 100644 --- a/lib/optimization/table_data.go +++ b/lib/optimization/table_data.go @@ -66,10 +66,10 @@ func (t *TableData) ContainOtherOperations() bool { return t.containOtherOperations } -func (t *TableData) PrimaryKeys(uppercaseEscNames bool, destKind constants.DestinationKind) []columns.Wrapper { +func (t *TableData) PrimaryKeys(destKind constants.DestinationKind) []columns.Wrapper { var pks []columns.Wrapper for _, pk := range t.primaryKeys { - pks = append(pks, columns.NewWrapper(columns.NewColumn(pk, typing.Invalid), uppercaseEscNames, destKind)) + pks = append(pks, columns.NewWrapper(columns.NewColumn(pk, typing.Invalid), destKind)) } return pks diff --git a/lib/sql/escape.go b/lib/sql/escape.go index 180f303dc..5ac6a22f8 100644 --- a/lib/sql/escape.go +++ b/lib/sql/escape.go @@ -2,7 +2,6 @@ package sql import ( "fmt" - "log/slog" "slices" "strconv" "strings" @@ -13,9 +12,9 @@ import ( // symbolsToEscape are additional keywords that we need to escape var symbolsToEscape = []string{":"} -func EscapeNameIfNecessary(name string, uppercaseEscNames bool, destKind constants.DestinationKind) string { +func EscapeNameIfNecessary(name string, destKind constants.DestinationKind) string { if NeedsEscaping(name, destKind) { - return EscapeName(name, uppercaseEscNames, destKind) + return EscapeName(name, destKind) } return name } @@ -49,16 +48,10 @@ func NeedsEscaping(name string, destKind constants.DestinationKind) bool { return false } -func EscapeName(name string, uppercaseEscNames bool, destKind constants.DestinationKind) string { - if uppercaseEscNames { +func EscapeName(name string, destKind constants.DestinationKind) string { + if destKind == constants.Snowflake { + // Snowflake identifiers are uppercase by default. name = strings.ToUpper(name) - } else { - if destKind == constants.Snowflake { - slog.Warn("Escaped Snowflake identifier is not being uppercased", - slog.String("name", name), - slog.Bool("uppercaseEscapedNames", uppercaseEscNames), - ) - } } if destKind == constants.BigQuery { diff --git a/lib/sql/escape_test.go b/lib/sql/escape_test.go index ed44be702..92ef9b499 100644 --- a/lib/sql/escape_test.go +++ b/lib/sql/escape_test.go @@ -9,91 +9,77 @@ import ( func TestEscapeNameIfNecessary(t *testing.T) { type _testCase struct { - name string - nameToEscape string - destKind constants.DestinationKind - expectedName string - expectedNameWhenUpperCfg string + name string + nameToEscape string + destKind constants.DestinationKind + expectedName string } testCases := []_testCase{ { - name: "snowflake", - destKind: constants.Snowflake, - nameToEscape: "order", - expectedName: `"order"`, - expectedNameWhenUpperCfg: `"ORDER"`, + name: "snowflake", + destKind: constants.Snowflake, + nameToEscape: "order", + expectedName: `"ORDER"`, }, { - name: "snowflake #2", - destKind: constants.Snowflake, - nameToEscape: "hello", - expectedName: `hello`, - expectedNameWhenUpperCfg: "hello", + name: "snowflake #2", + destKind: constants.Snowflake, + nameToEscape: "hello", + expectedName: `hello`, }, { - name: "redshift", - destKind: constants.Redshift, - nameToEscape: "order", - expectedName: `"order"`, - expectedNameWhenUpperCfg: `"ORDER"`, + name: "redshift", + destKind: constants.Redshift, + nameToEscape: "order", + expectedName: `"order"`, }, { - name: "redshift #2", - destKind: constants.Redshift, - nameToEscape: "hello", - expectedName: `hello`, - expectedNameWhenUpperCfg: "hello", + name: "redshift #2", + destKind: constants.Redshift, + nameToEscape: "hello", + expectedName: `hello`, }, { - name: "bigquery", - destKind: constants.BigQuery, - nameToEscape: "order", - expectedName: "`order`", - expectedNameWhenUpperCfg: "`ORDER`", + name: "bigquery", + destKind: constants.BigQuery, + nameToEscape: "order", + expectedName: "`order`", }, { - name: "bigquery, #2", - destKind: constants.BigQuery, - nameToEscape: "hello", - expectedName: "hello", - expectedNameWhenUpperCfg: "hello", + name: "bigquery, #2", + destKind: constants.BigQuery, + nameToEscape: "hello", + expectedName: "hello", }, { - name: "redshift, #1 (delta)", - destKind: constants.Redshift, - nameToEscape: "delta", - expectedName: `"delta"`, - expectedNameWhenUpperCfg: `"DELTA"`, + name: "redshift, #1 (delta)", + destKind: constants.Redshift, + nameToEscape: "delta", + expectedName: `"delta"`, }, { - name: "snowflake, #1 (delta)", - destKind: constants.Snowflake, - nameToEscape: "delta", - expectedName: `delta`, - expectedNameWhenUpperCfg: `delta`, + name: "snowflake, #1 (delta)", + destKind: constants.Snowflake, + nameToEscape: "delta", + expectedName: `delta`, }, { - name: "redshift, symbols", - destKind: constants.Redshift, - nameToEscape: "receivedat:__", - expectedName: `"receivedat:__"`, - expectedNameWhenUpperCfg: `"RECEIVEDAT:__"`, + name: "redshift, symbols", + destKind: constants.Redshift, + nameToEscape: "receivedat:__", + expectedName: `"receivedat:__"`, }, { - name: "redshift, numbers", - destKind: constants.Redshift, - nameToEscape: "0", - expectedName: `"0"`, - expectedNameWhenUpperCfg: `"0"`, + name: "redshift, numbers", + destKind: constants.Redshift, + nameToEscape: "0", + expectedName: `"0"`, }, } for _, testCase := range testCases { - actualName := EscapeNameIfNecessary(testCase.nameToEscape, false, testCase.destKind) + actualName := EscapeNameIfNecessary(testCase.nameToEscape, testCase.destKind) assert.Equal(t, testCase.expectedName, actualName, testCase.name) - - actualUpperName := EscapeNameIfNecessary(testCase.nameToEscape, true, testCase.destKind) - assert.Equal(t, testCase.expectedNameWhenUpperCfg, actualUpperName, testCase.name) } } diff --git a/lib/typing/columns/columns.go b/lib/typing/columns/columns.go index 2ed85ff2f..a1c316aab 100644 --- a/lib/typing/columns/columns.go +++ b/lib/typing/columns/columns.go @@ -86,8 +86,8 @@ func (c *Column) RawName() string { // Name will give you c.name // Plus we will escape it if the column name is part of the reserved words from destinations. // If so, it'll change from `start` => `"start"` as suggested by Snowflake. -func (c *Column) Name(uppercaseEscNames bool, destKind constants.DestinationKind) string { - return sql.EscapeNameIfNecessary(c.name, uppercaseEscNames, destKind) +func (c *Column) Name(destKind constants.DestinationKind) string { + return sql.EscapeNameIfNecessary(c.name, destKind) } type Columns struct { @@ -198,7 +198,7 @@ func (c *Columns) GetColumnsToUpdate() []string { // GetEscapedColumnsToUpdate will filter all the `Invalid` columns so that we do not update it. // It will escape the returned columns. -func (c *Columns) GetEscapedColumnsToUpdate(uppercaseEscNames bool, destKind constants.DestinationKind) []string { +func (c *Columns) GetEscapedColumnsToUpdate(destKind constants.DestinationKind) []string { if c == nil { return []string{} } @@ -212,7 +212,7 @@ func (c *Columns) GetEscapedColumnsToUpdate(uppercaseEscNames bool, destKind con continue } - cols = append(cols, col.Name(uppercaseEscNames, destKind)) + cols = append(cols, col.Name(destKind)) } return cols @@ -257,7 +257,7 @@ func (c *Columns) DeleteColumn(name string) { } // UpdateQuery will parse the columns and then returns a list of strings like: cc.first_name=c.first_name,cc.last_name=c.last_name,cc.email=c.email -func (c *Columns) UpdateQuery(destKind constants.DestinationKind, uppercaseEscNames bool, skipDeleteCol bool) string { +func (c *Columns) UpdateQuery(destKind constants.DestinationKind, skipDeleteCol bool) string { var cols []string for _, column := range c.GetColumns() { if column.ShouldSkip() { @@ -269,7 +269,7 @@ func (c *Columns) UpdateQuery(destKind constants.DestinationKind, uppercaseEscNa continue } - colName := column.Name(uppercaseEscNames, destKind) + colName := column.Name(destKind) if column.ToastColumn { if column.KindDetails == typing.Struct { cols = append(cols, processToastStructCol(colName, destKind)) diff --git a/lib/typing/columns/columns_test.go b/lib/typing/columns/columns_test.go index 8a2ef5b91..13803f87f 100644 --- a/lib/typing/columns/columns_test.go +++ b/lib/typing/columns/columns_test.go @@ -145,7 +145,7 @@ func TestColumn_Name(t *testing.T) { { colName: "start", expectedName: "start", - expectedNameEsc: `"start"`, // since this is a reserved word. + expectedNameEsc: `"START"`, // since this is a reserved word. expectedNameEscBq: "`start`", // BQ escapes via backticks. }, { @@ -169,8 +169,8 @@ func TestColumn_Name(t *testing.T) { assert.Equal(t, testCase.expectedName, col.RawName(), testCase.colName) - assert.Equal(t, testCase.expectedNameEsc, col.Name(false, constants.Snowflake), testCase.colName) - assert.Equal(t, testCase.expectedNameEscBq, col.Name(false, constants.BigQuery), testCase.colName) + assert.Equal(t, testCase.expectedNameEsc, col.Name(constants.Snowflake), testCase.colName) + assert.Equal(t, testCase.expectedNameEscBq, col.Name(constants.BigQuery), testCase.colName) } } @@ -265,13 +265,13 @@ func TestColumns_GetEscapedColumnsToUpdate(t *testing.T) { { name: "happy path", cols: happyPathCols, - expectedColsEsc: []string{"hi", "bye", `"start"`}, + expectedColsEsc: []string{"hi", "bye", `"START"`}, expectedColsEscBq: []string{"hi", "bye", "`start`"}, }, { name: "happy path + extra col", cols: extraCols, - expectedColsEsc: []string{"hi", "bye", `"start"`}, + expectedColsEsc: []string{"hi", "bye", `"START"`}, expectedColsEscBq: []string{"hi", "bye", "`start`"}, }, } @@ -281,8 +281,8 @@ func TestColumns_GetEscapedColumnsToUpdate(t *testing.T) { columns: testCase.cols, } - assert.Equal(t, testCase.expectedColsEsc, cols.GetEscapedColumnsToUpdate(false, constants.Snowflake), testCase.name) - assert.Equal(t, testCase.expectedColsEscBq, cols.GetEscapedColumnsToUpdate(false, constants.BigQuery), testCase.name) + assert.Equal(t, testCase.expectedColsEsc, cols.GetEscapedColumnsToUpdate(constants.Snowflake), testCase.name) + assert.Equal(t, testCase.expectedColsEscBq, cols.GetEscapedColumnsToUpdate(constants.BigQuery), testCase.name) } } @@ -519,7 +519,7 @@ func TestColumnsUpdateQuery(t *testing.T) { } for _, _testCase := range testCases { - actualQuery := _testCase.columns.UpdateQuery(_testCase.destKind, false, _testCase.skipDeleteCol) + actualQuery := _testCase.columns.UpdateQuery(_testCase.destKind, _testCase.skipDeleteCol) assert.Equal(t, _testCase.expectedString, actualQuery, _testCase.name) } } diff --git a/lib/typing/columns/wrapper.go b/lib/typing/columns/wrapper.go index cb5f37643..2366bcc8d 100644 --- a/lib/typing/columns/wrapper.go +++ b/lib/typing/columns/wrapper.go @@ -7,10 +7,10 @@ type Wrapper struct { escapedName string } -func NewWrapper(col Column, uppercaseEscNames bool, destKind constants.DestinationKind) Wrapper { +func NewWrapper(col Column, destKind constants.DestinationKind) Wrapper { return Wrapper{ name: col.name, - escapedName: col.Name(uppercaseEscNames, destKind), + escapedName: col.Name(destKind), } } diff --git a/lib/typing/columns/wrapper_test.go b/lib/typing/columns/wrapper_test.go index 7a624ab18..8dd2140bd 100644 --- a/lib/typing/columns/wrapper_test.go +++ b/lib/typing/columns/wrapper_test.go @@ -34,26 +34,26 @@ func TestWrapper_Complete(t *testing.T) { { name: "group", expectedRawName: "group", - expectedEscapedName: `"group"`, + expectedEscapedName: `"GROUP"`, expectedEscapedNameBQ: "`group`", }, } for _, testCase := range testCases { // Snowflake escape - w := NewWrapper(NewColumn(testCase.name, typing.Invalid), false, constants.Snowflake) + w := NewWrapper(NewColumn(testCase.name, typing.Invalid), constants.Snowflake) assert.Equal(t, testCase.expectedEscapedName, w.EscapedName(), testCase.name) assert.Equal(t, testCase.expectedRawName, w.RawName(), testCase.name) // BigQuery escape - w = NewWrapper(NewColumn(testCase.name, typing.Invalid), false, constants.BigQuery) + w = NewWrapper(NewColumn(testCase.name, typing.Invalid), constants.BigQuery) assert.Equal(t, testCase.expectedEscapedNameBQ, w.EscapedName(), testCase.name) assert.Equal(t, testCase.expectedRawName, w.RawName(), testCase.name) for _, destKind := range []constants.DestinationKind{constants.Snowflake, constants.BigQuery} { - w = NewWrapper(NewColumn(testCase.name, typing.Invalid), false, destKind) + w = NewWrapper(NewColumn(testCase.name, typing.Invalid), destKind) assert.Equal(t, testCase.expectedRawName, w.RawName(), testCase.name) } }