From bda079ee3a4a1c67d5e94de38f55506ab3a12e6c Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 09:49:56 +0200 Subject: [PATCH 1/8] gen4: better planning for NOT IN predicates Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/route_planning.go | 64 ++++++++++++++----- .../planbuilder/testdata/filter_cases.txt | 21 ++++++ 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index b5b2fab500c..1f2d44bf4e4 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -274,17 +274,19 @@ type vindexPlusPredicates struct { predicates []sqlparser.Expr } -// addPredicate clones this routePlan and returns a new one with these predicates added to it. if the predicates can help, +// addPredicate adds these predicates added to it. if the predicates can help, // they will improve the routeOpCode func (rp *routePlan) addPredicate(predicates ...sqlparser.Expr) error { - newVindexFound, err := rp.searchForNewVindexes(predicates) - if err != nil { - return err - } + if rp.canImprove() { + newVindexFound, err := rp.searchForNewVindexes(predicates) + if err != nil { + return err + } - // if we didn't open up any new vindex options, no need to enter here - if newVindexFound { - rp.pickBestAvailableVindex() + // if we didn't open up any new vindex options, no need to enter here + if newVindexFound { + rp.pickBestAvailableVindex() + } } // any predicates that cover more than a single table need to be added here @@ -293,6 +295,37 @@ func (rp *routePlan) addPredicate(predicates ...sqlparser.Expr) error { return nil } +// canImprove returns true if additional predicates could help improving this plan +func (rp *routePlan) canImprove() bool { + return rp.routeOpCode != engine.SelectNone +} + +func (rp *routePlan) isImpossibleIN(node *sqlparser.ComparisonExpr) bool { + switch nodeR := node.Right.(type) { + case sqlparser.ValTuple: + // WHERE col IN (null) + if len(nodeR) == 1 && sqlparser.IsNull(nodeR[0]) { + rp.routeOpCode = engine.SelectNone + return true + } + } + return false +} + +func (rp *routePlan) isImpossibleNotIN(node *sqlparser.ComparisonExpr) bool { + switch node := node.Right.(type) { + case sqlparser.ValTuple: + for _, n := range node { + if sqlparser.IsNull(n) { + rp.routeOpCode = engine.SelectNone + return true + } + } + } + + return false +} + func (rp *routePlan) searchForNewVindexes(predicates []sqlparser.Expr) (bool, error) { newVindexFound := false for _, filter := range predicates { @@ -314,20 +347,19 @@ func (rp *routePlan) searchForNewVindexes(predicates []sqlparser.Expr) (bool, er } newVindexFound = newVindexFound || foundEqualOpVindex case sqlparser.InOp: - switch nodeR := node.Right.(type) { - case sqlparser.ValTuple: - // WHERE col IN (null) - if len(nodeR) == 1 && sqlparser.IsNull(nodeR[0]) { - rp.routeOpCode = engine.SelectNone - return false, nil - } + if rp.isImpossibleIN(node) { + return false, nil } - foundEqualOpVindex, err := rp.planInOp(node) if err != nil { return false, err } newVindexFound = newVindexFound || foundEqualOpVindex + case sqlparser.NotInOp: + // NOT IN is always a scatter, except when we can be sure it would return nothing + if rp.isImpossibleNotIN(node) { + return false, nil + } default: return false, semantics.Gen4NotSupportedF("%s", sqlparser.String(filter)) diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index a79943f3f8b..cd5d391cca7 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1762,6 +1762,27 @@ Gen4 plan same as above "Table": "music" } } +Gen4 plan same as above + +# Single table with unique vindex match and NOT IN (null, 1, 2) predicates inverted +"select id from music where id NOT IN (null, 1, 2) and user_id = 4" +{ + "QueryType": "SELECT", + "Original": "select id from music where id NOT IN (null, 1, 2) and user_id = 4", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectNone", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from music where 1 != 1", + "Query": "select id from music where id not in (null, 1, 2) and user_id = 4", + "Table": "music" + } +} +Gen4 plan same as above + # query trying to query two different keyspaces at the same time "SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = 'user' AND TABLE_SCHEMA = 'main'" From d64a6513ecf6bba151199f13fda387f2481083d8 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 10:53:30 +0200 Subject: [PATCH 2/8] refactoring of route_planning to make LIKE easier to add Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/route_planning.go | 77 +++++++++++-------- .../vtgate/planbuilder/route_planning_test.go | 10 +-- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 1f2d44bf4e4..bc236f36fa4 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -266,12 +266,13 @@ func (rp *routePlan) cost() int { // vindexPlusPredicates is a struct used to store all the predicates that the vindex can be used to query type vindexPlusPredicates struct { - vindex *vindexes.ColumnVindex - values []sqltypes.PlanValue - // Vindex is covered if all the columns in the vindex have an associated predicate - covered bool - opcode engine.RouteOpcode - predicates []sqlparser.Expr + colVindex *vindexes.ColumnVindex + values []sqltypes.PlanValue + + // when we have the predicates found, we also know how to interact with this vindex + foundVindex vindexes.Vindex + opcode engine.RouteOpcode + predicates []sqlparser.Expr } // addPredicate adds these predicates added to it. if the predicates can help, @@ -341,20 +342,20 @@ func (rp *routePlan) searchForNewVindexes(predicates []sqlparser.Expr) (bool, er switch node.Operator { case sqlparser.EqualOp: - foundEqualOpVindex, err := rp.planEqualOp(node) + found, err := rp.planEqualOp(node) if err != nil { return false, err } - newVindexFound = newVindexFound || foundEqualOpVindex + newVindexFound = newVindexFound || found case sqlparser.InOp: if rp.isImpossibleIN(node) { return false, nil } - foundEqualOpVindex, err := rp.planInOp(node) + found, err := rp.planInOp(node) if err != nil { return false, err } - newVindexFound = newVindexFound || foundEqualOpVindex + newVindexFound = newVindexFound || found case sqlparser.NotInOp: // NOT IN is always a scatter, except when we can be sure it would return nothing if rp.isImpossibleNotIN(node) { @@ -380,25 +381,33 @@ func (rp *routePlan) planEqualOp(node *sqlparser.ComparisonExpr) (bool, error) { } other = node.Left } - value, err := sqlparser.NewPlanValue(other) - if err != nil { - // if we are unable to create a PlanValue, we can't use a vindex, but we don't have to fail - if strings.Contains(err.Error(), "expression is too complex") { - return false, nil - } - // something else went wrong, return the error + val, err := makePlanValue(other) + if err != nil || val == nil { return false, err } - opcode := func(vindex *vindexes.ColumnVindex) engine.RouteOpcode { + opcode := func(vindex *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { if vindex.Vindex.IsUnique() { - return engine.SelectEqualUnique + return vindex.Vindex, engine.SelectEqualUnique } - return engine.SelectEqual + return vindex.Vindex, engine.SelectEqual } - return rp.haveMatchingVindex(node, column, value, opcode), err + return rp.haveMatchingVindex(node, column, *val, opcode), err +} + +func makePlanValue(n sqlparser.Expr) (*sqltypes.PlanValue, error) { + value, err := sqlparser.NewPlanValue(n) + if err != nil { + // if we are unable to create a PlanValue, we can't use a vindex, but we don't have to fail + if strings.Contains(err.Error(), "expression is too complex") { + return nil, nil + } + // something else went wrong, return the error + return nil, err + } + return &value, nil } func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) { @@ -422,7 +431,9 @@ func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) { return false, nil } } - opcode := func(*vindexes.ColumnVindex) engine.RouteOpcode { return engine.SelectIN } + opcode := func(v *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { + return v.Vindex, engine.SelectIN + } return rp.haveMatchingVindex(node, column, value, opcode), err } @@ -430,22 +441,24 @@ func (rp *routePlan) haveMatchingVindex( node *sqlparser.ComparisonExpr, column *sqlparser.ColName, value sqltypes.PlanValue, - opcode func(*vindexes.ColumnVindex) engine.RouteOpcode, + opcode func(*vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode), ) bool { newVindexFound := false for _, v := range rp.vindexPreds { - if v.covered { + if v.foundVindex != nil { continue } - for _, col := range v.vindex.Columns { + for _, col := range v.colVindex.Columns { // If the column for the predicate matches any column in the vindex add it to the list if column.Name.Equal(col) { v.values = append(v.values, value) v.predicates = append(v.predicates, node) // Vindex is covered if all the columns in the vindex have a associated predicate - v.covered = len(v.values) == len(v.vindex.Columns) - v.opcode = opcode(v.vindex) - newVindexFound = newVindexFound || v.covered + covered := len(v.values) == len(v.colVindex.Columns) + if covered { + v.foundVindex, v.opcode = opcode(v.colVindex) + } + newVindexFound = newVindexFound || covered } } } @@ -455,13 +468,13 @@ func (rp *routePlan) haveMatchingVindex( // pickBestAvailableVindex goes over the available vindexes for this route and picks the best one available. func (rp *routePlan) pickBestAvailableVindex() { for _, v := range rp.vindexPreds { - if !v.covered { + if v.foundVindex == nil { continue } // Choose the minimum cost vindex from the ones which are covered - if rp.vindex == nil || v.vindex.Vindex.Cost() < rp.vindex.Cost() { + if rp.vindex == nil || v.colVindex.Vindex.Cost() < rp.vindex.Cost() { rp.routeOpCode = v.opcode - rp.vindex = v.vindex.Vindex + rp.vindex = v.foundVindex rp.vindexValues = v.values rp.vindexPredicates = v.predicates } @@ -777,7 +790,7 @@ func createRoutePlan(table *queryTable, solves semantics.TableSet, vschema Conte } for _, columnVindex := range vschemaTable.ColumnVindexes { - plan.vindexPreds = append(plan.vindexPreds, &vindexPlusPredicates{vindex: columnVindex}) + plan.vindexPreds = append(plan.vindexPreds, &vindexPlusPredicates{colVindex: columnVindex}) } switch { diff --git a/go/vt/vtgate/planbuilder/route_planning_test.go b/go/vt/vtgate/planbuilder/route_planning_test.go index 4209225bd66..70161799c35 100644 --- a/go/vt/vtgate/planbuilder/route_planning_test.go +++ b/go/vt/vtgate/planbuilder/route_planning_test.go @@ -108,9 +108,7 @@ func TestMergeJoins(t *testing.T) { func TestClone(t *testing.T) { original := &routePlan{ routeOpCode: engine.SelectEqualUnique, - vindexPreds: []*vindexPlusPredicates{{ - covered: false, - }}, + vindexPreds: []*vindexPlusPredicates{{}}, } clone := original.clone() @@ -120,7 +118,7 @@ func TestClone(t *testing.T) { assert.Equal(t, clonedRP.routeOpCode, engine.SelectDBA) assert.Equal(t, original.routeOpCode, engine.SelectEqualUnique) - clonedRP.vindexPreds[0].covered = true - assert.True(t, clonedRP.vindexPreds[0].covered) - assert.False(t, original.vindexPreds[0].covered) + clonedRP.vindexPreds[0].foundVindex = &vindexes.Null{} + assert.NotNil(t, clonedRP.vindexPreds[0].foundVindex) + assert.Nil(t, original.vindexPreds[0].foundVindex) } From 4c5ea9d4728d767d2aa983467579a3ad30ba736e Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 11:24:47 +0200 Subject: [PATCH 3/8] add test for LIKE query Signed-off-by: Andres Taylor --- .../planbuilder/testdata/filter_cases.txt | 22 +++++++++++++++++++ .../planbuilder/testdata/schema_test.json | 21 ++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index cd5d391cca7..60201c62bd4 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1972,3 +1972,25 @@ Gen4 plan same as above "SysTableTableSchema": "[VARBINARY(\"ks\")]" } } + +# solving LIKE query with a CFC prefix vindex +"select c2 from cfc_vindex_col where c1 like 'A%'" +{ + "QueryType": "SELECT", + "Original": "select c2 from cfc_vindex_col where c1 like 'A%'", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectEqual", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select c2 from cfc_vindex_col where 1 != 1", + "Query": "select c2 from cfc_vindex_col where c1 like 'A%'", + "Table": "cfc_vindex_col", + "Values": [ + "A%" + ], + "Vindex": "cfc" + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/schema_test.json b/go/vt/vtgate/planbuilder/testdata/schema_test.json index fd56617707b..1a0213bd46e 100644 --- a/go/vt/vtgate/planbuilder/testdata/schema_test.json +++ b/go/vt/vtgate/planbuilder/testdata/schema_test.json @@ -78,6 +78,9 @@ "vindex2": { "type": "lookup_test", "owner": "samecolvin" + }, + "cfc": { + "type": "cfc" } }, "tables": { @@ -257,6 +260,24 @@ "name": "user_index" } ] + }, + "cfc_vindex_col": { + "column_vindexes": [ + { + "column": "c1", + "name": "cfc" + } + ], + "columns": [ + { + "name": "c1", + "type": "VARCHAR" + }, + { + "name": "c2", + "type": "VARCHAR" + } + ] } } }, From 08e2a83649024141f0679a9ec7848de50215bb72 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 11:21:36 +0200 Subject: [PATCH 4/8] allow gen4 to plan LIKE using CFC vindexes Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/route_planning.go | 29 +++++++++++++++++++ .../planbuilder/testdata/filter_cases.txt | 1 + 2 files changed, 30 insertions(+) diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index bc236f36fa4..563b493bed9 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -361,6 +361,12 @@ func (rp *routePlan) searchForNewVindexes(predicates []sqlparser.Expr) (bool, er if rp.isImpossibleNotIN(node) { return false, nil } + case sqlparser.LikeOp: + found, err := rp.planLikeOp(node) + if err != nil { + return false, err + } + newVindexFound = newVindexFound || found default: return false, semantics.Gen4NotSupportedF("%s", sqlparser.String(filter)) @@ -437,6 +443,29 @@ func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) { return rp.haveMatchingVindex(node, column, value, opcode), err } +func (rp *routePlan) planLikeOp(node *sqlparser.ComparisonExpr) (bool, error) { + column, ok := node.Left.(*sqlparser.ColName) + if !ok { + return false, nil + } + + val, err := makePlanValue(node.Right) + if err != nil || val == nil { + return false, err + } + + opcode := func(vindex *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { + if prefixable, ok := vindex.Vindex.(vindexes.Prefixable); ok { + return prefixable.PrefixVindex(), engine.SelectEqual + } + + // if we can't use the vindex as a prefix-vindex, we can't use this vindex at all + return nil, engine.SelectScatter + } + + return rp.haveMatchingVindex(node, column, *val, opcode), err +} + func (rp *routePlan) haveMatchingVindex( node *sqlparser.ComparisonExpr, column *sqlparser.ColName, diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index 60201c62bd4..0f4cc3f8423 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1994,3 +1994,4 @@ Gen4 plan same as above "Vindex": "cfc" } } +Gen4 plan same as above From c3b23ca99c39df44c1c4c91e947b2ec0a02301ac Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 12:03:39 +0200 Subject: [PATCH 5/8] renamed fields in IsExpr Signed-off-by: Andres Taylor --- go/vt/sqlparser/ast.go | 4 ++-- go/vt/sqlparser/ast_clone.go | 2 +- go/vt/sqlparser/ast_equals.go | 4 ++-- go/vt/sqlparser/ast_format.go | 2 +- go/vt/sqlparser/ast_format_fast.go | 4 ++-- go/vt/sqlparser/ast_rewrite.go | 4 ++-- go/vt/sqlparser/ast_visit.go | 2 +- go/vt/sqlparser/cached_size.go | 6 +++--- go/vt/sqlparser/precedence_test.go | 2 +- go/vt/sqlparser/random_expr.go | 4 ++-- go/vt/sqlparser/sql.go | 2 +- go/vt/sqlparser/sql.y | 2 +- go/vt/vtgate/planbuilder/route.go | 4 ++-- 13 files changed, 21 insertions(+), 21 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 6fbecdf7462..db3d010d4a0 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -1794,8 +1794,8 @@ type ( // IsExpr represents an IS ... or an IS NOT ... expression. IsExpr struct { - Operator IsExprOperator - Expr Expr + Left Expr + Right IsExprOperator } // IsExprOperator is an enum for IsExpr.Operator diff --git a/go/vt/sqlparser/ast_clone.go b/go/vt/sqlparser/ast_clone.go index bc8d8ca456b..16f9e2ed2c3 100644 --- a/go/vt/sqlparser/ast_clone.go +++ b/go/vt/sqlparser/ast_clone.go @@ -972,7 +972,7 @@ func CloneRefOfIsExpr(n *IsExpr) *IsExpr { return nil } out := *n - out.Expr = CloneExpr(n.Expr) + out.Left = CloneExpr(n.Left) return &out } diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go index 48239b83dba..1107a2489cc 100644 --- a/go/vt/sqlparser/ast_equals.go +++ b/go/vt/sqlparser/ast_equals.go @@ -1709,8 +1709,8 @@ func EqualsRefOfIsExpr(a, b *IsExpr) bool { if a == nil || b == nil { return false } - return a.Operator == b.Operator && - EqualsExpr(a.Expr, b.Expr) + return EqualsExpr(a.Left, b.Left) && + a.Right == b.Right } // EqualsJoinCondition does deep equals between the two objects. diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go index dfe328c9e7b..1ce0e24c301 100644 --- a/go/vt/sqlparser/ast_format.go +++ b/go/vt/sqlparser/ast_format.go @@ -960,7 +960,7 @@ func (node *RangeCond) Format(buf *TrackedBuffer) { // Format formats the node. func (node *IsExpr) Format(buf *TrackedBuffer) { - buf.astPrintf(node, "%v %s", node.Expr, node.Operator.ToString()) + buf.astPrintf(node, "%v %s", node.Left, node.Right.ToString()) } // Format formats the node. diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go index 6585d9bb905..4302c556de2 100644 --- a/go/vt/sqlparser/ast_format_fast.go +++ b/go/vt/sqlparser/ast_format_fast.go @@ -1300,9 +1300,9 @@ func (node *RangeCond) formatFast(buf *TrackedBuffer) { // formatFast formats the node. func (node *IsExpr) formatFast(buf *TrackedBuffer) { - buf.printExpr(node, node.Expr, true) + buf.printExpr(node, node.Left, true) buf.WriteByte(' ') - buf.WriteString(node.Operator.ToString()) + buf.WriteString(node.Right.ToString()) } // formatFast formats the node. diff --git a/go/vt/sqlparser/ast_rewrite.go b/go/vt/sqlparser/ast_rewrite.go index 561fb886144..0ca9fbf25a6 100644 --- a/go/vt/sqlparser/ast_rewrite.go +++ b/go/vt/sqlparser/ast_rewrite.go @@ -2248,8 +2248,8 @@ func (a *application) rewriteRefOfIsExpr(parent SQLNode, node *IsExpr, replacer return true } } - if !a.rewriteExpr(node, node.Expr, func(newNode, parent SQLNode) { - parent.(*IsExpr).Expr = newNode.(Expr) + if !a.rewriteExpr(node, node.Left, func(newNode, parent SQLNode) { + parent.(*IsExpr).Left = newNode.(Expr) }) { return false } diff --git a/go/vt/sqlparser/ast_visit.go b/go/vt/sqlparser/ast_visit.go index cd5b1a58095..d2d1aafa99f 100644 --- a/go/vt/sqlparser/ast_visit.go +++ b/go/vt/sqlparser/ast_visit.go @@ -1206,7 +1206,7 @@ func VisitRefOfIsExpr(in *IsExpr, f Visit) error { if cont, err := f(in); err != nil || !cont { return err } - if err := VisitExpr(in.Expr, f); err != nil { + if err := VisitExpr(in.Left, f); err != nil { return err } return nil diff --git a/go/vt/sqlparser/cached_size.go b/go/vt/sqlparser/cached_size.go index a983fddb4d3..c34d52e057b 100644 --- a/go/vt/sqlparser/cached_size.go +++ b/go/vt/sqlparser/cached_size.go @@ -1176,10 +1176,10 @@ func (cached *IsExpr) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(24) + size += int64(17) } - // field Expr vitess.io/vitess/go/vt/sqlparser.Expr - if cc, ok := cached.Expr.(cachedObject); ok { + // field Left vitess.io/vitess/go/vt/sqlparser.Expr + if cc, ok := cached.Left.(cachedObject); ok { size += cc.CachedSize(true) } return size diff --git a/go/vt/sqlparser/precedence_test.go b/go/vt/sqlparser/precedence_test.go index 7b917f8e698..20ef03e0d3e 100644 --- a/go/vt/sqlparser/precedence_test.go +++ b/go/vt/sqlparser/precedence_test.go @@ -35,7 +35,7 @@ func readable(node Expr) string { case *BinaryExpr: return fmt.Sprintf("(%s %s %s)", readable(node.Left), node.Operator.ToString(), readable(node.Right)) case *IsExpr: - return fmt.Sprintf("(%s %s)", readable(node.Expr), node.Operator.ToString()) + return fmt.Sprintf("(%s %s)", readable(node.Left), node.Right.ToString()) default: return String(node) } diff --git a/go/vt/sqlparser/random_expr.go b/go/vt/sqlparser/random_expr.go index 61ab7ed6536..12d7e44b499 100644 --- a/go/vt/sqlparser/random_expr.go +++ b/go/vt/sqlparser/random_expr.go @@ -314,7 +314,7 @@ func (g *generator) isExpr() Expr { ops := []IsExprOperator{IsNullOp, IsNotNullOp, IsTrueOp, IsNotTrueOp, IsFalseOp, IsNotFalseOp} return &IsExpr{ - Operator: ops[g.r.Intn(len(ops))], - Expr: g.booleanExpr(), + Right: ops[g.r.Intn(len(ops))], + Left: g.booleanExpr(), } } diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index 267c8d0b8ad..9033a48d410 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -10778,7 +10778,7 @@ yydefault: var yyLOCAL Expr //line sql.y:3515 { - yyLOCAL = &IsExpr{Operator: yyDollar[3].isExprOperatorUnion(), Expr: yyDollar[1].exprUnion()} + yyLOCAL = &IsExpr{Right: yyDollar[3].isExprOperatorUnion(), Left: yyDollar[1].exprUnion()} } yyVAL.union = yyLOCAL case 680: diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index 6aa73b910ac..2f580af3bb0 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -3513,7 +3513,7 @@ expression: } | expression IS is_suffix { - $$ = &IsExpr{Operator: $3, Expr: $1} + $$ = &IsExpr{Left: $1, Right: $3} } | value_expression { diff --git a/go/vt/vtgate/planbuilder/route.go b/go/vt/vtgate/planbuilder/route.go index 8267fe9f4e7..56615be5372 100644 --- a/go/vt/vtgate/planbuilder/route.go +++ b/go/vt/vtgate/planbuilder/route.go @@ -668,11 +668,11 @@ func (rb *route) computeEqualPlan(pb *primitiveBuilder, comparison *sqlparser.Co // computeIS computes the plan for an equality constraint. func (rb *route) computeISPlan(pb *primitiveBuilder, comparison *sqlparser.IsExpr) (opcode engine.RouteOpcode, vindex vindexes.SingleColumn, expr sqlparser.Expr) { // we only handle IS NULL correct. IsExpr can contain other expressions as well - if comparison.Operator != sqlparser.IsNullOp { + if comparison.Right != sqlparser.IsNullOp { return engine.SelectScatter, nil, nil } - vindex = pb.st.Vindex(comparison.Expr, rb) + vindex = pb.st.Vindex(comparison.Left, rb) // fallback to scatter gather if there is no vindex if vindex == nil { return engine.SelectScatter, nil, nil From 42a0a9936f16972b2f025eab8144527f7ed20941 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 12:15:20 +0200 Subject: [PATCH 6/8] gen4: plan IS NULL Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/route_planning.go | 69 ++++++++++++------- .../planbuilder/testdata/filter_cases.txt | 1 + 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 563b493bed9..0d708e8c2ab 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -371,11 +371,25 @@ func (rp *routePlan) searchForNewVindexes(predicates []sqlparser.Expr) (bool, er default: return false, semantics.Gen4NotSupportedF("%s", sqlparser.String(filter)) } + case *sqlparser.IsExpr: + found, err := rp.planIsExpr(node) + if err != nil { + return false, err + } + newVindexFound = newVindexFound || found } } return newVindexFound, nil } +func equalOrEqualUnique(vindex *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { + if vindex.Vindex.IsUnique() { + return vindex.Vindex, engine.SelectEqualUnique + } + + return vindex.Vindex, engine.SelectEqual +} + func (rp *routePlan) planEqualOp(node *sqlparser.ComparisonExpr) (bool, error) { column, ok := node.Left.(*sqlparser.ColName) other := node.Right @@ -392,28 +406,7 @@ func (rp *routePlan) planEqualOp(node *sqlparser.ComparisonExpr) (bool, error) { return false, err } - opcode := func(vindex *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { - if vindex.Vindex.IsUnique() { - return vindex.Vindex, engine.SelectEqualUnique - } - - return vindex.Vindex, engine.SelectEqual - } - - return rp.haveMatchingVindex(node, column, *val, opcode), err -} - -func makePlanValue(n sqlparser.Expr) (*sqltypes.PlanValue, error) { - value, err := sqlparser.NewPlanValue(n) - if err != nil { - // if we are unable to create a PlanValue, we can't use a vindex, but we don't have to fail - if strings.Contains(err.Error(), "expression is too complex") { - return nil, nil - } - // something else went wrong, return the error - return nil, err - } - return &value, nil + return rp.haveMatchingVindex(node, column, *val, equalOrEqualUnique), err } func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) { @@ -466,8 +459,38 @@ func (rp *routePlan) planLikeOp(node *sqlparser.ComparisonExpr) (bool, error) { return rp.haveMatchingVindex(node, column, *val, opcode), err } +func (rp *routePlan) planIsExpr(node *sqlparser.IsExpr) (bool, error) { + // we only handle IS NULL correct. IsExpr can contain other expressions as well + if node.Right != sqlparser.IsNullOp { + return false, nil + } + column, ok := node.Left.(*sqlparser.ColName) + if !ok { + return false, nil + } + val, err := makePlanValue(&sqlparser.NullVal{}) + if err != nil || val == nil { + return false, err + } + + return rp.haveMatchingVindex(node, column, *val, equalOrEqualUnique), err +} + +func makePlanValue(n sqlparser.Expr) (*sqltypes.PlanValue, error) { + value, err := sqlparser.NewPlanValue(n) + if err != nil { + // if we are unable to create a PlanValue, we can't use a vindex, but we don't have to fail + if strings.Contains(err.Error(), "expression is too complex") { + return nil, nil + } + // something else went wrong, return the error + return nil, err + } + return &value, nil +} + func (rp *routePlan) haveMatchingVindex( - node *sqlparser.ComparisonExpr, + node sqlparser.Expr, column *sqlparser.ColName, value sqltypes.PlanValue, opcode func(*vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode), diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index 0f4cc3f8423..480314df077 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1664,6 +1664,7 @@ Gen4 plan same as above "Vindex": "music_user_map" } } +Gen4 plan same as above # SELECT with IS NOT NULL "select id from music where id is not null" From c823bc6623b7e2ebb7983def3d3a7eb124f18b88 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 12:32:56 +0200 Subject: [PATCH 7/8] =?UTF-8?q?make=20parser=F0=9F=A4=A6=F0=9F=8F=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Andres Taylor --- go/vt/sqlparser/sql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index 9033a48d410..eb09e595182 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -10778,7 +10778,7 @@ yydefault: var yyLOCAL Expr //line sql.y:3515 { - yyLOCAL = &IsExpr{Right: yyDollar[3].isExprOperatorUnion(), Left: yyDollar[1].exprUnion()} + yyLOCAL = &IsExpr{Left: yyDollar[1].exprUnion(), Right: yyDollar[3].isExprOperatorUnion()} } yyVAL.union = yyLOCAL case 680: From 2c6c50a2c9a7e5a28fc654f59192ec4af43bd725 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 3 Jun 2021 13:24:19 +0200 Subject: [PATCH 8/8] refactor for readability as per review Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/route_planning.go | 35 ++++++++++++---------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 0d708e8c2ab..6ed1768f407 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -382,12 +382,16 @@ func (rp *routePlan) searchForNewVindexes(predicates []sqlparser.Expr) (bool, er return newVindexFound, nil } -func equalOrEqualUnique(vindex *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { +func justTheVindex(vindex *vindexes.ColumnVindex) vindexes.Vindex { + return vindex.Vindex +} + +func equalOrEqualUnique(vindex *vindexes.ColumnVindex) engine.RouteOpcode { if vindex.Vindex.IsUnique() { - return vindex.Vindex, engine.SelectEqualUnique + return engine.SelectEqualUnique } - return vindex.Vindex, engine.SelectEqual + return engine.SelectEqual } func (rp *routePlan) planEqualOp(node *sqlparser.ComparisonExpr) (bool, error) { @@ -406,7 +410,7 @@ func (rp *routePlan) planEqualOp(node *sqlparser.ComparisonExpr) (bool, error) { return false, err } - return rp.haveMatchingVindex(node, column, *val, equalOrEqualUnique), err + return rp.haveMatchingVindex(node, column, *val, equalOrEqualUnique, justTheVindex), err } func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) { @@ -430,10 +434,8 @@ func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) { return false, nil } } - opcode := func(v *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { - return v.Vindex, engine.SelectIN - } - return rp.haveMatchingVindex(node, column, value, opcode), err + opcode := func(*vindexes.ColumnVindex) engine.RouteOpcode { return engine.SelectIN } + return rp.haveMatchingVindex(node, column, value, opcode, justTheVindex), err } func (rp *routePlan) planLikeOp(node *sqlparser.ComparisonExpr) (bool, error) { @@ -447,16 +449,17 @@ func (rp *routePlan) planLikeOp(node *sqlparser.ComparisonExpr) (bool, error) { return false, err } - opcode := func(vindex *vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode) { + selectEqual := func(*vindexes.ColumnVindex) engine.RouteOpcode { return engine.SelectEqual } + vdx := func(vindex *vindexes.ColumnVindex) vindexes.Vindex { if prefixable, ok := vindex.Vindex.(vindexes.Prefixable); ok { - return prefixable.PrefixVindex(), engine.SelectEqual + return prefixable.PrefixVindex() } // if we can't use the vindex as a prefix-vindex, we can't use this vindex at all - return nil, engine.SelectScatter + return nil } - return rp.haveMatchingVindex(node, column, *val, opcode), err + return rp.haveMatchingVindex(node, column, *val, selectEqual, vdx), err } func (rp *routePlan) planIsExpr(node *sqlparser.IsExpr) (bool, error) { @@ -473,7 +476,7 @@ func (rp *routePlan) planIsExpr(node *sqlparser.IsExpr) (bool, error) { return false, err } - return rp.haveMatchingVindex(node, column, *val, equalOrEqualUnique), err + return rp.haveMatchingVindex(node, column, *val, equalOrEqualUnique, justTheVindex), err } func makePlanValue(n sqlparser.Expr) (*sqltypes.PlanValue, error) { @@ -493,7 +496,8 @@ func (rp *routePlan) haveMatchingVindex( node sqlparser.Expr, column *sqlparser.ColName, value sqltypes.PlanValue, - opcode func(*vindexes.ColumnVindex) (vindexes.Vindex, engine.RouteOpcode), + opcode func(*vindexes.ColumnVindex) engine.RouteOpcode, + vfunc func(*vindexes.ColumnVindex) vindexes.Vindex, ) bool { newVindexFound := false for _, v := range rp.vindexPreds { @@ -508,7 +512,8 @@ func (rp *routePlan) haveMatchingVindex( // Vindex is covered if all the columns in the vindex have a associated predicate covered := len(v.values) == len(v.colVindex.Columns) if covered { - v.foundVindex, v.opcode = opcode(v.colVindex) + v.opcode = opcode(v.colVindex) + v.foundVindex = vfunc(v.colVindex) } newVindexFound = newVindexFound || covered }