Skip to content

Commit

Permalink
[typing] Kill Decimal.Value method
Browse files Browse the repository at this point in the history
  • Loading branch information
@nathan-artie
nathan-artie committed Jun 27, 2024
1 parent 1a5b566 commit f553ff6
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 65 deletions.
3 changes: 1 addition & 2 deletions clients/shared/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
31 changes: 9 additions & 22 deletions lib/cdc/util/relational_event_decimal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package util
import (
"encoding/json"
"io"
"math/big"
"os"
"testing"

Expand Down Expand Up @@ -47,25 +46,21 @@ 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",
"numeric_5_0": "5",
}

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) {
Expand All @@ -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) {
Expand All @@ -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)
}
}
23 changes: 1 addition & 22 deletions lib/debezium/types_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package debezium

import (
"math/big"
"testing"
"time"

Expand Down Expand Up @@ -450,7 +449,6 @@ func TestField_DecodeDecimal(t *testing.T) {
expectedPrecision int32
expectNilPtrPrecision bool
expectedScale int32
expectBigFloat bool
expectedErr string
}{
{
Expand Down Expand Up @@ -482,7 +480,6 @@ func TestField_DecodeDecimal(t *testing.T) {
expectedValue: "5",
expectedPrecision: 5,
expectedScale: 0,
expectBigFloat: true,
},
{
name: "NUMERIC(5,2)",
Expand All @@ -494,7 +491,6 @@ func TestField_DecodeDecimal(t *testing.T) {
expectedValue: "578.01",
expectedPrecision: 5,
expectedScale: 2,
expectBigFloat: true,
},
{
name: "NUMERIC(38, 0) - small #",
Expand All @@ -503,7 +499,6 @@ func TestField_DecodeDecimal(t *testing.T) {
"scale": "0",
"connect.decimal.precision": "38",
},
expectBigFloat: true,
expectedValue: "567",
expectedPrecision: 38,
expectedScale: 0,
Expand All @@ -515,7 +510,6 @@ func TestField_DecodeDecimal(t *testing.T) {
"scale": "0",
"connect.decimal.precision": "38",
},
expectBigFloat: true,
expectedValue: "99999999999999999999999999999999999999",
expectedPrecision: 38,
expectedScale: 0,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}

}
19 changes: 0 additions & 19 deletions lib/typing/decimal/decimal.go
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package decimal

import (
"log/slog"
"math/big"

"github.com/artie-labs/transfer/lib/ptr"
"github.com/cockroachdb/apd/v3"
)
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit f553ff6

Please sign in to comment.