Skip to content

Commit

Permalink
Merge pull request #8382 from planetscale/gen4-fail-more2
Browse files Browse the repository at this point in the history
Gen4 fail more2
  • Loading branch information
systay authored Jun 28, 2021
2 parents e49f910 + e62a676 commit 6dd97de
Show file tree
Hide file tree
Showing 25 changed files with 502 additions and 75 deletions.
21 changes: 11 additions & 10 deletions go/vt/sqlparser/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,17 @@ type (
Distinct bool
StraightJoinHint bool
SQLCalcFoundRows bool
From []TableExpr
Comments Comments
SelectExprs SelectExprs
Where *Where
GroupBy GroupBy
Having *Where
OrderBy OrderBy
Limit *Limit
Lock Lock
Into *SelectInto
// The From field must be the first AST element of this struct so the rewriter sees it first
From []TableExpr
Comments Comments
SelectExprs SelectExprs
Where *Where
GroupBy GroupBy
Having *Where
OrderBy OrderBy
Limit *Limit
Lock Lock
Into *SelectInto
}

// SelectInto is a struct that represent the INTO part of a select query
Expand Down
4 changes: 1 addition & 3 deletions go/vt/vtgate/planbuilder/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package planbuilder

import (
"errors"

"vitess.io/vitess/go/vt/vtgate/semantics"

vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
Expand Down Expand Up @@ -100,7 +98,7 @@ func newJoin(lpb, rpb *primitiveBuilder, ajoin *sqlparser.JoinTableExpr, reserve
return err
}
case ajoin.Condition.Using != nil:
return errors.New("unsupported: join with USING(column_list) clause for complex queries")
return vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: join with USING(column_list) clause for complex queries")
}
}
lpb.plan = &join{
Expand Down
120 changes: 103 additions & 17 deletions go/vt/vtgate/planbuilder/jointree.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,20 +141,31 @@ func (p parenTables) tableID() semantics.TableSet {
}

// visit will traverse the route tables, going inside parenTables and visiting all routeTables
func (p parenTables) visit(f func(tbl *routeTable) error) error {
for _, r := range p {
switch r := r.(type) {
case *routeTable:
err := f(r)
if err != nil {
return err
}
case parenTables:
err := r.visit(f)
func visitTables(r relation, f func(tbl *routeTable) error) error {
switch r := r.(type) {
case *routeTable:
err := f(r)
if err != nil {
return err
}
case parenTables:
for _, r := range r {
err := visitTables(r, f)
if err != nil {
return err
}
}
return nil
case *leJoin:
err := visitTables(r.lhs, f)
if err != nil {
return err
}
err = visitTables(r.rhs, f)
if err != nil {
return err
}
return nil
}
return nil
}
Expand Down Expand Up @@ -355,11 +366,7 @@ func (rp *routePlan) planEqualOp(node *sqlparser.ComparisonExpr) (bool, error) {
return rp.haveMatchingVindex(node, column, *val, equalOrEqualUnique, justTheVindex), err
}

func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) {
column, ok := node.Left.(*sqlparser.ColName)
if !ok {
return false, nil
}
func (rp *routePlan) planSimpleInOp(node *sqlparser.ComparisonExpr, left *sqlparser.ColName) (bool, error) {
value, err := sqlparser.NewPlanValue(node.Right)
if err != nil {
// if we are unable to create a PlanValue, we can't use a vindex, but we don't have to fail
Expand All @@ -377,7 +384,70 @@ func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) {
}
}
opcode := func(*vindexes.ColumnVindex) engine.RouteOpcode { return engine.SelectIN }
return rp.haveMatchingVindex(node, column, value, opcode, justTheVindex), err
return rp.haveMatchingVindex(node, left, value, opcode, justTheVindex), err
}

func (rp *routePlan) planCompositeInOp(node *sqlparser.ComparisonExpr, left sqlparser.ValTuple) (bool, error) {
right, rightIsValTuple := node.Right.(sqlparser.ValTuple)
if !rightIsValTuple {
return false, nil
}
return rp.planCompositeInOpRecursive(node, left, right, nil)
}

func (rp *routePlan) planCompositeInOpRecursive(node *sqlparser.ComparisonExpr, left, right sqlparser.ValTuple, coordinates []int) (bool, error) {
foundVindex := false
cindex := len(coordinates)
coordinates = append(coordinates, 0)
for i, expr := range left {
coordinates[cindex] = i
switch expr := expr.(type) {
case sqlparser.ValTuple:
ok, err := rp.planCompositeInOpRecursive(node, expr, right, coordinates)
if err != nil {
return false, err
}
return ok || foundVindex, nil
case *sqlparser.ColName:
// check if left col is a vindex
if !rp.hasVindex(expr) {
continue
}

rightVals := make(sqlparser.ValTuple, len(right))
for j, currRight := range right {
switch currRight := currRight.(type) {
case sqlparser.ValTuple:
val := tupleAccess(currRight, coordinates)
if val == nil {
return false, nil
}
rightVals[j] = val
default:
return false, nil
}
}
newPlanValues, err := makePlanValue(rightVals)
if newPlanValues == nil || err != nil {
return false, err
}

opcode := func(*vindexes.ColumnVindex) engine.RouteOpcode { return engine.SelectMultiEqual }
newVindex := rp.haveMatchingVindex(node, expr, *newPlanValues, opcode, justTheVindex)
foundVindex = newVindex || foundVindex
}
}
return foundVindex, nil
}

func (rp *routePlan) planInOp(node *sqlparser.ComparisonExpr) (bool, error) {
switch left := node.Left.(type) {
case *sqlparser.ColName:
return rp.planSimpleInOp(node, left)
case sqlparser.ValTuple:
return rp.planCompositeInOp(node, left)
}
return false, nil
}

func (rp *routePlan) planLikeOp(node *sqlparser.ComparisonExpr) (bool, error) {
Expand Down Expand Up @@ -434,6 +504,17 @@ func makePlanValue(n sqlparser.Expr) (*sqltypes.PlanValue, error) {
return &value, nil
}

func (rp routePlan) hasVindex(column *sqlparser.ColName) bool {
for _, v := range rp.vindexPreds {
for _, col := range v.colVindex.Columns {
if column.Name.Equal(col) {
return true
}
}
}
return false
}

func (rp *routePlan) haveMatchingVindex(
node sqlparser.Expr,
column *sqlparser.ColName,
Expand Down Expand Up @@ -482,7 +563,12 @@ func (rp *routePlan) pickBestAvailableVindex() {

// Predicates takes all known predicates for this route and ANDs them together
func (rp *routePlan) Predicates() sqlparser.Expr {
return sqlparser.AndExpressions(rp.predicates...)
predicates := rp.predicates
_ = visitTables(rp.tables, func(tbl *routeTable) error {
predicates = append(predicates, tbl.qtable.Predicates...)
return nil
})
return sqlparser.AndExpressions(predicates...)
}

func (rp *routePlan) pushOutputColumns(col []*sqlparser.ColName, _ *semantics.SemTable) []int {
Expand Down
5 changes: 4 additions & 1 deletion go/vt/vtgate/planbuilder/jointree_transformers.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ func transformRoutePlan(n *routePlan) (*route, error) {
switch predicate := predicate.(type) {
case *sqlparser.ComparisonExpr:
if predicate.Operator == sqlparser.InOp {
predicate.Right = sqlparser.ListArg(engine.ListVarName)
switch predicate.Left.(type) {
case *sqlparser.ColName:
predicate.Right = sqlparser.ListArg(engine.ListVarName)
}
}
}
}
Expand Down
20 changes: 15 additions & 5 deletions go/vt/vtgate/planbuilder/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ func init() {
vindexes.Register("costly", newCostlyIndex)
}

const samePlanMarker = "Gen4 plan same as above\n"
const (
samePlanMarker = "Gen4 plan same as above\n"
gen4ErrorPrefix = "Gen4 error: "
)

func TestPlan(t *testing.T) {
vschemaWrapper := &vschemaWrapper{
Expand Down Expand Up @@ -235,7 +238,11 @@ func TestWithDefaultKeyspaceFromFile(t *testing.T) {
// We are testing this separately so we can set a default keyspace
testOutputTempDir, err := ioutil.TempDir("", "plan_test")
require.NoError(t, err)
defer os.RemoveAll(testOutputTempDir)
defer func() {
if !t.Failed() {
_ = os.RemoveAll(testOutputTempDir)
}
}()
vschema := &vschemaWrapper{
v: loadSchema(t, "schema_test.json"),
keyspace: &vindexes.Keyspace{
Expand Down Expand Up @@ -607,9 +614,11 @@ func iterateExecFile(name string) (testCaseIterator chan testCase) {
if err != nil && err != io.EOF {
panic(fmt.Sprintf("error reading file %s line# %d: %s", name, lineno, err.Error()))
}
if len(binput) > 0 && string(binput) == samePlanMarker {
nextLine := string(binput)
switch {
case nextLine == samePlanMarker:
output2Planner = output
} else if len(binput) > 0 && (binput[0] == '"' || binput[0] == '{') {
case strings.HasPrefix(nextLine, "{"):
output2Planner = append(output2Planner, binput...)
for {
l, err := r.ReadBytes('\n')
Expand All @@ -627,8 +636,9 @@ func iterateExecFile(name string) (testCaseIterator chan testCase) {
break
}
}
case strings.HasPrefix(nextLine, gen4ErrorPrefix):
output2Planner = []byte(nextLine[len(gen4ErrorPrefix) : len(nextLine)-1])
}

testCaseIterator <- testCase{
file: name,
lineno: lineno,
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vtgate/planbuilder/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"errors"

querypb "vitess.io/vitess/go/vt/proto/query"
"vitess.io/vitess/go/vt/proto/vtrpc"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vterrors"
"vitess.io/vitess/go/vt/vtgate/engine"
Expand All @@ -46,7 +46,7 @@ func planProjection(pb *primitiveBuilder, in logicalPlan, expr *sqlparser.Aliase
} else {
// Pushing of non-trivial expressions not allowed for RHS of left joins.
if _, ok := expr.Expr.(*sqlparser.ColName); !ok && node.ejoin.Opcode == engine.LeftJoin {
return nil, nil, 0, errors.New("unsupported: cross-shard left join and column expressions")
return nil, nil, 0, vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: cross-shard left join and column expressions")
}

newRight, col, colNumber, err := planProjection(pb, node.Right, expr, origin)
Expand Down Expand Up @@ -167,5 +167,5 @@ func planProjection(pb *primitiveBuilder, in logicalPlan, expr *sqlparser.Aliase
return node, rc, len(node.resultColumns) - 1, nil

}
return nil, nil, 0, vterrors.Errorf(vtrpc.Code_UNIMPLEMENTED, "[BUG] unreachable %T.projection", in)
return nil, nil, 0, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "[BUG] unreachable %T.projection", in)
}
7 changes: 6 additions & 1 deletion go/vt/vtgate/planbuilder/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,12 @@ func (rb *route) computeCompositeINPlan(pb *primitiveBuilder, comparison *sqlpar
// iterateCompositeIN recursively walks the LHS tuple of the IN clause looking
// for column names. For those that match a vindex, it builds a multi-value plan
// using the corresponding values in the RHS. It returns the best of the plans built.
func (rb *route) iterateCompositeIN(pb *primitiveBuilder, comparison *sqlparser.ComparisonExpr, coordinates []int, tuple sqlparser.ValTuple) (opcode engine.RouteOpcode, vindex vindexes.SingleColumn, values sqlparser.Expr) {
func (rb *route) iterateCompositeIN(
pb *primitiveBuilder,
comparison *sqlparser.ComparisonExpr,
coordinates []int,
tuple sqlparser.ValTuple,
) (opcode engine.RouteOpcode, vindex vindexes.SingleColumn, values sqlparser.Expr) {
opcode = engine.SelectScatter

cindex := len(coordinates)
Expand Down
13 changes: 11 additions & 2 deletions go/vt/vtgate/planbuilder/route_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ func gen4Planner(_ string) func(sqlparser.Statement, *sqlparser.ReservedVars, Co
}

func newBuildSelectPlan(sel *sqlparser.Select, vschema ContextVSchema) (engine.Primitive, error) {

directives := sqlparser.ExtractCommentDirectives(sel.Comments)
if len(directives) > 0 {
return nil, semantics.Gen4NotSupportedF("comment directives")
}
keyspace, err := vschema.DefaultKeyspace()
if err != nil {
return nil, err
Expand Down Expand Up @@ -256,6 +261,10 @@ func planLimit(limit *sqlparser.Limit, plan logicalPlan) (logicalPlan, error) {

func planHorizon(sel *sqlparser.Select, plan logicalPlan, semTable *semantics.SemTable) (logicalPlan, error) {
rb, ok := plan.(*route)
if !ok && semTable.ProjectionErr != nil {
return nil, semTable.ProjectionErr
}

if ok && rb.isSingleShard() {
createSingleShardRoutePlan(sel, rb)
return plan, nil
Expand All @@ -270,7 +279,7 @@ func planHorizon(sel *sqlparser.Select, plan logicalPlan, semTable *semantics.Se
return nil, err
}
for _, e := range qp.selectExprs {
if _, err := pushProjection(e, plan, semTable); err != nil {
if _, err := pushProjection(e, plan, semTable, true); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -625,7 +634,7 @@ func findColumnVindex(a *routePlan, exp sqlparser.Expr, sem *semantics.SemTable)

var singCol vindexes.SingleColumn

_ = a.tables.visit(func(table *routeTable) error {
_ = visitTables(a.tables, func(table *routeTable) error {
if leftDep.IsSolvedBy(table.qtable.TableID) {
for _, vindex := range table.vtable.ColumnVindexes {
sC, isSingle := vindex.Vindex.(vindexes.SingleColumn)
Expand Down
Loading

0 comments on commit 6dd97de

Please sign in to comment.