From f553ff62be7870b262a132c0823a2fae56d44303 Mon Sep 17 00:00:00 2001 From: Nathan <148575555+nathan-artie@users.noreply.github.com> Date: Wed, 26 Jun 2024 17:18:05 -0700 Subject: [PATCH] [typing] Kill `Decimal.Value` method --- clients/shared/utils.go | 3 +- lib/cdc/util/relational_event_decimal_test.go | 31 ++++++------------- lib/debezium/types_test.go | 23 +------------- lib/typing/decimal/decimal.go | 19 ------------ 4 files changed, 11 insertions(+), 65 deletions(-) diff --git a/clients/shared/utils.go b/clients/shared/utils.go index 1b7022053..e80e17a76 100644 --- a/clients/shared/utils.go +++ b/clients/shared/utils.go @@ -44,8 +44,7 @@ func DefaultValue(column columns.Column, dialect sql.Dialect, additionalDateFmts return nil, fmt.Errorf("colVal is not type *decimal.Decimal") } - // TODO: Call [String] instead. - return val.Value(), nil + return val.String(), nil case typing.String.Kind: return sql.QuoteLiteral(fmt.Sprint(column.DefaultValue())), nil } diff --git a/lib/cdc/util/relational_event_decimal_test.go b/lib/cdc/util/relational_event_decimal_test.go index ad287190e..590050c79 100644 --- a/lib/cdc/util/relational_event_decimal_test.go +++ b/lib/cdc/util/relational_event_decimal_test.go @@ -3,7 +3,6 @@ package util import ( "encoding/json" "io" - "math/big" "os" "testing" @@ -47,8 +46,8 @@ func TestSchemaEventPayload_Numeric_GetData(t *testing.T) { retMap, err := schemaEventPayload.GetData(nil, &kafkalib.TopicConfig{}) assert.NoError(t, err) - assert.Equal(t, "123456.789", retMap["numeric_test"].(*decimal.Decimal).Value()) - assert.Equal(t, 0, big.NewFloat(1234).Cmp(retMap["numeric_5"].(*decimal.Decimal).Value().(*big.Float))) + assert.Equal(t, "123456.789", retMap["numeric_test"].(*decimal.Decimal).String()) + assert.Equal(t, "1234", retMap["numeric_5"].(*decimal.Decimal).String()) numericWithScaleMap := map[string]string{ "numeric_5_2": "568.01", "numeric_5_6": "0.023456", @@ -56,16 +55,12 @@ func TestSchemaEventPayload_Numeric_GetData(t *testing.T) { } for key, expectedValue := range numericWithScaleMap { - // Numeric data types that actually have scale fails when comparing *big.Float using `.Cmp`, so we are using STRING() instead. - _, isOk := retMap[key].(*decimal.Decimal).Value().(*big.Float) - assert.True(t, isOk) - // Now, we know the data type is *big.Float, let's check the .String() value. assert.Equal(t, expectedValue, retMap[key].(*decimal.Decimal).String()) } - assert.Equal(t, "58569102859845154622791691858438258688", retMap["numeric_39_0"].(*decimal.Decimal).Value()) - assert.Equal(t, "5856910285984515462279169185843825868.22", retMap["numeric_39_2"].(*decimal.Decimal).Value()) - assert.Equal(t, "585691028598451546227958438258688.123456", retMap["numeric_39_6"].(*decimal.Decimal).Value()) + assert.Equal(t, "58569102859845154622791691858438258688", retMap["numeric_39_0"].(*decimal.Decimal).String()) + assert.Equal(t, "5856910285984515462279169185843825868.22", retMap["numeric_39_2"].(*decimal.Decimal).String()) + assert.Equal(t, "585691028598451546227958438258688.123456", retMap["numeric_39_6"].(*decimal.Decimal).String()) } func TestSchemaEventPayload_Decimal_GetData(t *testing.T) { @@ -79,23 +74,19 @@ func TestSchemaEventPayload_Decimal_GetData(t *testing.T) { assert.NoError(t, err) retMap, err := schemaEventPayload.GetData(nil, &kafkalib.TopicConfig{}) assert.NoError(t, err) - assert.Equal(t, "123.45", retMap["decimal_test"].(*decimal.Decimal).Value()) + assert.Equal(t, "123.45", retMap["decimal_test"].(*decimal.Decimal).String()) decimalWithScaleMap := map[string]string{ "decimal_test_5": "123", "decimal_test_5_2": "123.45", } for key, expectedValue := range decimalWithScaleMap { - // Numeric data types that actually have scale fails when comparing *big.Float using `.Cmp`, so we are using STRING() instead. - _, isOk := retMap[key].(*decimal.Decimal).Value().(*big.Float) - assert.True(t, isOk) - // Now, we know the data type is *big.Float, let's check the .String() value. assert.Equal(t, expectedValue, retMap[key].(*decimal.Decimal).String(), key) } - assert.Equal(t, "58569102859845154622791691858438258688", retMap["decimal_test_39"].(*decimal.Decimal).Value(), "decimal_test_39") - assert.Equal(t, "585691028598451546227916918584382586.22", retMap["decimal_test_39_2"].(*decimal.Decimal).Value(), "decimal_test_39_2") - assert.Equal(t, "585691028598451546227916918584388.123456", retMap["decimal_test_39_6"].(*decimal.Decimal).Value(), "decimal_test_39_6") + assert.Equal(t, "58569102859845154622791691858438258688", retMap["decimal_test_39"].(*decimal.Decimal).String(), "decimal_test_39") + assert.Equal(t, "585691028598451546227916918584382586.22", retMap["decimal_test_39_2"].(*decimal.Decimal).String(), "decimal_test_39_2") + assert.Equal(t, "585691028598451546227916918584388.123456", retMap["decimal_test_39_6"].(*decimal.Decimal).String(), "decimal_test_39_6") } func TestSchemaEventPayload_Money_GetData(t *testing.T) { @@ -115,10 +106,6 @@ func TestSchemaEventPayload_Money_GetData(t *testing.T) { } for key, expectedValue := range decimalWithScaleMap { - // Numeric data types that actually have scale fails when comparing *big.Float using `.Cmp`, so we are using STRING() instead. - _, isOk := retMap[key].(*decimal.Decimal).Value().(*big.Float) - assert.True(t, isOk) - // Now, we know the data type is *big.Float, let's check the .String() value. assert.Equal(t, expectedValue, retMap[key].(*decimal.Decimal).String(), key) } } diff --git a/lib/debezium/types_test.go b/lib/debezium/types_test.go index 63dfc3e48..8b02c1f73 100644 --- a/lib/debezium/types_test.go +++ b/lib/debezium/types_test.go @@ -1,7 +1,6 @@ package debezium import ( - "math/big" "testing" "time" @@ -450,7 +449,6 @@ func TestField_DecodeDecimal(t *testing.T) { expectedPrecision int32 expectNilPtrPrecision bool expectedScale int32 - expectBigFloat bool expectedErr string }{ { @@ -482,7 +480,6 @@ func TestField_DecodeDecimal(t *testing.T) { expectedValue: "5", expectedPrecision: 5, expectedScale: 0, - expectBigFloat: true, }, { name: "NUMERIC(5,2)", @@ -494,7 +491,6 @@ func TestField_DecodeDecimal(t *testing.T) { expectedValue: "578.01", expectedPrecision: 5, expectedScale: 2, - expectBigFloat: true, }, { name: "NUMERIC(38, 0) - small #", @@ -503,7 +499,6 @@ func TestField_DecodeDecimal(t *testing.T) { "scale": "0", "connect.decimal.precision": "38", }, - expectBigFloat: true, expectedValue: "567", expectedPrecision: 38, expectedScale: 0, @@ -515,7 +510,6 @@ func TestField_DecodeDecimal(t *testing.T) { "scale": "0", "connect.decimal.precision": "38", }, - expectBigFloat: true, expectedValue: "99999999999999999999999999999999999999", expectedPrecision: 38, expectedScale: 0, @@ -527,7 +521,6 @@ func TestField_DecodeDecimal(t *testing.T) { "scale": "2", "connect.decimal.precision": "38", }, - expectBigFloat: true, expectedValue: "33.21", expectedPrecision: 38, expectedScale: 2, @@ -539,7 +532,6 @@ func TestField_DecodeDecimal(t *testing.T) { "scale": "2", "connect.decimal.precision": "38", }, - expectBigFloat: true, expectedValue: "9999999999999999999999999999999999.99", expectedPrecision: 38, expectedScale: 2, @@ -551,7 +543,6 @@ func TestField_DecodeDecimal(t *testing.T) { "scale": "4", "connect.decimal.precision": "38", }, - expectBigFloat: true, expectedValue: "484.4419", expectedPrecision: 38, expectedScale: 4, @@ -563,7 +554,6 @@ func TestField_DecodeDecimal(t *testing.T) { "scale": "4", "connect.decimal.precision": "38", }, - expectBigFloat: true, expectedValue: "999999999999999999999999999999.9999", expectedPrecision: 38, expectedScale: 4, @@ -596,7 +586,6 @@ func TestField_DecodeDecimal(t *testing.T) { params: map[string]any{ "scale": "2", }, - expectBigFloat: true, expectedValue: "123456.98", expectNilPtrPrecision: true, expectedScale: 2, @@ -618,9 +607,6 @@ func TestField_DecodeDecimal(t *testing.T) { } assert.NoError(t, err) - decVal := dec.Value() - _, isOk := decVal.(*big.Float) - assert.Equal(t, testCase.expectBigFloat, isOk, testCase.name) assert.Equal(t, testCase.expectedValue, dec.String(), testCase.name) if testCase.expectNilPtrPrecision { @@ -713,16 +699,9 @@ func TestField_DecodeDebeziumVariableDecimal(t *testing.T) { continue } - // It should never be a *big.Float - _, isOk := dec.Value().(*big.Float) - assert.False(t, isOk, testCase.name) - - // It should be a string instead. - _, isOk = dec.Value().(string) - assert.True(t, isOk, testCase.name) assert.Equal(t, int32(-1), *dec.Precision(), testCase.name) assert.Equal(t, testCase.expectedScale, dec.Scale(), testCase.name) - assert.Equal(t, testCase.expectedValue, dec.Value(), testCase.name) + assert.Equal(t, testCase.expectedValue, dec.String(), testCase.name) } } diff --git a/lib/typing/decimal/decimal.go b/lib/typing/decimal/decimal.go index 6314b86c4..6fae8c9b6 100644 --- a/lib/typing/decimal/decimal.go +++ b/lib/typing/decimal/decimal.go @@ -1,9 +1,6 @@ package decimal import ( - "log/slog" - "math/big" - "github.com/artie-labs/transfer/lib/ptr" "github.com/cockroachdb/apd/v3" ) @@ -55,22 +52,6 @@ func (d *Decimal) String() string { return d.value.Text('f') } -func (d *Decimal) Value() any { - // -1 precision is used for variable scaled decimal - // We are opting to emit this as a STRING because the value is technically unbounded (can get to ~1 GB). - if d.precision != nil && (*d.precision > MaxPrecisionBeforeString || *d.precision == PrecisionNotSpecified) { - return d.String() - } - - // Depending on the precision, we will want to convert value to STRING or keep as a FLOAT. - // TODO: [Value] is only called in one place, look into calling [String] instead. - if out, ok := new(big.Float).SetString(d.String()); ok { - return out - } - slog.Error("Failed to convert apd.Decimal to big.Float", slog.String("value", d.String())) - return d.String() -} - func (d *Decimal) Details() DecimalDetails { precision := PrecisionNotSpecified if d.precision != nil {