diff --git a/go/test/endtoend/vtgate/gen4/gen4_test.go b/go/test/endtoend/vtgate/gen4/gen4_test.go index de18e1aa257..bb350f7632a 100644 --- a/go/test/endtoend/vtgate/gen4/gen4_test.go +++ b/go/test/endtoend/vtgate/gen4/gen4_test.go @@ -196,6 +196,28 @@ func TestSubQueries(t *testing.T) { assertMatches(t, conn, `select (select id from t2 order by id limit 1) from t2 order by id limit 2`, `[[INT64(1)] [INT64(1)]]`) } +func TestPlannerWarning(t *testing.T) { + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.NoError(t, err) + defer conn.Close() + + // straight_join query + _ = checkedExec(t, conn, `select 1 from t1 straight_join t2 on t1.id = t2.id`) + assertMatches(t, conn, `show warnings`, `[[VARCHAR("Warning") UINT16(1235) VARCHAR("straight join is converted to normal join")]]`) + + // execute same query again. + _ = checkedExec(t, conn, `select 1 from t1 straight_join t2 on t1.id = t2.id`) + assertMatches(t, conn, `show warnings`, `[[VARCHAR("Warning") UINT16(1235) VARCHAR("straight join is converted to normal join")]]`) + + // random query to reset the warning. + _ = checkedExec(t, conn, `select 1 from t1`) + + // execute same query again. + _ = checkedExec(t, conn, `select 1 from t1 straight_join t2 on t1.id = t2.id`) + assertMatches(t, conn, `show warnings`, `[[VARCHAR("Warning") UINT16(1235) VARCHAR("straight join is converted to normal join")]]`) +} + func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) { t.Helper() qr := checkedExec(t, conn, query) diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go index e471b91147c..2e33f3930cc 100644 --- a/go/vt/vtgate/executor_select_test.go +++ b/go/vt/vtgate/executor_select_test.go @@ -2675,3 +2675,36 @@ func TestSelectScatterFails(t *testing.T) { _, err = executorExecSession(executor, "select /*vt+ ALLOW_SCATTER */ id from user", nil, sess) require.NoError(t, err) } + +func TestGen4SelectStraightJoin(t *testing.T) { + executor, sbc1, _, _ := createExecutorEnv() + executor.normalize = true + *plannerVersion = "gen4" + defer func() { + // change it back to v3 + *plannerVersion = "v3" + }() + + session := NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"}) + query := "select u.id from user u straight_join user2 u2 on u.id = u2.id" + _, err := executor.Execute(context.Background(), + "TestGen4SelectStraightJoin", + session, + query, map[string]*querypb.BindVariable{}, + ) + require.NoError(t, err) + wantQueries := []*querypb.BoundQuery{ + { + Sql: "select u.id from `user` as u, user2 as u2 where u.id = u2.id", + BindVariables: map[string]*querypb.BindVariable{}, + }, + } + wantWarnings := []*querypb.QueryWarning{ + { + Code: 1235, + Message: "straight join is converted to normal join", + }, + } + utils.MustMatch(t, wantQueries, sbc1.Queries) + utils.MustMatch(t, wantWarnings, session.Warnings) +} diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index 8f2cfac35a8..001459ac039 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -109,8 +109,6 @@ func getOperatorFromTableExpr(tableExpr sqlparser.TableExpr, semTable *semantics lhs, rhs = rhs, lhs } return &Join{LHS: lhs, RHS: rhs, LeftJoin: true, Predicate: tableExpr.Condition.On}, nil - case sqlparser.StraightJoinType: - return nil, semantics.Gen4NotSupportedF(tableExpr.Join.ToString()) default: return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: %s", tableExpr.Join.ToString()) } diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index b9f62bb7469..468face549c 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -63,6 +63,9 @@ type ContextVSchema interface { // that could become a problem if they move to a sharded keyspace WarnUnshardedOnly(format string, params ...interface{}) + // PlannerWarning records warning created during planning. + PlannerWarning(message string) + // ForeignKeyMode returns the foreign_key flag value ForeignKeyMode() string } diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index d990bf3bba2..a804f972b22 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -78,6 +78,9 @@ func gen4planSQLCalcFoundRows(vschema ContextVSchema, sel *sqlparser.Select, que if err != nil { return nil, err } + // record any warning as planner warning. + vschema.PlannerWarning(semTable.Warning) + plan, err := buildSQLCalcFoundRowsPlan(query, sel, reservedVars, vschema, planSelectGen4) if err != nil { return nil, err @@ -120,6 +123,8 @@ func newBuildSelectPlan(selStmt sqlparser.SelectStatement, reservedVars *sqlpars if err != nil { return nil, err } + // record any warning as planner warning. + vschema.PlannerWarning(semTable.Warning) err = queryRewrite(semTable, reservedVars, selStmt) if err != nil { diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go index 490bb5acbb4..794c7894bc6 100644 --- a/go/vt/vtgate/planbuilder/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/horizon_planning.go @@ -259,11 +259,12 @@ func pushProjection(expr *sqlparser.AliasedExpr, plan logicalPlan, semTable *sem } return 0, false, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "cannot push projections in ordered aggregates") case *vindexFunc: + colsBefore := len(node.eVindexFunc.Cols) i, err := node.SupplyProjection(expr, reuseCol) if err != nil { return 0, false, err } - return i, true, nil + return i /* col added */, len(node.eVindexFunc.Cols) > colsBefore, nil case *limit: return pushProjection(expr, node.input, semTable, inner, reuseCol) case *distinct: @@ -629,9 +630,10 @@ func (hp *horizonPlanning) planOrderBy(ctx *planningContext, orderExprs []abstra plan.input = newUnderlyingPlan return plan, nil case *simpleProjection: - return hp.createMemorySortPlan(ctx, plan, orderExprs) + return hp.createMemorySortPlan(ctx, plan, orderExprs, true) case *vindexFunc: - return nil, semantics.Gen4NotSupportedF("unsupported: ordering on vindex func") + // This is evaluated at VTGate only, so weight_string function cannot be used. + return hp.createMemorySortPlan(ctx, plan, orderExprs /* useWeightStr */, false) default: return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "ordering on complex query %T", plan) } @@ -760,7 +762,7 @@ func (hp *horizonPlanning) planOrderByForJoin(ctx *planningContext, orderExprs [ plan.Left = newLeft return plan, nil } - sortPlan, err := hp.createMemorySortPlan(ctx, plan, orderExprs) + sortPlan, err := hp.createMemorySortPlan(ctx, plan, orderExprs, true) if err != nil { return nil, err } @@ -807,7 +809,7 @@ func findExprInOrderedAggr(plan *orderedAggregate, order abstract.OrderBy) (int, return 0, 0, false } -func (hp *horizonPlanning) createMemorySortPlan(ctx *planningContext, plan logicalPlan, orderExprs []abstract.OrderBy) (logicalPlan, error) { +func (hp *horizonPlanning) createMemorySortPlan(ctx *planningContext, plan logicalPlan, orderExprs []abstract.OrderBy, useWeightStr bool) (logicalPlan, error) { primitive := &engine.MemorySort{} ms := &memorySort{ resultsBuilder: resultsBuilder{ @@ -819,7 +821,11 @@ func (hp *horizonPlanning) createMemorySortPlan(ctx *planningContext, plan logic } for _, order := range orderExprs { - offset, weightStringOffset, added, err := wrapAndPushExpr(order.Inner.Expr, order.WeightStrExpr, plan, ctx.semTable) + wsExpr := order.WeightStrExpr + if !useWeightStr { + wsExpr = nil + } + offset, weightStringOffset, added, err := wrapAndPushExpr(order.Inner.Expr, wsExpr, plan, ctx.semTable) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index f65fca732e2..1f3953573a6 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -437,6 +437,9 @@ type vschemaWrapper struct { version PlannerVersion } +func (vw *vschemaWrapper) PlannerWarning(_ string) { +} + func (vw *vschemaWrapper) ForeignKeyMode() string { return "allow" } diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt index aab055e22e7..de0cb57ff30 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.txt @@ -784,7 +784,7 @@ Gen4 plan same as above } } -# Straight-join +# Straight-join (Gen4 ignores the straight_join hint) "select m1.col from unsharded as m1 straight_join unsharded as m2" { "QueryType": "SELECT", @@ -801,6 +801,21 @@ Gen4 plan same as above "Table": "unsharded" } } +{ + "QueryType": "SELECT", + "Original": "select m1.col from unsharded as m1 straight_join unsharded as m2", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectUnsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select m1.col from unsharded as m1, unsharded as m2 where 1 != 1", + "Query": "select m1.col from unsharded as m1, unsharded as m2", + "Table": "unsharded" + } +} # Three-way join "select user.col from user join unsharded as m1 join unsharded as m2" diff --git a/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt b/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt index 761e3689b83..d1a32f7f0ea 100644 --- a/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt @@ -706,6 +706,7 @@ Gen4 plan same as above ] } } +Gen4 plan same as above # unary expression "select a from user order by binary a desc" diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index fd2ae5f8405..0dd65ebe7c0 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -42,6 +42,7 @@ type analyzer struct { inProjection int projErr error + warning string } // newAnalyzer create the semantic analyzer @@ -74,7 +75,6 @@ func Analyze(statement sqlparser.SelectStatement, currentDb string, si SchemaInf // Creation of the semantic table semTable := analyzer.newSemTable(statement) - semTable.ProjectionErr = analyzer.projErr return semTable, nil } @@ -86,6 +86,7 @@ func (a analyzer) newSemTable(statement sqlparser.SelectStatement) *SemTable { Tables: a.tables.Tables, selectScope: a.scoper.rScope, ProjectionErr: a.projErr, + Warning: a.warning, Comments: statement.GetComments(), SubqueryMap: a.binder.subqueryMap, SubqueryRef: a.binder.subqueryRef, @@ -126,6 +127,8 @@ func (a *analyzer) analyzeDown(cursor *sqlparser.Cursor) bool { a.setError(err) return true } + // log any warn in rewriting. + a.warning = a.rewriter.warning a.enterProjection(cursor) // this is the visitor going down the tree. Returning false here would just not visit the children diff --git a/go/vt/vtgate/semantics/early_rewriter.go b/go/vt/vtgate/semantics/early_rewriter.go index 613d39c39a5..fcab3a63559 100644 --- a/go/vt/vtgate/semantics/early_rewriter.go +++ b/go/vt/vtgate/semantics/early_rewriter.go @@ -25,8 +25,9 @@ import ( ) type earlyRewriter struct { - scoper *scoper - clause string + scoper *scoper + clause string + warning string } func (r *earlyRewriter) down(cursor *sqlparser.Cursor) error { @@ -59,6 +60,11 @@ func (r *earlyRewriter) down(cursor *sqlparser.Cursor) error { if changed { cursor.ReplaceAndRevisit(selExprs) } + case *sqlparser.JoinTableExpr: + if node.Join == sqlparser.StraightJoinType { + node.Join = sqlparser.NormalJoinType + r.warning = "straight join is converted to normal join" + } case *sqlparser.Order: r.clause = "order clause" case sqlparser.GroupBy: diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 8c252c2b78a..c140c3b631f 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -90,6 +90,8 @@ type ( // ColumnEqualities is used to enable transitive closures // if a == b and b == c then a == c ColumnEqualities map[columnName][]sqlparser.Expr + + Warning string } columnName struct { diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index a63a7697948..5f9b6ae2c3a 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -371,6 +371,7 @@ func (vc *vcursorImpl) TargetString() string { return vc.safeSession.TargetString } +// MaxBufferingRetries is to represent max retries on buffering. const MaxBufferingRetries = 3 func (vc *vcursorImpl) ExecutePrimitive(primitive engine.Primitive, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) { @@ -746,7 +747,18 @@ func (vc *vcursorImpl) WarnUnshardedOnly(format string, params ...interface{}) { } } -// ForeignKey implements the VCursor interface +// PlannerWarning implements the VCursor interface +func (vc *vcursorImpl) PlannerWarning(message string) { + if message == "" { + return + } + vc.warnings = append(vc.warnings, &querypb.QueryWarning{ + Code: mysql.ERNotSupportedYet, + Message: message, + }) +} + +// ForeignKeyMode implements the VCursor interface func (vc *vcursorImpl) ForeignKeyMode() string { if foreignKeyMode == nil { return ""