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

Store Decimal precision and size while normalising #15785

Merged
merged 12 commits into from
Apr 25, 2024
8 changes: 0 additions & 8 deletions go/mysql/decimal/decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,14 +579,6 @@ func (d Decimal) Exponent() int32 {
return d.exp
}

func (d Decimal) Size() int32 {
digitsCount := int32(len(d.value.String()))
if d.value.Sign() == -1 {
digitsCount--
}
return max(digitsCount, -d.exp)
}

func (d Decimal) Int64() (int64, bool) {
scaledD := d.rescale(0)
return scaledD.value.Int64(), scaledD.value.IsInt64()
Expand Down
52 changes: 30 additions & 22 deletions go/mysql/decimal/decimal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"testing/quick"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type testEnt struct {
Expand Down Expand Up @@ -955,48 +954,57 @@ func TestDecimal_ScalesNotEqual(t *testing.T) {
}

func TestDecimal_Cmp1(t *testing.T) {
a := New(123, 3)
b := New(-1234, 2)
assert.Equal(t, 1, a.Cmp(b))
}

func TestSizeAndScaleFromString(t *testing.T) {
testcases := []struct {
value string
sizeExpected int32
value string
sizeExpected int32
scaleExpected int32
}{
{
value: "0.00003",
sizeExpected: 5,
value: "0.00003",
sizeExpected: 6,
scaleExpected: 5,
},
{
value: "-0.00003",
sizeExpected: 5,
value: "-0.00003",
sizeExpected: 6,
scaleExpected: 5,
},
{
value: "12.00003",
sizeExpected: 7,
value: "12.00003",
sizeExpected: 7,
scaleExpected: 5,
},
{
value: "-12.00003",
sizeExpected: 7,
value: "-12.00003",
sizeExpected: 7,
scaleExpected: 5,
},
{
value: "1000003",
sizeExpected: 7,
value: "1000003",
sizeExpected: 7,
scaleExpected: 0,
},
{
value: "-1000003",
sizeExpected: 7,
value: "-1000003",
sizeExpected: 7,
scaleExpected: 0,
},
}
for _, testcase := range testcases {
t.Run(testcase.value, func(t *testing.T) {
val, err := NewFromString(testcase.value)
require.NoError(t, err)
assert.EqualValues(t, testcase.sizeExpected, val.Size())
siz, scale := SizeAndScaleFromString(testcase.value)
assert.EqualValues(t, testcase.sizeExpected, siz)
assert.EqualValues(t, testcase.scaleExpected, scale)
})
}
}

func TestDecimal_Size(t *testing.T) {

}

func TestDecimal_Cmp2(t *testing.T) {
a := New(123, 3)
b := New(1234, 2)
Expand Down
16 changes: 16 additions & 0 deletions go/mysql/decimal/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"math"
"math/big"
"math/bits"
"strings"

"vitess.io/vitess/go/mysql/fastparse"
)
Expand Down Expand Up @@ -71,6 +72,21 @@ func parseDecimal64(s []byte) (Decimal, error) {
}, nil
}

// SizeAndScaleFromString
func SizeAndScaleFromString(s string) (int32, int32) {
sign := 0
switch s[0] {
case '+', '-':
sign = 1
}
lenWithoutSign := len(s) - sign
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
idx := strings.Index(s, ".")
if idx == -1 {
return int32(lenWithoutSign), 0
}
return int32(lenWithoutSign - 1), int32(len(s) - 1 - idx)
}

func NewFromMySQL(s []byte) (Decimal, error) {
var original = s
var neg bool
Expand Down
2 changes: 1 addition & 1 deletion go/vt/sqlparser/ast_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ func (node *Argument) Format(buf *TrackedBuffer) {
if node.Size != 0 || node.Scale != 0 {
buf.astPrintf(node, "(%d", node.Size)
if node.Scale != 0 {
buf.astPrintf(node, ", %d", node.Scale)
buf.astPrintf(node, ",%d", node.Scale)
}
buf.WriteString(")")
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/sqlparser/ast_format_fast.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 3 additions & 6 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,12 +567,9 @@ func NewTypedArgumentFromLiteral(in string, lit *Literal) (*Argument, error) {
arg := &Argument{Name: in, Type: lit.SQLType()}
switch arg.Type {
case sqltypes.Decimal:
dec, err := decimal.NewFromMySQL(lit.Bytes())
if err != nil {
return nil, err
}
arg.Scale = -dec.Exponent()
arg.Size = dec.Size()
siz, scale := decimal.SizeAndScaleFromString(lit.Val)
arg.Scale = scale
arg.Size = siz
}
return arg, nil
}
Expand Down
3 changes: 3 additions & 0 deletions go/vt/sqlparser/normalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ func (nz *normalizer) convertLiteralDedup(node *Literal, cursor *Cursor) {
arg, err := NewTypedArgumentFromLiteral(bvname, node)
if err != nil {
nz.err = err
return
}
cursor.Replace(arg)
}
Expand All @@ -231,6 +232,7 @@ func (nz *normalizer) convertLiteral(node *Literal, cursor *Cursor) {
arg, err := NewTypedArgumentFromLiteral(bvname, node)
if err != nil {
nz.err = err
return
}
cursor.Replace(arg)
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -279,6 +281,7 @@ func (nz *normalizer) parameterize(left, right Expr) Expr {
arg, err := NewTypedArgumentFromLiteral(bvname, lit)
if err != nil {
nz.err = err
return nil
}
return arg
}
Expand Down
4 changes: 2 additions & 2 deletions go/vt/sqlparser/normalizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ func TestNormalize(t *testing.T) {
}, {
// float val
in: "select * from t where foobar = 1.2",
outstmt: "select * from t where foobar = :foobar /* DECIMAL(2, 1) */",
outstmt: "select * from t where foobar = :foobar /* DECIMAL(2,1) */",
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
outbv: map[string]*querypb.BindVariable{
"foobar": sqltypes.DecimalBindVariable("1.2"),
},
}, {
// multiple vals
in: "select * from t where foo = 1.2 and bar = 2",
outstmt: "select * from t where foo = :foo /* DECIMAL(2, 1) */ and bar = :bar /* INT64 */",
outstmt: "select * from t where foo = :foo /* DECIMAL(2,1) */ and bar = :bar /* INT64 */",
outbv: map[string]*querypb.BindVariable{
"foo": sqltypes.DecimalBindVariable("1.2"),
"bar": sqltypes.Int64BindVariable(2),
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/semantics/typer.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (t *typer) up(cursor *sqlparser.Cursor) error {
t.m[node] = evalengine.NewType(node.SQLType(), collations.CollationForType(node.SQLType(), t.collationEnv.DefaultConnectionCharset()))
case *sqlparser.Argument:
if node.Type >= 0 {
t.m[node] = evalengine.NewTypeEx(node.Type, collations.CollationForType(node.Type, t.collationEnv.DefaultConnectionCharset()), true, node.Size, node.Scale)
t.m[node] = evalengine.NewTypeEx(node.Type, collations.CollationForType(node.Type, t.collationEnv.DefaultConnectionCharset()), true, node.Size, node.Scale, nil)
}
case sqlparser.AggrFunc:
code, ok := opcode.SupportedAggregates[node.AggrName()]
Expand Down
Loading