Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gen4: support for ordering on vindex function and straight_join queries to be planned by ignoring the hint as warning #8990

Merged
merged 4 commits into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions go/test/endtoend/vtgate/gen4/gen4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 33 additions & 0 deletions go/vt/vtgate/executor_select_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 0 additions & 2 deletions go/vt/vtgate/planbuilder/abstract/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/planbuilder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions go/vt/vtgate/planbuilder/gen4_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 12 additions & 6 deletions go/vt/vtgate/planbuilder/horizon_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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{
Expand All @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/planbuilder/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,9 @@ type vschemaWrapper struct {
version PlannerVersion
}

func (vw *vschemaWrapper) PlannerWarning(_ string) {
}

func (vw *vschemaWrapper) ForeignKeyMode() string {
return "allow"
}
Expand Down
17 changes: 16 additions & 1 deletion go/vt/vtgate/planbuilder/testdata/from_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion go/vt/vtgate/semantics/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type analyzer struct {
inProjection int

projErr error
warning string
}

// newAnalyzer create the semantic analyzer
Expand Down Expand Up @@ -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
}

Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions go/vt/vtgate/semantics/early_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions go/vt/vtgate/semantics/semantic_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 13 additions & 1 deletion go/vt/vtgate/vcursor_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 ""
Expand Down