Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[typing] Remove pointer for DecimalDetails precision #771

Merged
merged 3 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clients/bigquery/dialect/dialect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,14 @@ func TestBigQueryDialect_KindForDataType(t *testing.T) {
kd, err := dialect.KindForDataType("numeric(5, 2)", "")
assert.NoError(t, err)
assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
assert.Equal(t, int32(5), *kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(5), kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), kd.ExtendedDecimalDetails.Scale())
}
{
kd, err := dialect.KindForDataType("bignumeric(5, 2)", "")
assert.NoError(t, err)
assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
assert.Equal(t, int32(5), *kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(5), kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), kd.ExtendedDecimalDetails.Scale())
}
}
Expand Down
2 changes: 1 addition & 1 deletion clients/mssql/dialect/dialect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestMSSQLDialect_KindForDataType(t *testing.T) {
kd, err := dialect.KindForDataType("numeric(5, 2)", "")
assert.NoError(t, err)
assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
assert.Equal(t, int32(5), *kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(5), kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), kd.ExtendedDecimalDetails.Scale())
}
{
Expand Down
2 changes: 1 addition & 1 deletion clients/redshift/dialect/dialect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestRedshiftDialect_KindForDataType(t *testing.T) {
kd, err := dialect.KindForDataType("numeric(5,2)", "")
assert.NoError(t, err)
assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
assert.Equal(t, int32(5), *kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(5), kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), kd.ExtendedDecimalDetails.Scale())
}
}
Expand Down
4 changes: 2 additions & 2 deletions clients/snowflake/dialect/dialect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ func TestSnowflakeDialect_KindForDataType_Floats(t *testing.T) {
kd, err := SnowflakeDialect{}.KindForDataType("NUMERIC(38, 2)", "")
assert.NoError(t, err)
assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
assert.Equal(t, int32(38), *kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(38), kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), kd.ExtendedDecimalDetails.Scale())
}
{
kd, err := SnowflakeDialect{}.KindForDataType("NUMBER(38, 2)", "")
assert.NoError(t, err)
assert.Equal(t, typing.EDecimal.Kind, kd.Kind)
assert.Equal(t, int32(38), *kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(38), kd.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), kd.ExtendedDecimalDetails.Scale())
}
{
Expand Down
19 changes: 9 additions & 10 deletions lib/cdc/util/optional_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/stretchr/testify/assert"

"github.com/artie-labs/transfer/lib/ptr"
"github.com/artie-labs/transfer/lib/typing"
"github.com/artie-labs/transfer/lib/typing/decimal"
)
Expand Down Expand Up @@ -67,35 +66,35 @@ func TestGetOptionalSchema(t *testing.T) {
"bit_test": typing.Boolean,
"numeric_test": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(decimal.PrecisionNotSpecified), decimal.DefaultScale),
ExtendedDecimalDetails: decimal.NewDecimalDetails(decimal.PrecisionNotSpecified, decimal.DefaultScale),
},
"numeric_5": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(5), 0),
ExtendedDecimalDetails: decimal.NewDecimalDetails(5, 0),
},
"numeric_5_2": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(5), 2),
ExtendedDecimalDetails: decimal.NewDecimalDetails(5, 2),
},
"numeric_5_6": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(5), 6),
ExtendedDecimalDetails: decimal.NewDecimalDetails(5, 6),
},
"numeric_5_0": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(5), 0),
ExtendedDecimalDetails: decimal.NewDecimalDetails(5, 0),
},
"numeric_39_0": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(39), 0),
ExtendedDecimalDetails: decimal.NewDecimalDetails(39, 0),
},
"numeric_39_2": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(39), 2),
ExtendedDecimalDetails: decimal.NewDecimalDetails(39, 2),
},
"numeric_39_6": {
Kind: typing.EDecimal.Kind,
ExtendedDecimalDetails: decimal.NewDecimalDetails(ptr.ToInt32(39), 6),
ExtendedDecimalDetails: decimal.NewDecimalDetails(39, 6),
},
},
},
Expand All @@ -116,7 +115,7 @@ func TestGetOptionalSchema(t *testing.T) {
if expectedValue.ExtendedDecimalDetails != nil || actualVal.ExtendedDecimalDetails != nil {
assert.NotNil(t, actualVal.ExtendedDecimalDetails, testMsg)
assert.Equal(t, expectedValue.ExtendedDecimalDetails.Scale(), actualVal.ExtendedDecimalDetails.Scale(), testMsg)
assert.Equal(t, *expectedValue.ExtendedDecimalDetails.Precision(), *actualVal.ExtendedDecimalDetails.Precision(), testMsg)
assert.Equal(t, expectedValue.ExtendedDecimalDetails.Precision(), actualVal.ExtendedDecimalDetails.Precision(), testMsg)
} else {
assert.Nil(t, actualVal.ExtendedDecimalDetails, testMsg)
}
Expand Down
9 changes: 7 additions & 2 deletions lib/debezium/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,16 @@ func (f Field) ToKindDetails() typing.KindDetails {
case JSON, GeometryPointType, GeometryType, GeographyType:
return typing.Struct
case KafkaDecimalType:
scale, precision, err := f.GetScaleAndPrecision()
scale, precisionPtr, err := f.GetScaleAndPrecision()
if err != nil {
return typing.Invalid
}

var precision int32 = decimal.PrecisionNotSpecified
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

precision := decimal.PrecisionNotSpecified

if precisionPtr != nil {
precision = *precisionPtr
}

eDecimal := typing.EDecimal
eDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(precision, scale)
return eDecimal
Expand All @@ -106,7 +111,7 @@ func (f Field) ToKindDetails() typing.KindDetails {
// This is because scale is not specified at the column level, rather at the row level
// It shouldn't matter much anyway since the column type we are creating is `TEXT` to avoid boundary errors.
eDecimal := typing.EDecimal
eDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(ptr.ToInt32(decimal.PrecisionNotSpecified), decimal.DefaultScale)
eDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(decimal.PrecisionNotSpecified, decimal.DefaultScale)
return eDecimal
}

Expand Down
4 changes: 2 additions & 2 deletions lib/debezium/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ func TestField_ToKindDetails(t *testing.T) {
}

eDecimal := typing.EDecimal
eDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(ptr.ToInt32(decimal.PrecisionNotSpecified), decimal.DefaultScale)
eDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(decimal.PrecisionNotSpecified, decimal.DefaultScale)

kafkaDecimalType := typing.EDecimal
kafkaDecimalType.ExtendedDecimalDetails = decimal.NewDecimalDetails(ptr.ToInt32(10), 5)
kafkaDecimalType.ExtendedDecimalDetails = decimal.NewDecimalDetails(10, 5)

tcs := []_tc{
{
Expand Down
10 changes: 5 additions & 5 deletions lib/optimization/event_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestTableData_UpdateInMemoryColumnsFromDestination(t *testing.T) {
tableDataCols.AddColumn(columns.NewColumn("ext_dec", typing.String))

extDecimalType := typing.EDecimal
extDecimalType.ExtendedDecimalDetails = decimal.NewDecimalDetails(ptr.ToInt32(22), 2)
extDecimalType.ExtendedDecimalDetails = decimal.NewDecimalDetails(22, 2)
tableDataCols.AddColumn(columns.NewColumn("ext_dec_filled", extDecimalType))

tableDataCols.AddColumn(columns.NewColumn(strCol, typing.String))
Expand Down Expand Up @@ -121,30 +121,30 @@ func TestTableData_UpdateInMemoryColumnsFromDestination(t *testing.T) {
assert.Equal(t, typing.String, extDecCol.KindDetails)

extDecimal := typing.EDecimal
extDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(ptr.ToInt32(30), 2)
extDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(30, 2)
assert.NoError(t, tableData.MergeColumnsFromDestination(columns.NewColumn("ext_dec", extDecimal)))
// Now it should be ext decimal type
extDecCol, isOk = tableData.inMemoryColumns.GetColumn("ext_dec")
assert.True(t, isOk)
assert.Equal(t, typing.EDecimal.Kind, extDecCol.KindDetails.Kind)
// Check precision and scale too.
assert.Equal(t, int32(30), *extDecCol.KindDetails.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(30), extDecCol.KindDetails.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), extDecCol.KindDetails.ExtendedDecimalDetails.Scale())

// Testing ext_dec_filled since it's already filled out
extDecColFilled, isOk := tableData.inMemoryColumns.GetColumn("ext_dec_filled")
assert.True(t, isOk)
assert.Equal(t, typing.EDecimal.Kind, extDecColFilled.KindDetails.Kind)
// Check precision and scale too.
assert.Equal(t, int32(22), *extDecColFilled.KindDetails.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(22), extDecColFilled.KindDetails.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), extDecColFilled.KindDetails.ExtendedDecimalDetails.Scale())

assert.NoError(t, tableData.MergeColumnsFromDestination(columns.NewColumn("ext_dec_filled", extDecimal)))
extDecColFilled, isOk = tableData.inMemoryColumns.GetColumn("ext_dec_filled")
assert.True(t, isOk)
assert.Equal(t, typing.EDecimal.Kind, extDecColFilled.KindDetails.Kind)
// Check precision and scale too.
assert.Equal(t, int32(22), *extDecColFilled.KindDetails.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(22), extDecColFilled.KindDetails.ExtendedDecimalDetails.Precision())
assert.Equal(t, int32(2), extDecColFilled.KindDetails.ExtendedDecimalDetails.Scale())
}
{
Expand Down
2 changes: 1 addition & 1 deletion lib/parquetutil/parse_values_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

func TestParseValue(t *testing.T) {
eDecimal := typing.EDecimal
eDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(ptr.ToInt32(30), 5)
eDecimal.ExtendedDecimalDetails = decimal.NewDecimalDetails(30, 5)

eTime := typing.ETime
eTime.ExtendedTimeDetails = &ext.Time
Expand Down
17 changes: 6 additions & 11 deletions lib/typing/decimal/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

func (d *DecimalDetails) isNumeric() bool {
if d.precision == nil || *d.precision == PrecisionNotSpecified {
if d.precision == PrecisionNotSpecified {
return false
}

Expand All @@ -17,11 +17,11 @@ func (d *DecimalDetails) isNumeric() bool {
}

// max(1,s) <= p <= s + 29
return numbers.BetweenEq(max(1, d.scale), d.scale+29, *d.precision)
return numbers.BetweenEq(max(1, d.scale), d.scale+29, d.precision)
}

func (d *DecimalDetails) isBigNumeric() bool {
if d.precision == nil || *d.precision == -1 {
if d.precision == PrecisionNotSpecified {
return false
}

Expand All @@ -31,18 +31,13 @@ func (d *DecimalDetails) isBigNumeric() bool {
}

// max(1,s) <= p <= s + 38
return numbers.BetweenEq(max(1, d.scale), d.scale+38, *d.precision)
return numbers.BetweenEq(max(1, d.scale), d.scale+38, d.precision)
}

func (d *DecimalDetails) toKind(maxPrecision int32, exceededKind string) string {
precision := maxPrecision
Copy link
Contributor Author

@nathan-artie nathan-artie Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a subtle change here in that before if d.precision was nil we would use the maxPrecision and now we'll treat it the same as -1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fine

if d.precision != nil {
precision = *d.precision
}

if precision > maxPrecision || precision == -1 {
if d.precision > maxPrecision || d.precision == PrecisionNotSpecified {
return exceededKind
}

return fmt.Sprintf("NUMERIC(%v, %v)", precision, d.scale)
return fmt.Sprintf("NUMERIC(%v, %v)", d.precision, d.scale)
}
6 changes: 5 additions & 1 deletion lib/typing/decimal/decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,9 @@ func (d *Decimal) Value() any {
}

func (d *Decimal) Details() DecimalDetails {
return DecimalDetails{scale: d.Scale(), precision: d.precision}
var precision int32 = PrecisionNotSpecified
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

precision := PrecisionNotSpecified

if d.precision != nil {
precision = *d.precision
}
return DecimalDetails{scale: d.Scale(), precision: precision}
}
38 changes: 19 additions & 19 deletions lib/typing/decimal/decimal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ func TestNewDecimal(t *testing.T) {
// Nil precision:
assert.Equal(t, "0", NewDecimal(nil, numbers.MustParseDecimal("0")).String())
// Precision = -1 (PrecisionNotSpecified):
assert.Equal(t, DecimalDetails{scale: 2, precision: ptr.ToInt32(-1)}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("12.34")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: -1}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("12.34")).Details())
// Precision = scale:
assert.Equal(t, DecimalDetails{scale: 2, precision: ptr.ToInt32(2)}, NewDecimal(ptr.ToInt32(2), numbers.MustParseDecimal("12.34")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: 2}, NewDecimal(ptr.ToInt32(2), numbers.MustParseDecimal("12.34")).Details())
// Precision < scale:
assert.Equal(t, DecimalDetails{scale: 2, precision: ptr.ToInt32(3)}, NewDecimal(ptr.ToInt32(1), numbers.MustParseDecimal("12.34")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: 3}, NewDecimal(ptr.ToInt32(1), numbers.MustParseDecimal("12.34")).Details())
// Precision > scale:
assert.Equal(t, DecimalDetails{scale: 2, precision: ptr.ToInt32(4)}, NewDecimal(ptr.ToInt32(4), numbers.MustParseDecimal("12.34")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: 4}, NewDecimal(ptr.ToInt32(4), numbers.MustParseDecimal("12.34")).Details())
}

func TestDecimal_Scale(t *testing.T) {
Expand All @@ -33,23 +33,23 @@ func TestDecimal_Scale(t *testing.T) {

func TestDecimal_Details(t *testing.T) {
// Nil precision:
assert.Equal(t, DecimalDetails{scale: 0}, NewDecimal(nil, numbers.MustParseDecimal("0")).Details())
assert.Equal(t, DecimalDetails{scale: 0}, NewDecimal(nil, numbers.MustParseDecimal("12345")).Details())
assert.Equal(t, DecimalDetails{scale: 0}, NewDecimal(nil, numbers.MustParseDecimal("-12")).Details())
assert.Equal(t, DecimalDetails{scale: 2}, NewDecimal(nil, numbers.MustParseDecimal("12345.12")).Details())
assert.Equal(t, DecimalDetails{scale: 3}, NewDecimal(nil, numbers.MustParseDecimal("-12345.123")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: -1}, NewDecimal(nil, numbers.MustParseDecimal("0")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: -1}, NewDecimal(nil, numbers.MustParseDecimal("12345")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: -1}, NewDecimal(nil, numbers.MustParseDecimal("-12")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: -1}, NewDecimal(nil, numbers.MustParseDecimal("12345.12")).Details())
assert.Equal(t, DecimalDetails{scale: 3, precision: -1}, NewDecimal(nil, numbers.MustParseDecimal("-12345.123")).Details())

// -1 precision (PrecisionNotSpecified):
assert.Equal(t, DecimalDetails{scale: 0, precision: ptr.ToInt32(-1)}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("0")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: ptr.ToInt32(-1)}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("12345")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: ptr.ToInt32(-1)}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("-12")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: ptr.ToInt32(-1)}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("12345.12")).Details())
assert.Equal(t, DecimalDetails{scale: 3, precision: ptr.ToInt32(-1)}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("-12345.123")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: -1}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("0")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: -1}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("12345")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: -1}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("-12")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: -1}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("12345.12")).Details())
assert.Equal(t, DecimalDetails{scale: 3, precision: -1}, NewDecimal(ptr.ToInt32(-1), numbers.MustParseDecimal("-12345.123")).Details())

// 10 precision:
assert.Equal(t, DecimalDetails{scale: 0, precision: ptr.ToInt32(10)}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("0")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: ptr.ToInt32(10)}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("12345")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: ptr.ToInt32(10)}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("-12")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: ptr.ToInt32(10)}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("12345.12")).Details())
assert.Equal(t, DecimalDetails{scale: 3, precision: ptr.ToInt32(10)}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("-12345.123")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: 10}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("0")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: 10}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("12345")).Details())
assert.Equal(t, DecimalDetails{scale: 0, precision: 10}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("-12")).Details())
assert.Equal(t, DecimalDetails{scale: 2, precision: 10}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("12345.12")).Details())
assert.Equal(t, DecimalDetails{scale: 3, precision: 10}, NewDecimal(ptr.ToInt32(10), numbers.MustParseDecimal("-12345.123")).Details())
}
24 changes: 10 additions & 14 deletions lib/typing/decimal/details.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,20 @@ package decimal

import (
"fmt"

"github.com/artie-labs/transfer/lib/ptr"
)

type DecimalDetails struct {
scale int32
precision *int32
precision int32
}

func NewDecimalDetails(precision *int32, scale int32) *DecimalDetails {
if precision != nil {
if scale > *precision && *precision != -1 {
// Note: -1 precision means it's not specified.
func NewDecimalDetails(precision int32, scale int32) *DecimalDetails {
if scale > precision && precision != -1 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use constant PrecisionNotSpecified instead of -1

// Note: -1 precision means it's not specified.

// This is typically not possible, but Postgres has a design flaw that allows you to do things like: NUMERIC(5, 6) which actually equates to NUMERIC(7, 6)
// We are setting precision to be scale + 1 to account for the leading zero for decimal numbers.
precision = ptr.ToInt32(scale + 1)
}
// This is typically not possible, but Postgres has a design flaw that allows you to do things like: NUMERIC(5, 6) which actually equates to NUMERIC(7, 6)
// We are setting precision to be scale + 1 to account for the leading zero for decimal numbers.
precision = scale + 1
}

return &DecimalDetails{
Expand All @@ -32,7 +28,7 @@ func (d DecimalDetails) Scale() int32 {
return d.scale
}

func (d DecimalDetails) Precision() *int32 {
func (d DecimalDetails) Precision() int32 {
return d.precision
}

Expand All @@ -55,9 +51,9 @@ func (d *DecimalDetails) RedshiftKind() string {
// BigQueryKind - is inferring logic from: https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
func (d *DecimalDetails) BigQueryKind() string {
if d.isNumeric() {
return fmt.Sprintf("NUMERIC(%v, %v)", *d.precision, d.scale)
return fmt.Sprintf("NUMERIC(%v, %v)", d.precision, d.scale)
} else if d.isBigNumeric() {
return fmt.Sprintf("BIGNUMERIC(%v, %v)", *d.precision, d.scale)
return fmt.Sprintf("BIGNUMERIC(%v, %v)", d.precision, d.scale)
}

return "STRING"
Expand Down
4 changes: 1 addition & 3 deletions lib/typing/decimal/details_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/artie-labs/transfer/lib/ptr"
)

func TestDecimalDetailsKind(t *testing.T) {
Expand Down Expand Up @@ -70,7 +68,7 @@ func TestDecimalDetailsKind(t *testing.T) {
}

for _, testCase := range testCases {
d := NewDecimalDetails(ptr.ToInt32(testCase.Precision), testCase.Scale)
d := NewDecimalDetails(testCase.Precision, testCase.Scale)
assert.Equal(t, testCase.ExpectedSnowflakeKind, d.SnowflakeKind(), testCase.Name)
assert.Equal(t, testCase.ExpectedRedshiftKind, d.RedshiftKind(), testCase.Name)
assert.Equal(t, testCase.ExpectedBigQueryKind, d.BigQueryKind(), testCase.Name)
Expand Down
2 changes: 1 addition & 1 deletion lib/typing/numeric.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ func ParseNumeric(parts []string) KindDetails {
}

eDec := EDecimal
eDec.ExtendedDecimalDetails = decimal.NewDecimalDetails(&parsedNumbers[0], parsedNumbers[1])
eDec.ExtendedDecimalDetails = decimal.NewDecimalDetails(parsedNumbers[0], parsedNumbers[1])
return eDec
}
Loading