From 193ef7cba1fc0be7f4b789941c79e421d14d4bbb Mon Sep 17 00:00:00 2001 From: pokeeffe-molecula <85502298+pokeeffe-molecula@users.noreply.github.com> Date: Tue, 1 Nov 2022 20:46:34 -0500 Subject: [PATCH] Test expression eval for tuple values in inserts (fb-1555) (#2270) refactored comparison, equality and arithmetic expr eval for decimal data types and added a test to cover expression eval for inserts fixed failing test --- pql/decimal.go | 47 ++++++++++++++++++- sql3/planner/executionplanner_test.go | 8 +++- sql3/planner/expression.go | 30 +++++-------- sql3/sql_definitions_test.go | 9 ++++ sql3/sql_defs_binops_test.go | 65 ++++++++++++++------------- sql3/sql_defs_set_functions_test.go | 20 +++++---- 6 files changed, 116 insertions(+), 63 deletions(-) diff --git a/pql/decimal.go b/pql/decimal.go index 077c6b1ce..c29e6cbdf 100644 --- a/pql/decimal.go +++ b/pql/decimal.go @@ -111,6 +111,52 @@ func AddDecimal(a, b Decimal) Decimal { } } +// SubtractDecimal subtracts b from a and returns a new Decimal. +// +// If the Scale of a and b don't match, the returned Decimal will have the +// smallest Scale needed to precisely represent the sum. +func SubtractDecimal(a, b Decimal) Decimal { + ac, bc := sameScalify(a, b) + apv, bpv := &ac.value, &bc.value + apv.Sub(apv, bpv) + return Decimal{ + value: *apv, + Scale: ac.Scale, + } +} + +// MultiplyDecimal multiplies a by b and returns a new Decimal. +// +// If the Scale of a and b don't match, the returned Decimal will have the +// smallest Scale needed to precisely represent the sum. +func MultiplyDecimal(a, b Decimal) Decimal { + ac, bc := sameScalify(a, b) + apv, bpv := &ac.value, &bc.value + scaleFactor := big.NewInt(Pow10(ac.Scale)) + apv.Mul(apv, bpv) + apv.Div(apv, scaleFactor) + return Decimal{ + value: *apv, + Scale: ac.Scale, + } +} + +// DivideDecimal multiplies a by b and returns a new Decimal. +// +// If the Scale of a and b don't match, the returned Decimal will have the +// smallest Scale needed to precisely represent the sum. +func DivideDecimal(a, b Decimal) Decimal { + ac, bc := sameScalify(a, b) + apv, bpv := &ac.value, &bc.value + scaleFactor := big.NewInt(Pow10(ac.Scale)) + apv.Mul(apv, scaleFactor) + apv.Div(apv, bpv) + return Decimal{ + value: *apv, + Scale: ac.Scale, + } +} + // LessThan returns true if d < d2. func (d Decimal) LessThan(d2 Decimal) bool { return d.lessThan(d2, false) @@ -139,7 +185,6 @@ func (d *Decimal) withLargerScale(scale int64) *Decimal { val = val.Mul(val, ten) dc.Scale++ } - return dc } diff --git a/sql3/planner/executionplanner_test.go b/sql3/planner/executionplanner_test.go index 7db2d34d9..7c2831968 100644 --- a/sql3/planner/executionplanner_test.go +++ b/sql3/planner/executionplanner_test.go @@ -799,10 +799,14 @@ func TestPlanner_ExpressionsInSelectListLiterals(t *testing.T) { t.Fatal(err) } + opt := cmp.Comparer(func(x, y pql.Decimal) bool { + return x.EqualTo(y) + }) + if diff := cmp.Diff([][]interface{}{ - {float64(12.3), int64(1)}, + {pql.NewDecimal(1230, 2), int64(1)}, {nil, int64(2)}, - }, results); diff != "" { + }, results, opt); diff != "" { t.Fatal(diff) } diff --git a/sql3/planner/expression.go b/sql3/planner/expression.go index 7cf55be7b..4164af3a2 100644 --- a/sql3/planner/expression.go +++ b/sql3/planner/expression.go @@ -465,9 +465,6 @@ func (n *binOpPlanExpression) Evaluate(currentRow []interface{}) (interface{}, e return nil, nil } - var nl float64 - var nr float64 - coercedLhs, err := coerceValue(n.lhs.Type(), coercedDataType, evalLhs, parser.Pos{Line: 0, Column: 0}) if err != nil { return nil, err @@ -481,36 +478,29 @@ func (n *binOpPlanExpression) Evaluate(currentRow []interface{}) (interface{}, e nld, nlok := coercedLhs.(pql.Decimal) nrd, nrok := coercedRhs.(pql.Decimal) - //TODO(pok) eliminate the use of float here and return pql.Decimal values for arithmetic ops - if nlok { - nl = nld.Float64() - } - if nrok { - nr = nrd.Float64() - } if nlok && nrok { switch n.op { case parser.NE: - return nl != nr, nil + return !nld.EqualTo(nrd), nil case parser.EQ: - return nl == nr, nil + return nld.EqualTo(nrd), nil case parser.LE: - return nl <= nr, nil + return nld.LessThanOrEqualTo(nrd), nil case parser.GE: - return nl >= nr, nil + return nld.GreaterThanOrEqualTo(nrd), nil case parser.GT: - return nl > nr, nil + return nld.GreaterThan(nrd), nil case parser.LT: - return nl < nr, nil + return nld.LessThan(nrd), nil case parser.PLUS: - return nl + nr, nil + return pql.AddDecimal(nld, nrd), nil case parser.MINUS: - return nl - nr, nil + return pql.SubtractDecimal(nld, nrd), nil case parser.STAR: - return nl * nr, nil + return pql.MultiplyDecimal(nld, nrd), nil case parser.SLASH: - return nl / nr, nil + return pql.DivideDecimal(nld, nrd), nil default: return nil, sql3.NewErrInternalf("unhandled operator %d", n.op) diff --git a/sql3/sql_definitions_test.go b/sql3/sql_definitions_test.go index 3bde76a4f..70063bd0d 100644 --- a/sql3/sql_definitions_test.go +++ b/sql3/sql_definitions_test.go @@ -412,6 +412,15 @@ var insertTest = tableTest{ expRows: rows(), compare: compareExactUnordered, }, + { + // Insert with exprs + sqls: sqls( + "insert into testinsert (_id, a, b, s, bl, d, event, ievent) values (4, 40*10, 400+1, 'foo' || 'bar', 1 > 2, 10.12 + 3.1, ['A', 'B', 'C'], [1, 2, 3])", + ), + expHdrs: hdrs(), + expRows: rows(), + compare: compareExactUnordered, + }, { // InsertBadTable sqls: sqls( diff --git a/sql3/sql_defs_binops_test.go b/sql3/sql_defs_binops_test.go index 4b3eeedce..05a2283dd 100644 --- a/sql3/sql_defs_binops_test.go +++ b/sql3/sql_defs_binops_test.go @@ -1,8 +1,12 @@ package sql3_test -import "time" +import ( + "time" -//INT bin op tests + "github.com/molecula/featurebase/v3/pql" +) + +// INT bin op tests var binOpExprWithIntInt = tableTest{ table: tbl( "binoptesti_i", @@ -635,7 +639,7 @@ var binOpExprWithIntDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(32.34)), + row(pql.NewDecimal(3234, 2)), ), compare: compareExactUnordered, }, @@ -647,7 +651,7 @@ var binOpExprWithIntDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(7.66)), + row(pql.NewDecimal(766, 2)), ), compare: compareExactUnordered, }, @@ -659,7 +663,7 @@ var binOpExprWithIntDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(246.8)), + row(pql.NewDecimal(24680, 2)), ), compare: compareExactUnordered, }, @@ -671,8 +675,7 @@ var binOpExprWithIntDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - //TODO(pok) this float64 thing is for the birds - row(float64(1.6207455429497568)), + row(pql.NewDecimal(162, 2)), ), compare: compareExactUnordered, }, @@ -1139,7 +1142,7 @@ var binOpExprWithIntStringSet = tableTest{ }, } -//BOOL bin op tests +// BOOL bin op tests var binOpExprWithBoolInt = tableTest{ table: tbl( "binoptestb_i", @@ -2047,7 +2050,7 @@ var binOpExprWithBoolStringSet = tableTest{ }, } -//ID bin op tests +// ID bin op tests var binOpExprWithIDInt = tableTest{ table: tbl( "binoptestid_i", @@ -2678,7 +2681,7 @@ var binOpExprWithIDDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(32.34)), + row(pql.NewDecimal(3234, 2)), ), compare: compareExactUnordered, }, @@ -2690,7 +2693,7 @@ var binOpExprWithIDDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(7.66)), + row(pql.NewDecimal(766, 2)), ), compare: compareExactUnordered, }, @@ -2702,7 +2705,7 @@ var binOpExprWithIDDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(246.8)), + row(pql.NewDecimal(24680, 2)), ), compare: compareExactUnordered, }, @@ -2714,8 +2717,7 @@ var binOpExprWithIDDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - //TODO(pok) this float64 thing is for the birds - row(float64(1.6207455429497568)), + row(pql.NewDecimal(162, 2)), ), compare: compareExactUnordered, }, @@ -3182,7 +3184,7 @@ var binOpExprWithIDStringSet = tableTest{ }, } -//DECIMAL bin op tests +// DECIMAL bin op tests var binOpExprWithDecInt = tableTest{ table: tbl( "binoptestdec_i", @@ -3300,7 +3302,7 @@ var binOpExprWithDecInt = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(32.34)), + row(pql.NewDecimal(3234, 2)), ), compare: compareExactUnordered, }, @@ -3312,7 +3314,7 @@ var binOpExprWithDecInt = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(-7.66)), + row(pql.NewDecimal(-766, 2)), ), compare: compareExactUnordered, }, @@ -3324,7 +3326,7 @@ var binOpExprWithDecInt = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(246.8)), + row(pql.NewDecimal(24680, 2)), ), compare: compareExactUnordered, }, @@ -3336,7 +3338,7 @@ var binOpExprWithDecInt = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(0.617)), + row(pql.NewDecimal(61, 2)), ), compare: compareExactUnordered, }, @@ -3584,7 +3586,7 @@ var binOpExprWithDecID = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(32.34)), + row(pql.NewDecimal(3234, 2)), ), compare: compareExactUnordered, }, @@ -3596,7 +3598,7 @@ var binOpExprWithDecID = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(-7.66)), + row(pql.NewDecimal(-766, 2)), ), compare: compareExactUnordered, }, @@ -3608,7 +3610,7 @@ var binOpExprWithDecID = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(246.8)), + row(pql.NewDecimal(24680, 2)), ), compare: compareExactUnordered, }, @@ -3620,7 +3622,7 @@ var binOpExprWithDecID = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(0.617)), + row(pql.NewDecimal(61, 2)), ), compare: compareExactUnordered, }, @@ -3756,7 +3758,7 @@ var binOpExprWithDecDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(32.34)), + row(pql.NewDecimal(3234, 2)), ), compare: compareExactUnordered, }, @@ -3768,7 +3770,7 @@ var binOpExprWithDecDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(7.66)), + row(pql.NewDecimal(766, 2)), ), compare: compareExactUnordered, }, @@ -3780,7 +3782,7 @@ var binOpExprWithDecDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - row(float64(246.8)), + row(pql.NewDecimal(24680, 2)), ), compare: compareExactUnordered, }, @@ -3792,8 +3794,7 @@ var binOpExprWithDecDecimal = tableTest{ hdr("", fldTypeDecimal2), ), expRows: rows( - //TODO(pok) this float64 thing is for the birds - row(float64(1.6207455429497568)), + row(pql.NewDecimal(162, 2)), ), compare: compareExactUnordered, }, @@ -4260,7 +4261,7 @@ var binOpExprWithDecStringSet = tableTest{ }, } -//TIMESTAMP bin op tests +// TIMESTAMP bin op tests var binOpExprWithTSInt = tableTest{ table: tbl( "binoptestts_i", @@ -5188,7 +5189,7 @@ var binOpExprWithTSStringSet = tableTest{ }, } -//IDSET bin op tests +// IDSET bin op tests var binOpExprWithIDSetInt = tableTest{ table: tbl( "binoptestids_i", @@ -6097,7 +6098,7 @@ var binOpExprWithIDSetStringSet = tableTest{ }, } -//STRING bin op tests +// STRING bin op tests var binOpExprWithStringInt = tableTest{ table: tbl( "binoptests_i", @@ -7012,7 +7013,7 @@ var binOpExprWithStringStringSet = tableTest{ }, } -//STRINGSET bin op tests +// STRINGSET bin op tests var binOpExprWithStringSetInt = tableTest{ table: tbl( "binoptestss_i", diff --git a/sql3/sql_defs_set_functions_test.go b/sql3/sql_defs_set_functions_test.go index 39c05a6ac..35b5b61f6 100644 --- a/sql3/sql_defs_set_functions_test.go +++ b/sql3/sql_defs_set_functions_test.go @@ -116,9 +116,10 @@ var setFunctionTests = tableTest{ ), expRows: rows( row(int64(1), int64(10), int64(100), []string{"POST"}, []int64{101}), - row(int64(3), int64(30), int64(300), []string{"POST", "GET"}, nil), + row(int64(3), int64(30), int64(300), []string{"GET", "POST"}, nil), ), - compare: compareExactUnordered, + compare: compareExactUnordered, + sortStringKeys: true, }, { // SetContains @@ -135,9 +136,10 @@ var setFunctionTests = tableTest{ ), expRows: rows( row(int64(1), int64(10), int64(100), []string{"POST"}, []int64{101}), - row(int64(3), int64(30), int64(300), []string{"POST", "GET"}, nil), + row(int64(3), int64(30), int64(300), []string{"GET", "POST"}, nil), ), - compare: compareExactUnordered, + compare: compareExactUnordered, + sortStringKeys: true, }, { // SetContainsInt @@ -175,9 +177,10 @@ var setFunctionTests = tableTest{ expRows: rows( row(int64(1), int64(10), int64(100), []string{"POST"}, []int64{101}), row(int64(2), int64(20), int64(200), []string{"GET"}, nil), - row(int64(3), int64(30), int64(300), []string{"POST", "GET"}, nil), + row(int64(3), int64(30), int64(300), []string{"GET", "POST"}, nil), ), - compare: compareExactUnordered, + compare: compareExactUnordered, + sortStringKeys: true, }, { // SetContainsAndSetContains @@ -195,9 +198,10 @@ var setFunctionTests = tableTest{ hdr("ievent", fldTypeIDSet), ), expRows: rows( - row(int64(3), int64(30), int64(300), []string{"POST", "GET"}, nil), + row(int64(3), int64(30), int64(300), []string{"GET", "POST"}, nil), ), - compare: compareExactUnordered, + compare: compareExactUnordered, + sortStringKeys: true, }, { // SetContainsWrongType