From bc07fb4a96b7764fa21d4e547db69c77e1a10ea1 Mon Sep 17 00:00:00 2001 From: tgruben Date: Fri, 24 Mar 2023 13:33:05 -0500 Subject: [PATCH] SQL3 Test Coverage: complete coerceValue coverage (#2343) * complete coereceVal test coverage * sql between test * optimized between operator * basic operator test --- sql3/planner/expression.go | 4 - sql3/planner/expression_it_test.go | 10 ++- sql3/planner/expression_test.go | 132 +++++++++++++++++++++++++++++ sql3/planner/expressionpql.go | 28 +++++- sql3/planner/opinsert.go | 9 +- sql3/test/defs/defs.go | 23 ++--- sql3/test/defs/defs_binops.go | 75 ++++++++++++++-- sql3/test/defs/defs_select.go | 34 ++++++++ 8 files changed, 287 insertions(+), 28 deletions(-) create mode 100644 sql3/planner/expression_test.go diff --git a/sql3/planner/expression.go b/sql3/planner/expression.go index 5cd4d3534..260e7052d 100644 --- a/sql3/planner/expression.go +++ b/sql3/planner/expression.go @@ -91,8 +91,6 @@ func coerceValue(sourceType parser.ExprDataType, targetType parser.ExprDataType, } if tm, err := time.ParseInLocation(time.RFC3339Nano, val, time.UTC); err == nil { return tm, nil - } else if tm, err := time.ParseInLocation(time.RFC3339, val, time.UTC); err == nil { - return tm, nil } else if tm, err := time.ParseInLocation("2006-01-02", val, time.UTC); err == nil { return tm, nil } else { @@ -2983,8 +2981,6 @@ func wildCardToRegexp(pattern string) string { func timestampFromString(s string) (time.Time, error) { if tm, err := time.ParseInLocation(time.RFC3339Nano, s, time.UTC); err == nil { return tm, nil - } else if tm, err := time.ParseInLocation(time.RFC3339, s, time.UTC); err == nil { - return tm, nil } else if tm, err := time.ParseInLocation("2006-01-02", s, time.UTC); err == nil { return tm, nil } diff --git a/sql3/planner/expression_it_test.go b/sql3/planner/expression_it_test.go index 021e2c908..bf23baa34 100644 --- a/sql3/planner/expression_it_test.go +++ b/sql3/planner/expression_it_test.go @@ -10,12 +10,16 @@ import ( ) func TestExpressions(t *testing.T) { - t.Run("StringerTest", func(t *testing.T) { - uop := newUnaryOpPlanExpression(parser.PLUS, newIntLiteralPlanExpression(10), parser.NewDataTypeInt()) assert.Equal(t, uop.String(), "+10") + uop1 := newUnaryOpPlanExpression(parser.PLUS, newIntLiteralPlanExpression(10), parser.NewDataTypeID()) + assert.Equal(t, uop1.String(), "+10") + + uop2 := newUnaryOpPlanExpression(parser.MINUS, newIntLiteralPlanExpression(10), parser.NewDataTypeID()) + assert.Equal(t, uop2.String(), "-10") + bop := newBinOpPlanExpression(newIntLiteralPlanExpression(10), parser.PLUS, newIntLiteralPlanExpression(20), parser.NewDataTypeInt()) assert.Equal(t, bop.String(), "10+20") @@ -78,7 +82,5 @@ func TestExpressions(t *testing.T) { tplop := newExprTupleLiteralPlanExpression([]types.PlanExpression{newStringLiteralPlanExpression("foo"), newStringLiteralPlanExpression("bar")}, parser.NewDataTypeString()) assert.Equal(t, tplop.String(), "{'foo', 'bar'}") - }) - } diff --git a/sql3/planner/expression_test.go b/sql3/planner/expression_test.go new file mode 100644 index 000000000..a4024480b --- /dev/null +++ b/sql3/planner/expression_test.go @@ -0,0 +1,132 @@ +// Copyright 2022 Molecula Corp. All rights reserved. + +package planner + +import ( + "math" + "reflect" + "testing" + "time" + + "github.com/featurebasedb/featurebase/v3/pql" + "github.com/featurebasedb/featurebase/v3/sql3/parser" +) + +func Test_coerceValue(t *testing.T) { + type args struct { + sourceType parser.ExprDataType + targetType parser.ExprDataType + value interface{} + atPos parser.Pos + } + tests := []struct { + name string + args args + want interface{} + wantErr bool + }{ + { + name: "int-int", + args: args{sourceType: &parser.DataTypeInt{}, targetType: &parser.DataTypeInt{}, value: 1, atPos: parser.Pos{}}, + want: 1, + wantErr: false, + }, { + name: "int-id", + args: args{sourceType: &parser.DataTypeInt{}, targetType: &parser.DataTypeID{}, value: int64(42), atPos: parser.Pos{}}, + want: int64(42), + wantErr: false, + }, { + name: "int-decimal", + args: args{sourceType: &parser.DataTypeInt{}, targetType: &parser.DataTypeDecimal{Scale: 2}, value: int64(1), atPos: parser.Pos{}}, + want: pql.NewDecimal(int64(math.Pow(10, float64(2))), 2), + wantErr: false, + }, { + name: "int-timestamp", + args: args{sourceType: &parser.DataTypeInt{}, targetType: &parser.DataTypeTimestamp{}, value: int64(1679499982), atPos: parser.Pos{}}, + want: time.Unix(1679499982, 0).UTC(), + wantErr: false, + }, { + name: "ID-int", + args: args{sourceType: &parser.DataTypeID{}, targetType: &parser.DataTypeInt{}, value: int64(42), atPos: parser.Pos{}}, + want: int64(42), + wantErr: false, + }, { + name: "ID-decimal", + args: args{sourceType: &parser.DataTypeID{}, targetType: &parser.DataTypeDecimal{Scale: 2}, value: int64(1), atPos: parser.Pos{}}, + want: pql.NewDecimal(int64(math.Pow(10, float64(2))), 2), + wantErr: false, + }, { + name: "ID-timestamp", + args: args{sourceType: &parser.DataTypeID{}, targetType: &parser.DataTypeTimestamp{}, value: int64(1679499982), atPos: parser.Pos{}}, + want: time.Unix(1679499982, 0).UTC(), + wantErr: false, + }, { + name: "ID-ID", + args: args{sourceType: &parser.DataTypeID{}, targetType: &parser.DataTypeID{}, value: 0, atPos: parser.Pos{}}, + want: 0, + wantErr: false, + }, { + name: "decimal-decimal", + args: args{sourceType: &parser.DataTypeDecimal{}, targetType: &parser.DataTypeDecimal{}, value: 0, atPos: parser.Pos{}}, + want: 0, + wantErr: false, + }, { + name: "string-string", + args: args{sourceType: &parser.DataTypeString{}, targetType: &parser.DataTypeString{}, value: "hello", atPos: parser.Pos{}}, + want: "hello", + wantErr: false, + }, { + name: "string-timestamp", + args: args{sourceType: &parser.DataTypeString{}, targetType: &parser.DataTypeTimestamp{}, value: "2022-03-24", atPos: parser.Pos{}}, + want: func() time.Time { tm, _ := time.ParseInLocation("2006-01-02", "2022-03-24", time.UTC); return tm }(), + wantErr: false, + }, { + name: "timestamp-timestamp", + args: args{sourceType: &parser.DataTypeTimestamp{}, targetType: &parser.DataTypeTimestamp{}, value: 0, atPos: parser.Pos{}}, + want: 0, + wantErr: false, + }, { + name: "idset-idset", + args: args{sourceType: &parser.DataTypeIDSet{}, targetType: &parser.DataTypeIDSet{}, value: 0, atPos: parser.Pos{}}, + want: 0, + wantErr: false, + }, { + name: "idset-quantum", + args: args{sourceType: &parser.DataTypeIDSet{}, targetType: &parser.DataTypeIDSetQuantum{}, value: 10, atPos: parser.Pos{}}, + want: []interface{}{nil, 10}, + wantErr: false, + }, { + name: "stringset-idset", + args: args{sourceType: &parser.DataTypeStringSet{}, targetType: &parser.DataTypeStringSet{}, value: 0, atPos: parser.Pos{}}, + want: 0, + wantErr: false, + }, { + name: "stringset-quantum", + args: args{sourceType: &parser.DataTypeStringSet{}, targetType: &parser.DataTypeStringSetQuantum{}, value: "YMD", atPos: parser.Pos{}}, + want: []interface{}{nil, "YMD"}, + wantErr: false, + }, { + name: "tuple-stringsetquantum", + args: args{sourceType: &parser.DataTypeTuple{}, targetType: &parser.DataTypeStringSetQuantum{}, value: 0, atPos: parser.Pos{}}, + want: 0, + wantErr: false, + }, { + name: "tuple-idsetquantum", + args: args{sourceType: &parser.DataTypeTuple{}, targetType: &parser.DataTypeIDSetQuantum{}, value: 0, atPos: parser.Pos{}}, + want: 0, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := coerceValue(tt.args.sourceType, tt.args.targetType, tt.args.value, tt.args.atPos) + if (err != nil) != tt.wantErr { + t.Errorf("coerceValue() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("coerceValue() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/sql3/planner/expressionpql.go b/sql3/planner/expressionpql.go index e2b1a6b1c..f80500534 100644 --- a/sql3/planner/expressionpql.go +++ b/sql3/planner/expressionpql.go @@ -232,7 +232,33 @@ func (p *ExecutionPlanner) generatePQLCallFromExpr(ctx context.Context, expr typ call.Children = append(call.Children, rc) } return call, nil - + case *betweenOpPlanExpression: + lhs, ok := expr.lhs.(*qualifiedRefPlanExpression) + if !ok { + return nil, sql3.NewErrInternalf("expected expression type: planner.qualifiedRefPlanExpression got:%T", expr.lhs) + } + rexp, ok := expr.rhs.(*rangePlanExpression) + if !ok { + return nil, sql3.NewErrInternalf("expected expression type: planner.rangePlanExpression got:%T", expr.rhs) + } + lower, err := planExprToValue(rexp.lhs) + if err != nil { + return nil, err + } + upper, err := planExprToValue(rexp.rhs) + if err != nil { + return nil, err + } + call := &pql.Call{ + Name: "Row", + Args: map[string]interface{}{ + lhs.columnName: &pql.Condition{ + Op: pql.BETWEEN, + Value: []interface{}{lower, upper}, + }, + }, + } + return call, nil default: return nil, sql3.NewErrInternalf("unexpected expression type: %T", expr) } diff --git a/sql3/planner/opinsert.go b/sql3/planner/opinsert.go index 87dfee2d4..f7512456f 100644 --- a/sql3/planner/opinsert.go +++ b/sql3/planner/opinsert.go @@ -197,7 +197,10 @@ func (i *insertRowIter) Next(ctx context.Context) (types.Row, error) { default: // If we get to here, it's likey that the id type is unsupported // and will cause an error in batch.Add(). So there's no need to - // return an error here in this default. + // return an error here in this default. The only route here + // would be if a primary key had a type other than intLiteralPlanExpression + // or stringLiteralPlanExpression + row.ID = eval } } @@ -390,8 +393,8 @@ func (i *insertRowIter) Next(ctx context.Context) (types.Row, error) { return nil, errors.Wrapf(err, "converting timestamp to int64: %s", v) } row.Values[posVals[idx]] = i64 - //integers passed as input for Timestamp fields will be treated as time represented in number of seconds since epoch defined for the field - //for timestamp fields created using SQL epoch will be defaulted to unix epoch + // integers passed as input for Timestamp fields will be treated as time represented in number of seconds since epoch defined for the field + // for timestamp fields created using SQL epoch will be defaulted to unix epoch case int64: // Convert the input seconds to target timeunit defined for the Timestamp field // Add Base, which is the base epoch for Timestamp fields, to the input before saving diff --git a/sql3/test/defs/defs.go b/sql3/test/defs/defs.go index 282ef22c2..27a32cc4b 100644 --- a/sql3/test/defs/defs.go +++ b/sql3/test/defs/defs.go @@ -24,6 +24,7 @@ var TableTests []TableTest = []TableTest{ selectTests, selectKeyedTests, selectHavingTests, + selectBetweenTests, filterPredicates, filterPredicatesIdKey, filterPredicatesId, @@ -140,7 +141,7 @@ var TableTests []TableTest = []TableTest{ binOpExprWithStringSetString, binOpExprWithStringSetStringSet, - //cast tests + // cast tests castIntLiteral, castInt, @@ -152,26 +153,26 @@ var TableTests []TableTest = []TableTest{ castStringSet, castTimestamp, - //like tests + // like tests likeTests, notLikeTests, - //null tests + // null tests nullTests, notNullTests, - //null filter tests + // null filter tests nullFilterTests, - //between tests + // between tests betweenTests, notBetweenTests, - //in tests + // in tests inTests, notInTests, - //aggregate tests + // aggregate tests countTests, countDistinctTests, sumTests, @@ -179,14 +180,14 @@ var TableTests []TableTest = []TableTest{ percentileTests, minmaxTests, - //groupby tests + // groupby tests groupByTests, - //create table tests + // create table tests createTable, alterTable, - //joins + // joins joinTestsUsers, joinTestsOrders, joinTests, @@ -195,7 +196,7 @@ var TableTests []TableTest = []TableTest{ bulkInsertTable, bulkInsert, - //bool (batch logic) + // bool (batch logic) boolTests, // time quantums diff --git a/sql3/test/defs/defs_binops.go b/sql3/test/defs/defs_binops.go index b2889b65a..6ee539a33 100644 --- a/sql3/test/defs/defs_binops.go +++ b/sql3/test/defs/defs_binops.go @@ -234,6 +234,42 @@ var binOpExprWithIntInt = TableTest{ ), ExpErr: "operator '||' incompatible with type 'int'", }, + { + SQLs: sqls( + "select 1 - 1 ;", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + ), + ExpRows: rows( + row(int64(0)), + ), + Compare: CompareExactUnordered, + }, + { + SQLs: sqls( + "select 1 / 1 ;", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + ), + ExpRows: rows( + row(int64(1)), + ), + Compare: CompareExactUnordered, + }, + { + SQLs: sqls( + "select 1 % 1 ;", + ), + ExpHdrs: hdrs( + hdr("", fldTypeInt), + ), + ExpRows: rows( + row(int64(0)), + ), + Compare: CompareExactUnordered, + }, }, } @@ -2638,6 +2674,30 @@ var binOpExprWithIDID = TableTest{ ), ExpErr: "operator '||' incompatible with type 'id'", }, + { + SQLs: sqls( + "select _id + _id from binoptestid_id;", + ), + ExpHdrs: hdrs( + hdr("", fldTypeID), + ), + ExpRows: rows( + row(int64(20)), + ), + Compare: CompareExactUnordered, + }, + { + SQLs: sqls( + "select _id - _id from binoptestid_id;", + ), + ExpHdrs: hdrs( + hdr("", fldTypeID), + ), + ExpRows: rows( + row(int64(0)), + ), + Compare: CompareExactUnordered, + }, }, } @@ -4810,7 +4870,8 @@ var binOpExprWithTSTimestamp = TableTest{ ExpRows: rows( row(bool(false)), ), - Compare: CompareExactUnordered}, + Compare: CompareExactUnordered, + }, { SQLs: sqls( "select a = ts from binoptestts_ts;", @@ -4821,7 +4882,8 @@ var binOpExprWithTSTimestamp = TableTest{ ExpRows: rows( row(bool(true)), ), - Compare: CompareExactUnordered}, + Compare: CompareExactUnordered, + }, { SQLs: sqls( "select a <= ts from binoptestts_ts;", @@ -4844,7 +4906,8 @@ var binOpExprWithTSTimestamp = TableTest{ ExpRows: rows( row(bool(true)), ), - Compare: CompareExactUnordered}, + Compare: CompareExactUnordered, + }, { SQLs: sqls( "select a < ts from binoptestts_ts;", @@ -4855,7 +4918,8 @@ var binOpExprWithTSTimestamp = TableTest{ ExpRows: rows( row(bool(false)), ), - Compare: CompareExactUnordered}, + Compare: CompareExactUnordered, + }, { SQLs: sqls( "select a > ts from binoptestts_ts;", @@ -4866,7 +4930,8 @@ var binOpExprWithTSTimestamp = TableTest{ ExpRows: rows( row(bool(false)), ), - Compare: CompareExactUnordered}, + Compare: CompareExactUnordered, + }, { SQLs: sqls( "select a & ts from binoptestts_ts;", diff --git a/sql3/test/defs/defs_select.go b/sql3/test/defs/defs_select.go index f36197a5f..8e7802fe9 100644 --- a/sql3/test/defs/defs_select.go +++ b/sql3/test/defs/defs_select.go @@ -113,3 +113,37 @@ var selectKeyedTests = TableTest{ }, }, } + +var selectBetweenTests = TableTest{ + name: "selectBetweenTests", + Table: tbl( + "selectbetween", + srcHdrs( + srcHdr("_id", fldTypeString), + srcHdr("an_int", fldTypeInt, "min 0", "max 100"), + ), + srcRows( + srcRow(string("user1"), int64(11)), + srcRow(string("user2"), int64(22)), + srcRow(string("user3"), int64(33)), + srcRow(string("user4"), int64(44)), + ), + ), + SQLTests: []SQLTest{ + { + SQLs: sqls( + "select _id,an_int from selectbetween where an_int between 22 AND 33", + ), + ExpHdrs: hdrs( + hdr("_id", fldTypeString), + hdr("an_int", fldTypeInt), + ), + ExpRows: rows( + row(string("user2"), int64(22)), + row(string("user3"), int64(33)), + ), + Compare: CompareExactUnordered, + SortStringKeys: true, + }, + }, +}