Skip to content

Commit

Permalink
some basic refactor and test fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
  • Loading branch information
@harshit-gangal
harshit-gangal committed Jan 25, 2024
1 parent ae2d1f5 commit 888e373
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 111 deletions.
188 changes: 90 additions & 98 deletions go/vt/vtgate/executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1156,97 +1156,6 @@ func TestExecutorComment(t *testing.T) {
}
}

func TestExecutorOther(t *testing.T) {
executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t)

type cnts struct {
Sbc1Cnt int64
Sbc2Cnt int64
SbcLookupCnt int64
}

tcs := []struct {
targetStr string

hasNoKeyspaceErr bool
hasDestinationShardErr bool
wantCnts cnts
}{
{
targetStr: "",
hasNoKeyspaceErr: true,
},
{
targetStr: "TestExecutor[-]",
hasDestinationShardErr: true,
},
{
targetStr: KsTestUnsharded,
wantCnts: cnts{
Sbc1Cnt: 0,
Sbc2Cnt: 0,
SbcLookupCnt: 1,
},
},
{
targetStr: "TestExecutor",
wantCnts: cnts{
Sbc1Cnt: 1,
Sbc2Cnt: 0,
SbcLookupCnt: 0,
},
},
{
targetStr: "TestExecutor/-20",
wantCnts: cnts{
Sbc1Cnt: 1,
Sbc2Cnt: 0,
SbcLookupCnt: 0,
},
},
{
targetStr: "TestExecutor[00]",
wantCnts: cnts{
Sbc1Cnt: 1,
Sbc2Cnt: 0,
SbcLookupCnt: 0,
},
},
}

stmts := []string{
"describe select * from t1",
"explain select * from t1",
"repair table t1",
"optimize table t1",
}

for _, stmt := range stmts {
for _, tc := range tcs {
t.Run(fmt.Sprintf("%s-%s", stmt, tc.targetStr), func(t *testing.T) {
sbc1.ExecCount.Store(0)
sbc2.ExecCount.Store(0)
sbclookup.ExecCount.Store(0)

_, err := executor.Execute(ctx, nil, "TestExecute", NewSafeSession(&vtgatepb.Session{TargetString: tc.targetStr}), stmt, nil)
if tc.hasNoKeyspaceErr {
assert.Error(t, err, errNoKeyspace)
} else if tc.hasDestinationShardErr {
assert.Errorf(t, err, "Destination can only be a single shard for statement: %s", stmt)
} else {
assert.NoError(t, err)
}

utils.MustMatch(t, tc.wantCnts, cnts{
Sbc1Cnt: sbc1.ExecCount.Load(),
Sbc2Cnt: sbc2.ExecCount.Load(),
SbcLookupCnt: sbclookup.ExecCount.Load(),
})
})
}
}
}

func TestExecutorDDL(t *testing.T) {
executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t)

Expand Down Expand Up @@ -2146,8 +2055,8 @@ func TestServingKeyspaces(t *testing.T) {
require.Equal(t, `[[VARCHAR("TestUnsharded")]]`, fmt.Sprintf("%v", result.Rows))
}

func TestExecutorOtherRead(t *testing.T) {
executor, sbc1, sbc2, sbclookup, _ := createExecutorEnv(t)
func TestExecutorOther(t *testing.T) {
executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t)

type cnts struct {
Sbc1Cnt int64
Expand Down Expand Up @@ -2186,26 +2095,44 @@ func TestExecutorOtherRead(t *testing.T) {
SbcLookupCnt: 0,
},
},
{
targetStr: "TestExecutor/-20",
wantCnts: cnts{
Sbc1Cnt: 1,
Sbc2Cnt: 0,
SbcLookupCnt: 0,
},
},
{
targetStr: "TestExecutor[00]",
wantCnts: cnts{
Sbc1Cnt: 1,
Sbc2Cnt: 0,
SbcLookupCnt: 0,
},
},
}

stmts := []string{
"describe select * from t1",
"explain select * from t1",
"repair table t1",
"optimize table t1",
"do 1",
}

for _, stmt := range stmts {
for _, tc := range tcs {
t.Run(stmt+tc.targetStr, func(t *testing.T) {
t.Run(fmt.Sprintf("%s-%s", stmt, tc.targetStr), func(t *testing.T) {
sbc1.ExecCount.Store(0)
sbc2.ExecCount.Store(0)
sbclookup.ExecCount.Store(0)

_, err := executor.Execute(context.Background(), nil, "TestExecute", NewSafeSession(&vtgatepb.Session{TargetString: tc.targetStr}), stmt, nil)
_, err := executor.Execute(ctx, nil, "TestExecute", NewSafeSession(&vtgatepb.Session{TargetString: tc.targetStr}), stmt, nil)
if tc.hasNoKeyspaceErr {
assert.EqualError(t, err, errNoKeyspace.Error())
assert.Error(t, err, errNoKeyspace)
} else if tc.hasDestinationShardErr {
assert.Errorf(t, err, "Destination can only be a single shard for statement: %s, got: DestinationExactKeyRange(-)", stmt)
assert.Errorf(t, err, "Destination can only be a single shard for statement: %s", stmt)
} else {
assert.NoError(t, err)
}
Expand All @@ -2214,7 +2141,7 @@ func TestExecutorOtherRead(t *testing.T) {
Sbc1Cnt: sbc1.ExecCount.Load(),
Sbc2Cnt: sbc2.ExecCount.Load(),
SbcLookupCnt: sbclookup.ExecCount.Load(),
}, "count did not match")
})
})
}
}
Expand Down Expand Up @@ -2269,6 +2196,71 @@ func TestExecutorAnalyze(t *testing.T) {
}
}

func TestExecutorExplainStmt(t *testing.T) {
executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t)

type cnts struct {
Sbc1Cnt int64
Sbc2Cnt int64
SbcLookupCnt int64
}

tcs := []struct {
targetStr string

wantCnts cnts
}{
{
targetStr: "",
wantCnts: cnts{Sbc1Cnt: 1},
},
{
targetStr: "TestExecutor[-]",
wantCnts: cnts{Sbc1Cnt: 1, Sbc2Cnt: 1},
},
{
targetStr: KsTestUnsharded,
wantCnts: cnts{SbcLookupCnt: 1},
},
{
targetStr: "TestExecutor",
wantCnts: cnts{Sbc1Cnt: 1},
},
{
targetStr: "TestExecutor/-20",
wantCnts: cnts{Sbc1Cnt: 1},
},
{
targetStr: "TestExecutor[00]",
wantCnts: cnts{Sbc1Cnt: 1},
},
}

stmts := []string{
"describe select * from t1",
"explain select * from t1",
}

for _, stmt := range stmts {
for _, tc := range tcs {
t.Run(fmt.Sprintf("%s-%s", stmt, tc.targetStr), func(t *testing.T) {
sbc1.ExecCount.Store(0)
sbc2.ExecCount.Store(0)
sbclookup.ExecCount.Store(0)

_, err := executor.Execute(ctx, nil, "TestExecute", NewSafeSession(&vtgatepb.Session{TargetString: tc.targetStr}), stmt, nil)
assert.NoError(t, err)

utils.MustMatch(t, tc.wantCnts, cnts{
Sbc1Cnt: sbc1.ExecCount.Load(),
Sbc2Cnt: sbc2.ExecCount.Load(),
SbcLookupCnt: sbclookup.ExecCount.Load(),
})
})
}
}
}

func TestExecutorVExplain(t *testing.T) {
executor, _, _, _, ctx := createExecutorEnv(t)

Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Stat
case *sqlparser.ExplainTab:
return explainTabPlan(stmt, vschema)
case *sqlparser.ExplainStmt:
return buildExplainStmtPlan(stmt, reservedVars, vschema)
return buildRoutePlan(stmt, reservedVars, vschema, buildExplainStmtPlan)
case *sqlparser.VExplainStmt:
return buildVExplainPlan(ctx, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
case *sqlparser.OtherAdmin:
Expand Down
15 changes: 4 additions & 11 deletions go/vt/vtgate/planbuilder/vexplain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,18 @@ func buildVExplainLoggingPlan(ctx context.Context, explain *sqlparser.VExplainSt
}

// buildExplainStmtPlan takes an EXPLAIN query and if possible sends the whole query to a single shard
func buildExplainStmtPlan(
explain *sqlparser.ExplainStmt,
reservedVars *sqlparser.ReservedVars,
vschema plancontext.VSchema,
) (*planResult, error) {
func buildExplainStmtPlan(stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema) (*planResult, error) {
explain := stmt.(*sqlparser.ExplainStmt)
switch explain.Statement.(type) {
case sqlparser.SelectStatement, *sqlparser.Update, *sqlparser.Delete, *sqlparser.Insert:
return explainSelectPlan(explain, reservedVars, vschema)
return explainPlan(explain, reservedVars, vschema)
default:
return buildOtherReadAndAdmin(sqlparser.String(explain), vschema)
}

}

func explainDefaultPlan(explain *sqlparser.ExplainStmt, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema) (*planResult, error) {
func explainPlan(explain *sqlparser.ExplainStmt, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema) (*planResult, error) {
ctx, err := plancontext.CreatePlanningContext(explain.Statement, reservedVars, vschema, Gen4)
if err != nil {
return nil, err
Expand Down Expand Up @@ -156,7 +153,3 @@ func explainDefaultPlan(explain *sqlparser.ExplainStmt, reservedVars *sqlparser.
SingleShardOnly: true,
}, tables...), nil
}

func explainSelectPlan(explain *sqlparser.ExplainStmt, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema) (*planResult, error) {
return explainDefaultPlan(explain, reservedVars, vschema)
}
2 changes: 1 addition & 1 deletion go/vt/vtgate/testdata/executorVSchema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@
"type": "reference",
"source": "TestUnsharded.zip_detail"
},
"multicol_tbl": {
"multicol_tbl": {
"column_vindexes": [
{
"columns": ["cola", "colb"],
Expand Down

0 comments on commit 888e373

Please sign in to comment.