Merge branch 'master' into refactor-cols

artie-labs · Sep 9, 2024 · 04b6da8 · 04b6da8
2 parents 7eef047 + e3cf70e
commit 04b6da8
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 11 deletions.
diff --git a/clients/bigquery/dialect/dialect_test.go b/clients/bigquery/dialect/dialect_test.go
@@ -35,12 +35,34 @@ func TestBigQueryDialect_DataTypeForKind(t *testing.T) {
 
 func TestBigQueryDialect_KindForDataType(t *testing.T) {
 	dialect := BigQueryDialect{}
+	{
+		// Numeric
+		{
+			// Numeric(5)
+			kd, err := dialect.KindForDataType("NUMERIC(5)", "")
+			assert.NoError(t, err)
+
+			assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
+			assert.Equal(t, int32(5), kd.ExtendedDecimalDetails.Precision())
+			assert.Equal(t, int32(0), kd.ExtendedDecimalDetails.Scale())
+			assert.Equal(t, "NUMERIC(5, 0)", kd.ExtendedDecimalDetails.BigQueryKind())
+
+		}
+		{
+			// Numeric(5, 0)
+			kd, err := dialect.KindForDataType("NUMERIC(5, 0)", "")
+			assert.NoError(t, err)
+
+			assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
+			assert.Equal(t, int32(5), kd.ExtendedDecimalDetails.Precision())
+			assert.Equal(t, int32(0), kd.ExtendedDecimalDetails.Scale())
+			assert.Equal(t, "NUMERIC(5, 0)", kd.ExtendedDecimalDetails.BigQueryKind())
+		}
+	}
 
 	bqColToExpectedKind := map[string]typing.KindDetails{
 		// Number
 		"numeric":           typing.EDecimal,
-		"numeric(5)":        typing.Integer,
-		"numeric(5, 0)":     typing.Integer,
 		"numeric(5, 2)":     typing.EDecimal,
 		"numeric(8, 6)":     typing.EDecimal,
 		"bignumeric(38, 2)": typing.EDecimal,

diff --git a/clients/snowflake/dialect/dialect_test.go b/clients/snowflake/dialect/dialect_test.go
@@ -34,11 +34,36 @@ func TestSnowflakeDialect_DataTypeForKind(t *testing.T) {
 
 func TestSnowflakeDialect_KindForDataType_Number(t *testing.T) {
 	{
-		expectedIntegers := []string{"number(38, 0)", "number(2, 0)", "number(3, 0)"}
-		for _, expectedInteger := range expectedIntegers {
-			kd, err := SnowflakeDialect{}.KindForDataType(expectedInteger, "")
+		// Integers
+		{
+			// number(38, 0)
+			kd, err := SnowflakeDialect{}.KindForDataType("number(38, 0)", "")
 			assert.NoError(t, err)
-			assert.Equal(t, typing.Integer, kd, expectedInteger)
+
+			assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
+			assert.Equal(t, int32(38), kd.ExtendedDecimalDetails.Precision())
+			assert.Equal(t, int32(0), kd.ExtendedDecimalDetails.Scale())
+			assert.Equal(t, "NUMERIC(38, 0)", kd.ExtendedDecimalDetails.SnowflakeKind())
+		}
+		{
+			// number(2, 0)
+			kd, err := SnowflakeDialect{}.KindForDataType("number(2, 0)", "")
+			assert.NoError(t, err)
+
+			assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
+			assert.Equal(t, int32(2), kd.ExtendedDecimalDetails.Precision())
+			assert.Equal(t, int32(0), kd.ExtendedDecimalDetails.Scale())
+			assert.Equal(t, "NUMERIC(2, 0)", kd.ExtendedDecimalDetails.SnowflakeKind())
+		}
+		{
+			// number(3, 0)
+			kd, err := SnowflakeDialect{}.KindForDataType("number(3, 0)", "")
+			assert.NoError(t, err)
+
+			assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
+			assert.Equal(t, int32(3), kd.ExtendedDecimalDetails.Precision())
+			assert.Equal(t, int32(0), kd.ExtendedDecimalDetails.Scale())
+			assert.Equal(t, "NUMERIC(3, 0)", kd.ExtendedDecimalDetails.SnowflakeKind())
 		}
 	}
 	{

diff --git a/lib/optimization/table_data.go b/lib/optimization/table_data.go
@@ -2,6 +2,7 @@ package optimization
 
 import (
 	"fmt"
+	"log/slog"
 	"strings"
 	"time"
 
@@ -272,11 +273,14 @@ func (t *TableData) MergeColumnsFromDestination(destCols ...columns.Column) erro
 		}
 
 		if found {
+			// TODO: Simplify this whole logic
 			// If the inMemoryColumn is decimal and foundColumn is integer, don't copy it over.
 			// This is because parsing NUMERIC(...) will return an INTEGER if there's no decimal point.
 			// However, this will wipe the precision unit from the INTEGER which may cause integer overflow.
 			shouldSkip := inMemoryCol.KindDetails.Kind == typing.EDecimal.Kind && foundColumn.KindDetails.Kind == typing.Integer.Kind
-			if !shouldSkip {
+			if shouldSkip {
+				slog.Info("Skipping column", slog.String("column", inMemoryCol.Name()), slog.String("inMemoryKind", inMemoryCol.KindDetails.Kind), slog.String("foundKind", foundColumn.KindDetails.Kind))
+			} else {
 				// We should take `kindDetails.kind` and `backfilled` from foundCol
 				// We are not taking primaryKey and defaultValue because DWH does not have this information.
 				// Note: If our in-memory column is `Invalid`, it would get skipped during merge. However, if the column exists in

diff --git a/lib/sql/util.go b/lib/sql/util.go
@@ -33,7 +33,7 @@ func QuoteIdentifiers(identifiers []string, dialect Dialect) []string {
 // ParseDataTypeDefinition parses a column type definition returning the type and parameters.
 // "TEXT" -> "TEXT", {}
 // "VARCHAR(1234)" -> "VARCHAR", {"1234"}
-// "NUERMIC(5, 1)" -> "NUMERIC", {"5", "1"}
+// "NUMERIC(5, 1)" -> "NUMERIC", {"5", "1"}
 func ParseDataTypeDefinition(value string) (string, []string, error) {
 	value = strings.TrimSpace(value)
 

diff --git a/lib/typing/numeric.go b/lib/typing/numeric.go
@@ -24,7 +24,7 @@ func ParseNumeric(parts []string) KindDetails {
 
 	// If scale is 0 or not specified, then number is an int.
 	if len(parsedNumbers) == 1 || parsedNumbers[1] == 0 {
-		return Integer
+		return NewDecimalDetailsFromTemplate(EDecimal, decimal.NewDetails(parsedNumbers[0], 0))
 	}
 
 	return NewDecimalDetailsFromTemplate(EDecimal, decimal.NewDetails(parsedNumbers[0], parsedNumbers[1]))

diff --git a/lib/typing/numeric_test.go b/lib/typing/numeric_test.go
@@ -68,11 +68,15 @@ func TestParseNumeric(t *testing.T) {
 		// Integer
 		{
 			result := ParseNumeric([]string{"5"})
-			assert.Equal(t, Integer, result)
+			assert.Equal(t, EDecimal.Kind, result.Kind)
+			assert.Equal(t, int32(5), result.ExtendedDecimalDetails.Precision())
+			assert.Equal(t, int32(0), result.ExtendedDecimalDetails.Scale())
 		}
 		{
 			result := ParseNumeric([]string{"5", "0"})
-			assert.Equal(t, Integer, result)
+			assert.Equal(t, EDecimal.Kind, result.Kind)
+			assert.Equal(t, int32(5), result.ExtendedDecimalDetails.Precision())
+			assert.Equal(t, int32(0), result.ExtendedDecimalDetails.Scale())
 		}
 	}
 }