Merge branch 'master' into redshift-string-precision

artie-labs · Oct 9, 2024 · aa3c8b7 · aa3c8b7
2 parents ba626c9 + ccfa3fd
commit aa3c8b7
Show file tree

Hide file tree

Showing 9 changed files with 59 additions and 29 deletions.
diff --git a/clients/bigquery/dialect/dialect.go b/clients/bigquery/dialect/dialect.go
@@ -89,7 +89,7 @@ func (BigQueryDialect) KindForDataType(rawBqType string, _ string) (typing.KindD
 			// A `NUMERIC` type without precision or scale specified is NUMERIC(38, 9)
 			return typing.EDecimal, nil
 		}
-		return typing.ParseNumeric(parameters), nil
+		return typing.ParseNumeric(parameters)
 	case "decimal", "float", "float64", "bigdecimal":
 		return typing.Float, nil
 	case "int", "integer", "int64":

diff --git a/clients/bigquery/dialect/dialect_test.go b/clients/bigquery/dialect/dialect_test.go
@@ -36,6 +36,12 @@ func TestBigQueryDialect_KindForDataType(t *testing.T) {
 	dialect := BigQueryDialect{}
 	{
 		// Numeric
+		{
+			// Invalid
+			kd, err := dialect.KindForDataType("numeric(1,2,3)", "")
+			assert.ErrorContains(t, err, "invalid number of parts: 3")
+			assert.Equal(t, typing.Invalid, kd)
+		}
 		{
 			// Numeric(5)
 			kd, err := dialect.KindForDataType("NUMERIC(5)", "")
@@ -91,10 +97,9 @@ func TestBigQueryDialect_KindForDataType(t *testing.T) {
 		"time":      typing.NewKindDetailsFromTemplate(typing.ETime, ext.TimeKindType),
 		"date":      typing.NewKindDetailsFromTemplate(typing.ETime, ext.DateKindType),
 		//Invalid
-		"foo":            typing.Invalid,
-		"foofoo":         typing.Invalid,
-		"":               typing.Invalid,
-		"numeric(1,2,3)": typing.Invalid,
+		"foo":    typing.Invalid,
+		"foofoo": typing.Invalid,
+		"":       typing.Invalid,
 	}
 
 	for bqCol, expectedKind := range bqColToExpectedKind {

diff --git a/clients/databricks/dialect/typing.go b/clients/databricks/dialect/typing.go
@@ -48,7 +48,7 @@ func (DatabricksDialect) KindForDataType(rawType string, _ string) (typing.KindD
 		if err != nil {
 			return typing.Invalid, err
 		}
-		return typing.ParseNumeric(parameters), nil
+		return typing.ParseNumeric(parameters)
 	}
 
 	if strings.HasPrefix(rawType, "array") {

diff --git a/clients/mssql/dialect/dialect.go b/clients/mssql/dialect/dialect.go
@@ -78,7 +78,7 @@ func (MSSQLDialect) KindForDataType(rawType string, stringPrecision string) (typ
 		if err != nil {
 			return typing.Invalid, err
 		}
-		return typing.ParseNumeric(parameters), nil
+		return typing.ParseNumeric(parameters)
 	}
 
 	switch rawType {

diff --git a/clients/redshift/dialect/dialect.go b/clients/redshift/dialect/dialect.go
@@ -81,7 +81,7 @@ func (RedshiftDialect) KindForDataType(rawType string, stringPrecision string) (
 		if err != nil {
 			return typing.Invalid, err
 		}
-		return typing.ParseNumeric(parameters), nil
+		return typing.ParseNumeric(parameters)
 	}
 
 	if strings.Contains(rawType, "character varying") {

diff --git a/clients/snowflake/dialect/dialect.go b/clients/snowflake/dialect/dialect.go
@@ -68,7 +68,7 @@ func (SnowflakeDialect) KindForDataType(snowflakeType string, _ string) (typing.
 	// Geography, geometry date, time, varbinary, binary are currently not supported.
 	switch dataType {
 	case "number", "numeric":
-		return typing.ParseNumeric(parameters), nil
+		return typing.ParseNumeric(parameters)
 	case "decimal":
 		return typing.EDecimal, nil
 	case "float", "float4",

diff --git a/clients/snowflake/dialect/dialect_test.go b/clients/snowflake/dialect/dialect_test.go
@@ -108,7 +108,7 @@ func TestSnowflakeDialect_KindForDataType(t *testing.T) {
 		{
 			// Invalid because precision nor scale is included.
 			kd, err := SnowflakeDialect{}.KindForDataType("NUMERIC", "")
-			assert.NoError(t, err)
+			assert.ErrorContains(t, err, "invalid number of parts: 0")
 			assert.Equal(t, typing.Invalid, kd)
 		}
 		{

diff --git a/lib/typing/numeric.go b/lib/typing/numeric.go
@@ -1,32 +1,38 @@
 package typing
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 
 	"github.com/artie-labs/transfer/lib/typing/decimal"
 )
 
-// TODO: This function should return an error
-func ParseNumeric(parts []string) KindDetails {
+func ParseNumeric(parts []string) (KindDetails, error) {
 	if len(parts) == 0 || len(parts) > 2 {
-		return Invalid
+		return Invalid, fmt.Errorf("invalid number of parts: %d", len(parts))
 	}
 
 	var parsedNumbers []int32
 	for _, part := range parts {
 		parsedNumber, err := strconv.ParseInt(strings.TrimSpace(part), 10, 32)
 		if err != nil {
-			return Invalid
+			return Invalid, fmt.Errorf("failed to parse number: %w", err)
 		}
 
 		parsedNumbers = append(parsedNumbers, int32(parsedNumber))
 	}
 
 	// If scale is 0 or not specified, then number is an int.
 	if len(parsedNumbers) == 1 || parsedNumbers[1] == 0 {
-		return NewDecimalDetailsFromTemplate(EDecimal, decimal.NewDetails(parsedNumbers[0], 0))
+		return NewDecimalDetailsFromTemplate(
+			EDecimal,
+			decimal.NewDetails(parsedNumbers[0], 0),
+		), nil
 	}
 
-	return NewDecimalDetailsFromTemplate(EDecimal, decimal.NewDetails(parsedNumbers[0], parsedNumbers[1]))
+	return NewDecimalDetailsFromTemplate(
+		EDecimal,
+		decimal.NewDetails(parsedNumbers[0], parsedNumbers[1]),
+	), nil
 }
diff --git a/lib/typing/numeric_test.go b/lib/typing/numeric_test.go
@@ -12,53 +12,70 @@ func TestParseNumeric(t *testing.T) {
 	{
 		// Invalid
 		{
-			result := ParseNumeric([]string{})
+			result, err := ParseNumeric([]string{})
+			assert.ErrorContains(t, err, "invalid number of parts: 0")
 			assert.Equal(t, Invalid, result)
 		}
 		{
-			result := ParseNumeric([]string{"5", "a"})
+			result, err := ParseNumeric([]string{"5", "a"})
+			assert.ErrorContains(t, err, `failed to parse number: strconv.ParseInt: parsing "a": invalid syntax`)
 			assert.Equal(t, Invalid, result)
 		}
 		{
-			result := ParseNumeric([]string{"b", "5"})
+			result, err := ParseNumeric([]string{"b", "5"})
+			assert.ErrorContains(t, err, `failed to parse number: strconv.ParseInt: parsing "b": invalid syntax`)
 			assert.Equal(t, Invalid, result)
 		}
 		{
-			result := ParseNumeric([]string{"a", "b"})
+			result, err := ParseNumeric([]string{"a", "b"})
+			assert.ErrorContains(t, err, `failed to parse number: strconv.ParseInt: parsing "a"`)
 			assert.Equal(t, Invalid, result)
 		}
 		{
-			result := ParseNumeric([]string{"1", "2", "3"})
+			result, err := ParseNumeric([]string{"1", "2", "3"})
+			assert.ErrorContains(t, err, `invalid number of parts: 3`)
 			assert.Equal(t, Invalid, result)
 		}
 		{
 			// Test values that are larger than [math.MaxInt32].
-			assert.Equal(t, Invalid, ParseNumeric([]string{"10", fmt.Sprint(math.MaxInt32 + 1)}))
-			assert.Equal(t, Invalid, ParseNumeric([]string{fmt.Sprint(math.MaxInt32 + 1), "10"}))
+			{
+				result, err := ParseNumeric([]string{"10", fmt.Sprint(math.MaxInt32 + 1)})
+				assert.Equal(t, Invalid, result)
+				assert.ErrorContains(t, err, `failed to parse number: strconv.ParseInt: parsing "2147483648": value out of range`)
+			}
+			{
+				result, err := ParseNumeric([]string{fmt.Sprint(math.MaxInt32 + 1), "10"})
+				assert.Equal(t, Invalid, result)
+				assert.ErrorContains(t, err, `failed to parse number: strconv.ParseInt: parsing "2147483648": value out of range`)
+			}
 		}
 	}
 	{
 		// Decimals
 		{
-			result := ParseNumeric([]string{"5", "2"})
+			result, err := ParseNumeric([]string{"5", "2"})
+			assert.NoError(t, err)
 			assert.Equal(t, EDecimal.Kind, result.Kind)
 			assert.Equal(t, int32(5), result.ExtendedDecimalDetails.Precision())
 			assert.Equal(t, int32(2), result.ExtendedDecimalDetails.Scale())
 		}
 		{
-			result := ParseNumeric([]string{"5", "  2     "})
+			result, err := ParseNumeric([]string{"5", "  2     "})
+			assert.NoError(t, err)
 			assert.Equal(t, EDecimal.Kind, result.Kind)
 			assert.Equal(t, int32(5), result.ExtendedDecimalDetails.Precision())
 			assert.Equal(t, int32(2), result.ExtendedDecimalDetails.Scale())
 		}
 		{
-			result := ParseNumeric([]string{"39", "6"})
+			result, err := ParseNumeric([]string{"39", "6"})
+			assert.NoError(t, err)
 			assert.Equal(t, EDecimal.Kind, result.Kind)
 			assert.Equal(t, int32(39), result.ExtendedDecimalDetails.Precision())
 			assert.Equal(t, int32(6), result.ExtendedDecimalDetails.Scale())
 		}
 		{
-			result := ParseNumeric([]string{fmt.Sprint(math.MaxInt32), fmt.Sprint(math.MaxInt32)})
+			result, err := ParseNumeric([]string{fmt.Sprint(math.MaxInt32), fmt.Sprint(math.MaxInt32)})
+			assert.NoError(t, err)
 			assert.Equal(t, EDecimal.Kind, result.Kind)
 			assert.Equal(t, int32(math.MaxInt32), result.ExtendedDecimalDetails.Precision())
 			assert.Equal(t, int32(math.MaxInt32), result.ExtendedDecimalDetails.Scale())
@@ -67,13 +84,15 @@ func TestParseNumeric(t *testing.T) {
 	{
 		// Integer
 		{
-			result := ParseNumeric([]string{"5"})
+			result, err := ParseNumeric([]string{"5"})
+			assert.NoError(t, err)
 			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"})
+			result, err := ParseNumeric([]string{"5", "0"})
+			assert.NoError(t, err)
 			assert.Equal(t, EDecimal.Kind, result.Kind)
 			assert.Equal(t, int32(5), result.ExtendedDecimalDetails.Precision())
 			assert.Equal(t, int32(0), result.ExtendedDecimalDetails.Scale())