diff --git a/go/vt/vtgate/planbuilder/abstract/join.go b/go/vt/vtgate/planbuilder/abstract/join.go index 2946d1c9de3..3901d74d987 100644 --- a/go/vt/vtgate/planbuilder/abstract/join.go +++ b/go/vt/vtgate/planbuilder/abstract/join.go @@ -58,7 +58,6 @@ func (j *Join) PushPredicate(expr sqlparser.Expr, semTable *semantics.SemTable) j.LeftJoin = false return j.RHS.PushPredicate(expr, semTable) } - // TODO - we should do this on the vtgate level once we have a Filter primitive return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: cross-shard left join and where clause") case deps.IsSolvedBy(j.LHS.TableID().Merge(j.RHS.TableID())): j.Predicate = sqlparser.AndExpressions(j.Predicate, expr) diff --git a/go/vt/vtgate/planbuilder/fallback_planner.go b/go/vt/vtgate/planbuilder/fallback_planner.go index 85aba2442a6..8127a3ba600 100644 --- a/go/vt/vtgate/planbuilder/fallback_planner.go +++ b/go/vt/vtgate/planbuilder/fallback_planner.go @@ -48,7 +48,7 @@ func (fp *fallbackPlanner) plan(query string) func(sqlparser.Statement, *sqlpars backupF := fp.fallback(query) return func(stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema ContextVSchema) (engine.Primitive, error) { - res, err := primaryF(stmt, reservedVars, vschema) + res, err := primaryF(sqlparser.CloneStatement(stmt), reservedVars, vschema) if err != nil { return backupF(stmt, reservedVars, vschema) } diff --git a/go/vt/vtgate/planbuilder/fallback_planner_test.go b/go/vt/vtgate/planbuilder/fallback_planner_test.go index 7a75a1f2319..cdd31796513 100644 --- a/go/vt/vtgate/planbuilder/fallback_planner_test.go +++ b/go/vt/vtgate/planbuilder/fallback_planner_test.go @@ -28,10 +28,11 @@ import ( ) type testPlanner struct { - panic interface{} - err error - res engine.Primitive - called bool + panic interface{} + err error + res engine.Primitive + messWithAST func(sqlparser.Statement) + called bool } var _ selectPlanner = (*testPlanner)(nil).plan @@ -42,6 +43,9 @@ func (tp *testPlanner) plan(_ string) func(sqlparser.Statement, *sqlparser.Reser if tp.panic != nil { panic(tp.panic) } + if tp.messWithAST != nil { + tp.messWithAST(statement) + } return tp.res, tp.err } } @@ -78,3 +82,27 @@ func TestFallbackPlanner(t *testing.T) { assert.True(t, a.called) assert.True(t, b.called) } + +func TestFallbackClonesBeforePlanning(t *testing.T) { + a := &testPlanner{ + messWithAST: func(statement sqlparser.Statement) { + sel := statement.(*sqlparser.Select) + sel.SelectExprs = nil + }, + } + b := &testPlanner{} + fb := &fallbackPlanner{ + primary: a.plan, + fallback: b.plan, + } + + stmt := &sqlparser.Select{ + SelectExprs: sqlparser.SelectExprs{&sqlparser.StarExpr{}}, + } + var vschema ContextVSchema + + // first planner succeeds + _, _ = fb.plan("query")(stmt, nil, vschema) + + assert.NotNilf(t, stmt.SelectExprs, "should not have changed") +} diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index 76e78365b33..02de8f4d200 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -237,8 +237,8 @@ func planHorizon(ctx *planningContext, plan logicalPlan, in sqlparser.SelectStat case *sqlparser.Union: var err error rb, isRoute := plan.(*routeGen4) - if !isRoute && ctx.semTable.ProjectionErr != nil { - return nil, ctx.semTable.ProjectionErr + if !isRoute && ctx.semTable.ShardedError != nil { + return nil, ctx.semTable.ShardedError } if isRoute && rb.isSingleShard() { err = planSingleShardRoutePlan(node, rb) diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go index e00adc24468..06170f3f2f5 100644 --- a/go/vt/vtgate/planbuilder/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/horizon_planning.go @@ -40,8 +40,8 @@ type horizonPlanning struct { func (hp *horizonPlanning) planHorizon(ctx *planningContext, plan logicalPlan) (logicalPlan, error) { rb, isRoute := plan.(*routeGen4) - if !isRoute && ctx.semTable.ProjectionErr != nil { - return nil, ctx.semTable.ProjectionErr + if !isRoute && ctx.semTable.ShardedError != nil { + return nil, ctx.semTable.ShardedError } if isRoute && rb.isSingleShard() { diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index b755cccd155..d6b98a2ce51 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -4087,3 +4087,22 @@ Gen4 plan same as above "Vindex": "multicolIdx" } } + +# left join with where clause - should be handled by gen4 but still isn't +"select 0 from unsharded_a left join unsharded_b on unsharded_a.col = unsharded_b.col where coalesce(unsharded_b.col, 4) = 5" +{ + "QueryType": "SELECT", + "Original": "select 0 from unsharded_a left join unsharded_b on unsharded_a.col = unsharded_b.col where coalesce(unsharded_b.col, 4) = 5", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectUnsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select 0 from unsharded_a left join unsharded_b on unsharded_a.col = unsharded_b.col where 1 != 1", + "Query": "select 0 from unsharded_a left join unsharded_b on unsharded_a.col = unsharded_b.col where coalesce(unsharded_b.col, 4) = 5", + "Table": "unsharded_a, unsharded_b" + } +} +Gen4 error: unsupported: cross-shard left join and where clause diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index e45f30dc03a..1f07d05ef7b 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -81,7 +81,7 @@ func (a analyzer) newSemTable(statement sqlparser.SelectStatement, coll collatio ExprTypes: a.typer.exprTypes, Tables: a.tables.Tables, selectScope: a.scoper.rScope, - ProjectionErr: a.projErr, + ShardedError: a.projErr, Warning: a.warning, Comments: statement.GetComments(), SubqueryMap: a.binder.subqueryMap, diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 9d631db6677..8c76a7a5fd6 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -397,10 +397,10 @@ func TestUnknownColumnMap2(t *testing.T) { si := &FakeSI{Tables: test.schema} tbl, err := Analyze(parse.(sqlparser.SelectStatement), "", si) if test.err { - require.True(t, err != nil || tbl.ProjectionErr != nil) + require.True(t, err != nil || tbl.ShardedError != nil) } else { require.NoError(t, err) - require.NoError(t, tbl.ProjectionErr) + require.NoError(t, tbl.ShardedError) typ := tbl.TypeFor(expr) assert.Equal(t, test.typ, typ) } @@ -507,7 +507,7 @@ func TestScopeForSubqueries(t *testing.T) { sel2 := sel.SelectExprs[1].(*sqlparser.AliasedExpr).Expr.(*sqlparser.Subquery).Select.(*sqlparser.Select) exp := extract(sel2, 0) s1 := semTable.RecursiveDeps(exp) - require.NoError(t, semTable.ProjectionErr) + require.NoError(t, semTable.ShardedError) // if scoping works as expected, we should be able to see the inner table being used by the inner expression assert.Equal(t, tc.deps, s1) }) diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 68576cdb4a5..e38aea3a2d0 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -70,9 +70,8 @@ type ( // SemTable contains semantic analysis information about the query. SemTable struct { Tables []TableInfo - // ProjectionErr stores the error that we got during the semantic analysis of the SelectExprs. - // This is only a real error if we are unable to plan the query as a single route - ProjectionErr error + // ShardedError stores any errors that have to be generated if the query cannot be planned as a single route. + ShardedError error // Recursive contains the dependencies from the expression to the actual tables // in the query (i.e. not including derived tables). If an expression is a column on a derived table,