Skip to content

Commit

Permalink
Don't backfill or update table column if it's invalid (#140)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tang8330 authored Jun 27, 2023
1 parent 20657e1 commit b79fa0a
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 3 deletions.
4 changes: 4 additions & 0 deletions clients/bigquery/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ func (s *Store) Merge(ctx context.Context, tableData *optimization.TableData) er

// Backfill columns if necessary
for _, col := range tableData.ReadOnlyInMemoryCols().GetColumns() {
if col.ShouldSkip() {
continue
}

var attempts int
for {
err = s.backfillColumn(ctx, col, tableData.ToFqName(ctx, s.Label()))
Expand Down
4 changes: 4 additions & 0 deletions clients/redshift/redshift.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ func (s *Store) Merge(ctx context.Context, tableData *optimization.TableData) er

// Now iterate over all the in-memory cols and see which one requires backfill.
for _, col := range tableData.ReadOnlyInMemoryCols().GetColumns() {
if col.ShouldSkip() {
continue
}

err = utils.BackfillColumn(ctx, s, col, tableData.ToFqName(ctx, s.Label()))
if err != nil {
defaultVal, _ := col.DefaultValue(nil)
Expand Down
4 changes: 4 additions & 0 deletions clients/snowflake/staging.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ func (s *Store) mergeWithStages(ctx context.Context, tableData *optimization.Tab

// Now iterate over all the in-memory cols and see which one requires backfill.
for _, col := range tableData.ReadOnlyInMemoryCols().GetColumns() {
if col.ShouldSkip() {
continue
}

err = utils.BackfillColumn(ctx, s, col, tableData.ToFqName(ctx, s.Label()))
if err != nil {
defaultVal, _ := col.DefaultValue(nil)
Expand Down
2 changes: 1 addition & 1 deletion lib/dwh/ddl/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func AlterTable(ctx context.Context, args AlterTableArgs, cols ...columns.Column
// It's okay to combine since args.ColumnOp only takes one of: `Delete` or `Add`
var colSQLParts []string
for _, col := range cols {
if col.KindDetails == typing.Invalid {
if col.ShouldSkip() {
// Let's not modify the table if the column kind is invalid
continue
}
Expand Down
12 changes: 10 additions & 2 deletions lib/typing/columns/columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ type Column struct {
backfilled bool
}

func (c *Column) ShouldSkip() bool {
if c == nil || c.KindDetails.Kind == typing.Invalid.Kind {
return true
}

return false
}

func UnescapeColumnName(escapedName string, destKind constants.DestinationKind) string {
if destKind == constants.BigQuery {
return strings.ReplaceAll(escapedName, "`", "")
Expand Down Expand Up @@ -63,8 +71,8 @@ func (c *Column) ShouldBackfill() bool {
return false
}

if c.KindDetails.Kind == typing.Invalid.Kind {
// Don't backfill if the in-memory data is `INVALID`
if c.ShouldSkip() {
// Don't backfill
return false
}

Expand Down
32 changes: 32 additions & 0 deletions lib/typing/columns/columns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,38 @@ import (
"github.com/stretchr/testify/assert"
)

func TestColumn_ShouldSkip(t *testing.T) {
type _testCase struct {
name string
col *Column
expectedResult bool
}

testCases := []_testCase{
{
name: "col is nil",
expectedResult: true,
},
{
name: "invalid column",
col: &Column{
KindDetails: typing.Invalid,
},
expectedResult: true,
},
{
name: "normal column",
col: &Column{
KindDetails: typing.String,
},
},
}

for _, testCase := range testCases {
assert.Equal(t, testCase.expectedResult, testCase.col.ShouldSkip(), testCase.name)
}
}

func TestColumn_ShouldBackfill(t *testing.T) {
type _testCase struct {
name string
Expand Down

0 comments on commit b79fa0a

Please sign in to comment.