Skip to content

Commit

Permalink
fix: minimum scale required for decimal literal (#117)
Browse files Browse the repository at this point in the history
Also reports the offending line number when the parser encounters a malformed testcase.
  • Loading branch information
scgkiran authored Feb 7, 2025
1 parent e1e6157 commit 00a91c9
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 64 deletions.
20 changes: 10 additions & 10 deletions expr/decimal_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,18 @@ func modifyDecimalPrecisionAndScale(decimalBytes [16]byte, scale, targetPrecisio
// Normalize the decimal by removing trailing zeros.
dec.Reduce(dec)

// Adjust the scale to the target scale
err2 := validatePrecisionAndScale(dec, targetPrecision, targetScale)
if err2 != nil {
return result, 0, 0, err2
}

// After ensuring the targetScale is sufficient for dec, adjust the scale to the target scale
ctx := apd.BaseContext.WithPrecision(uint32(targetPrecision))
_, err := ctx.Quantize(dec, dec, -targetScale)
if err != nil {
return result, 0, 0, fmt.Errorf("error adjusting scale: %v", err)
}

err2 := validatePrecisionAndScale(dec, targetPrecision, targetScale)
if err2 != nil {
return result, 0, 0, err2
}

// Convert the adjusted decimal coefficient to a byte array.
byteArray := dec.Coeff.Bytes()
if len(byteArray) > 16 {
Expand All @@ -166,12 +166,12 @@ func modifyDecimalPrecisionAndScale(decimalBytes [16]byte, scale, targetPrecisio
func validatePrecisionAndScale(dec *apd.Decimal, targetPrecision int32, targetScale int32) error {
// Validate the minimum precision and scale.
minPrecision, minScale := getMinimumPrecisionAndScale(dec)
if targetPrecision < minPrecision {
return fmt.Errorf("number %s exceeds target precision %d, minimum precision needed is %d with target scale %d", dec.String(), targetPrecision, minPrecision, targetScale)
}
if targetScale < minScale {
return fmt.Errorf("number %v exceeds target scale %d, minimum scale needed is %d", dec.String(), targetScale, minScale)
}
if targetPrecision < minPrecision {
return fmt.Errorf("number %s exceeds target precision %d, minimum precision needed is %d with target scale %d", dec.String(), targetPrecision, minPrecision, targetScale)
}
if targetPrecision-targetScale < minPrecision-minScale {
return fmt.Errorf("number %v exceeds target precision %d with target scale %d, minimum precision needed is %d with minimum scale %d", dec.String(), targetPrecision, targetScale, minPrecision, minScale)
}
Expand All @@ -184,7 +184,7 @@ func getMinimumPrecisionAndScale(dec *apd.Decimal) (precision int32, scale int32
scale = 0
} else {
scale = -dec.Exponent
precision = max(int32(apd.NumDigits(&dec.Coeff)), scale+1)
precision = max(int32(apd.NumDigits(&dec.Coeff)), scale)
}
return precision, scale
}
11 changes: 11 additions & 0 deletions expr/decimal_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ func TestDecimalStringToBytes(t *testing.T) {
{"-12345", "c7cfffffffffffffffffffffffffffff", 5, 0, ""},
{"123.45", "39300000000000000000000000000000", 5, 2, ""},
{"-123.45", "c7cfffffffffffffffffffffffffffff", 5, 2, ""},
{"4.53", "c5010000000000000000000000000000", 3, 2, ""},
{"0.5", "05000000000000000000000000000000", 2, 1, ""},
{"0.25", "19000000000000000000000000000000", 3, 2, ""},
{"1", "01000000000000000000000000000000", 1, 0, ""},
{"1.000", "e8030000000000000000000000000000", 4, 3, ""},
{"-1.0", "f6ffffffffffffffffffffffffffffff", 2, 1, ""},
{"-1.00", "9cffffffffffffffffffffffffffffff", 3, 2, ""},
{"-1.000", "18fcffffffffffffffffffffffffffff", 4, 3, ""},
Expand Down Expand Up @@ -195,9 +199,16 @@ func TestModifyDecimalPrecisionAndScale(t *testing.T) {
{"12345.6789", "15cd5b07000000000000000000000000", 9, 4, 13, 8, "5004fb711f0100000000000000000000", "12345.67890000", false},
{"-1.00", "9cffffffffffffffffffffffffffffff", 3, 2, 5, 3, "18fcffffffffffffffffffffffffffff", "-1.000", false},
{"-1.0", "f6ffffffffffffffffffffffffffffff", 2, 1, 2, 0, "ffffffffffffffffffffffffffffffff", "-1", false},
{"-1.0", "f6ffffffffffffffffffffffffffffff", 2, 1, 1, 0, "ffffffffffffffffffffffffffffffff", "-1", false},
{"1.0", "0a000000000000000000000000000000", 2, 1, 2, 0, "01000000000000000000000000000000", "1", false},
{"1.0", "0a000000000000000000000000000000", 2, 1, 1, 0, "01000000000000000000000000000000", "1", false},
{"0.25", "19000000000000000000000000000000", 3, 2, 2, 2, "", "", false},
{"0.5", "05000000000000000000000000000000", 2, 1, 1, 1, "", "", false},
{"1.0", "0a000000000000000000000000000000", 2, 1, 40, 0, "", "", true},
{"4.53", "c5010000000000000000000000000000", 3, 2, 1, 0, "", "", true},
{"0.25", "19000000000000000000000000000000", 3, 2, 1, 0, "", "", true},
{"1.0", "0a000000000000000000000000000000", 2, 1, 1, 0, "01000000000000000000000000000000", "1", false},
{"1.000", "e8030000000000000000000000000000", 4, 3, 1, 0, "01000000000000000000000000000000", "1", false},
{"1", "01000000000000000000000000000000", 1, 0, 3, 2, "64000000000000000000000000000000", "1.00", false},
{"9223372036854775807", "ffffffffffffff7f0000000000000000", 19, 0, 30, 4, "f0d8ffffffffffff8713000000000000", "9223372036854775807.0000", false},
{"-9223372036854775808", "0000000000000080ffffffffffffffff", 19, 0, 30, 2, "0000000000000000ceffffffffffffff", "-9223372036854775808.00", false},
Expand Down
14 changes: 13 additions & 1 deletion testcases/parser/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,14 @@ func TestParseDecimalExample(t *testing.T) {
power(8::dec<38,0>, 2::dec<38, 0>) = 64::fp64
power(1.0::dec<38, 5>, -1.0::dec<38, 5>) = 1.0::fp64
power(-1::dec, 0.5::dec<38,1>) [complex_number_result:NAN] = nan::fp64
add(0.5::dec<1, 1>, 0.25::dec<2, 2>) = 0.75::dec<5, 2>
`

testFile, err := ParseTestCasesFromString(header + tests)
require.NoError(t, err)
require.NotNil(t, testFile)
assert.Len(t, testFile.TestCases, 3)
assert.Len(t, testFile.TestCases, 4)
assert.Equal(t, "power", testFile.TestCases[0].FuncName)
assert.Equal(t, "power", testFile.TestCases[1].FuncName)
assert.Equal(t, "power", testFile.TestCases[2].FuncName)
Expand All @@ -136,6 +138,15 @@ power(-1::dec, 0.5::dec<38,1>) [complex_number_result:NAN] = nan::fp64
assert.Equal(t, decMinus1, testFile.TestCases[2].Args[0].Value)
assert.Equal(t, decPoint5, testFile.TestCases[2].Args[1].Value)
assert.Equal(t, "fp64(NaN)", testFile.TestCases[2].Result.Value.String())

decPoint25Value, _ := literal.NewDecimalFromString("0.25")
decPoint75Value, _ := literal.NewDecimalFromString("0.75")
decPoint25, _ := decPoint25Value.(expr.WithTypeLiteral).WithType(&types.DecimalType{Precision: 2, Scale: 2, Nullability: types.NullabilityRequired})
decPoint75, _ := decPoint75Value.(expr.WithTypeLiteral).WithType(&types.DecimalType{Precision: 5, Scale: 2, Nullability: types.NullabilityRequired})
decPoint5, _ = decPoint5Value.(expr.WithTypeLiteral).WithType(&types.DecimalType{Precision: 1, Scale: 1, Nullability: types.NullabilityRequired})
assert.Equal(t, decPoint5, testFile.TestCases[3].Args[0].Value)
assert.Equal(t, decPoint25, testFile.TestCases[3].Args[1].Value)
assert.Equal(t, decPoint75, testFile.TestCases[3].Result.Value)
}

func TestParseTestWithVariousTypes(t *testing.T) {
Expand Down Expand Up @@ -564,6 +575,7 @@ func TestParseTestWithBadScalarTests(t *testing.T) {
{"add(123::fp32, 1.4E+::fp32) = 123::fp32", 18, "no viable alternative at input '1.4E'"},
{"add(123::fp32, 3.E.5::fp32) = 123::fp32", 17, "no viable alternative at input '3.E'"},
{"f1((1, 2, 3, 4)::i64) = 10::fp64", 0, "expected scalar testcase based on test file header, but got aggregate function testcase"},
{"add(4.53::dec<1, 0>, 0.25::dec<2, 2>) = 0.78::dec<5, 2>", 0, "Visit error at line 5: invalid argument number 4.53"},
}
for _, test := range tests {
t.Run(test.testCaseStr, func(t *testing.T) {
Expand Down
Loading

0 comments on commit 00a91c9

Please sign in to comment.