From d925a576bf453ebf165edeea23b67dd7265bd2ad Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 21 Aug 2020 17:00:32 +0200 Subject: [PATCH 1/5] Added failing endtoend test Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/misc_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index db8fc24e825..28357f1b2c5 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -276,6 +276,16 @@ func TestXXHash(t *testing.T) { assertMatches(t, conn, "select phone, keyspace_id from t7_xxhash_idx", `[]`) } +func TestShowTablesWithWhereClause(t *testing.T) { + defer cluster.PanicHandler(t) + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.Nil(t, err) + defer conn.Close() + + assertMatches(t, conn, "show tables from ks where Tables_in_ks='t5'", `[[VARCHAR("t5")]]`) +} + func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) { t.Helper() qr := exec(t, conn, query) From 4c4771e239e889a89e72855a0901c7aa58af5f9f Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 21 Aug 2020 17:00:52 +0200 Subject: [PATCH 2/5] refactored out handleShowTables Signed-off-by: Andres Taylor --- go/vt/vtgate/executor.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 22fb205e2dd..f9eb7be1dc0 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -630,14 +630,7 @@ func (e *Executor) handleShow(ctx context.Context, safeSession *SafeSession, sql } sql = sqlparser.String(show) case sqlparser.KeywordString(sqlparser.TABLES): - if show.ShowTablesOpt != nil && show.ShowTablesOpt.DbName != "" { - if destKeyspace == "" { - // Change "show tables from " to "show tables" directed to that keyspace. - destKeyspace = show.ShowTablesOpt.DbName - } - show.ShowTablesOpt.DbName = "" - } - sql = sqlparser.String(show) + destKeyspace, sql = handleShowTables(show, destKeyspace) case sqlparser.KeywordString(sqlparser.DATABASES), "vitess_keyspaces", "keyspaces": keyspaces, err := e.resolver.resolver.GetAllKeyspaces(ctx) if err != nil { @@ -827,6 +820,19 @@ func (e *Executor) handleShow(ctx context.Context, safeSession *SafeSession, sql return e.handleOther(ctx, safeSession, sql, bindVars, dest, destKeyspace, destTabletType, logStats, ignoreMaxMemoryRows) } +func handleShowTables(show *sqlparser.Show, destKeyspace string) (string, string) { + if show.ShowTablesOpt != nil && show.ShowTablesOpt.DbName != "" { + if destKeyspace == "" { + // Change "show tables from " to "show tables" directed to that keyspace. + destKeyspace = show.ShowTablesOpt.DbName + } + show.ShowTablesOpt.DbName = "" + } + + sql := sqlparser.String(show) + return destKeyspace, sql +} + func (e *Executor) showTablets() (*sqltypes.Result, error) { var rows [][]sqltypes.Value if UsingLegacyGateway() { From f5ac4546ce7f14cc8fd2d918fcef564d48a5117a Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 24 Aug 2020 10:42:13 +0200 Subject: [PATCH 3/5] rewrite queries to use database name instead of keyspace Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/misc_test.go | 4 +++- go/vt/sqlparser/ast_funcs.go | 7 +++++++ .../tabletserver/planbuilder/builder.go | 19 +++++++++++++++++++ .../vttablet/tabletserver/planbuilder/plan.go | 18 +++++++++++++++--- .../tabletserver/planbuilder/plan_test.go | 10 +++++----- .../planbuilder/testdata/exec_cases.txt | 16 ++++++++++++++++ go/vt/vttablet/tabletserver/query_engine.go | 2 +- go/vt/vttablet/tabletserver/query_executor.go | 4 ++-- 8 files changed, 68 insertions(+), 12 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 28357f1b2c5..d69e06008b4 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -283,7 +283,9 @@ func TestShowTablesWithWhereClause(t *testing.T) { require.Nil(t, err) defer conn.Close() - assertMatches(t, conn, "show tables from ks where Tables_in_ks='t5'", `[[VARCHAR("t5")]]`) + assertMatches(t, conn, "show tables from ks where Tables_in_ks='t6'", `[[VARCHAR("t6")]]`) + exec(t, conn, "begin") + assertMatches(t, conn, "show tables from ks where Tables_in_ks='t3'", `[[VARCHAR("t3")]]`) } func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) { diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index cd966f82426..2e606d3814e 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -508,6 +508,13 @@ func NewColIdent(str string) ColIdent { } } +// NewColName makes a new ColName +func NewColName(str string) *ColName { + return &ColName{ + Name: NewColIdent(str), + } +} + //NewSelect is used to create a select statement func NewSelect(comments Comments, exprs SelectExprs, selectOptions []string, from TableExprs, where *Where, groupBy GroupBy, having *Where) *Select { var cache *bool diff --git a/go/vt/vttablet/tabletserver/planbuilder/builder.go b/go/vt/vttablet/tabletserver/planbuilder/builder.go index abd09d7d987..4779d2119d8 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/builder.go +++ b/go/vt/vttablet/tabletserver/planbuilder/builder.go @@ -17,6 +17,8 @@ limitations under the License. package planbuilder import ( + "strings" + "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vttablet/tabletserver/schema" @@ -125,6 +127,23 @@ func analyzeInsert(ins *sqlparser.Insert, tables map[string]*schema.Table) (plan return plan, nil } +func analyzeShowTables(show *sqlparser.Show, dbName string) { + // rewrite WHERE clause if it exists + // `where Tables_in_Keyspace` => `where Tables_in_DbName` + filter := show.ShowTablesOpt.Filter.Filter + if filter != nil { + sqlparser.Rewrite(filter, func(cursor *sqlparser.Cursor) bool { + switch n := cursor.Node().(type) { + case *sqlparser.ColName: + if n.Qualifier.IsEmpty() && strings.HasPrefix(n.Name.Lowered(), "tables_in_") { + cursor.Replace(sqlparser.NewColName("Tables_in_" + dbName)) + } + } + return true + }, nil) + } +} + func analyzeSet(set *sqlparser.Set) (plan *Plan) { return &Plan{ PlanID: PlanSet, diff --git a/go/vt/vttablet/tabletserver/planbuilder/plan.go b/go/vt/vttablet/tabletserver/planbuilder/plan.go index 8f1285a97df..3c966791881 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/plan.go +++ b/go/vt/vttablet/tabletserver/planbuilder/plan.go @@ -64,11 +64,12 @@ const ( PlanSavepoint PlanRelease PlanSRollback + PlanShowTables NumPlans ) // Must exactly match order of plan constants. -var planName = [NumPlans]string{ +var planName = []string{ "Select", "SelectLock", "Nextval", @@ -88,6 +89,7 @@ var planName = [NumPlans]string{ "Savepoint", "Release", "RollbackSavepoint", + "ShowTables", } func (pt PlanType) String() string { @@ -151,7 +153,7 @@ func (plan *Plan) TableName() sqlparser.TableIdent { } // Build builds a plan based on the schema. -func Build(statement sqlparser.Statement, tables map[string]*schema.Table, isReservedConn bool) (plan *Plan, err error) { +func Build(statement sqlparser.Statement, tables map[string]*schema.Table, isReservedConn bool, dbName string) (plan *Plan, err error) { if !isReservedConn { err = checkForPoolingUnsafeConstructs(statement) if err != nil { @@ -180,7 +182,17 @@ func Build(statement sqlparser.Statement, tables map[string]*schema.Table, isRes // DDLs and other statements below don't get fully parsed. // We have to use the original query at the time of execution. plan = &Plan{PlanID: PlanDDL} - case *sqlparser.Show, *sqlparser.OtherRead, *sqlparser.Explain: + case *sqlparser.Show: + if stmt.Type == sqlparser.KeywordString(sqlparser.TABLES) { + analyzeShowTables(stmt, dbName) + plan = &Plan{ + PlanID: PlanShowTables, + FullQuery: GenerateFullQuery(stmt), + } + } else { + plan, err = &Plan{PlanID: PlanOtherRead}, nil + } + case *sqlparser.OtherRead, *sqlparser.Explain: plan, err = &Plan{PlanID: PlanOtherRead}, nil case *sqlparser.OtherAdmin: plan, err = &Plan{PlanID: PlanOtherAdmin}, nil diff --git a/go/vt/vttablet/tabletserver/planbuilder/plan_test.go b/go/vt/vttablet/tabletserver/planbuilder/plan_test.go index f1314592c17..34c7e9c87fb 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/plan_test.go +++ b/go/vt/vttablet/tabletserver/planbuilder/plan_test.go @@ -72,7 +72,7 @@ func TestPlan(t *testing.T) { var err error statement, err := sqlparser.Parse(tcase.input) if err == nil { - plan, err = Build(statement, testSchema, false) + plan, err = Build(statement, testSchema, false, "dbName") } PassthroughDMLs = false @@ -109,7 +109,7 @@ func TestPlanPoolUnsafe(t *testing.T) { statement, err := sqlparser.Parse(tcase.input) require.NoError(t, err) // In Pooled Connection, plan building will fail. - plan, err = Build(statement, testSchema, false /* isReservedConn */) + plan, err = Build(statement, testSchema, false /* isReservedConn */, "dbName") require.Error(t, err) out := err.Error() if out != tcase.output { @@ -123,7 +123,7 @@ func TestPlanPoolUnsafe(t *testing.T) { fmt.Printf("\"%s\"\n%s\n\n", tcase.input, out) } // In Reserved Connection, plan will be built. - plan, err = Build(statement, testSchema, true /* isReservedConn */) + plan, err = Build(statement, testSchema, true /* isReservedConn */, "dbName") require.NoError(t, err) require.NotEmpty(t, plan) }) @@ -141,7 +141,7 @@ func TestPlanInReservedConn(t *testing.T) { var err error statement, err := sqlparser.Parse(tcase.input) if err == nil { - plan, err = Build(statement, testSchema, true) + plan, err = Build(statement, testSchema, true, "dbName") } PassthroughDMLs = false @@ -192,7 +192,7 @@ func TestCustom(t *testing.T) { if err != nil { t.Fatalf("Got error: %v, parsing sql: %v", err.Error(), tcase.input) } - plan, err := Build(statement, schem, false) + plan, err := Build(statement, schem, false, "dbName") var out string if err != nil { out = err.Error() diff --git a/go/vt/vttablet/tabletserver/planbuilder/testdata/exec_cases.txt b/go/vt/vttablet/tabletserver/planbuilder/testdata/exec_cases.txt index 185afbafd84..79f9063187e 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/testdata/exec_cases.txt +++ b/go/vt/vttablet/tabletserver/planbuilder/testdata/exec_cases.txt @@ -787,3 +787,19 @@ options:PassthroughDMLs # syntax error "syntax error" "syntax error at position 7 near 'syntax'" + +# show tables #1 +"show tables like 'key%'" +{ + "PlanID": "ShowTables", + "TableName":"", + "FullQuery": "show tables like 'key%'" +} + +# show tables #2 +"show tables where Tables_in_keyspace='apa'" +{ + "PlanID": "ShowTables", + "TableName":"", + "FullQuery": "show tables where Tables_in_dbName = 'apa'" +} diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go index cfe94e972e6..14870a630d6 100644 --- a/go/vt/vttablet/tabletserver/query_engine.go +++ b/go/vt/vttablet/tabletserver/query_engine.go @@ -309,7 +309,7 @@ func (qe *QueryEngine) GetPlan(ctx context.Context, logStats *tabletenv.LogStats if err != nil { return nil, err } - splan, err := planbuilder.Build(statement, qe.tables, isReservedConn) + splan, err := planbuilder.Build(statement, qe.tables, isReservedConn, qe.env.Config().DB.DBName) if err != nil { return nil, err } diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index b2056eee21d..2cd757df3a7 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -121,7 +121,7 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) { } switch qre.plan.PlanID { - case planbuilder.PlanSelect, planbuilder.PlanSelectImpossible: + case planbuilder.PlanSelect, planbuilder.PlanSelectImpossible, planbuilder.PlanShowTables: maxrows := qre.getSelectLimit() qre.bindVars["#maxLimit"] = sqltypes.Int64BindVariable(maxrows + 1) qr, err := qre.execSelect() @@ -203,7 +203,7 @@ func (qre *QueryExecutor) txConnExec(conn *StatefulConnection) (*sqltypes.Result return qre.execSQL(conn, qre.query, true) case planbuilder.PlanSavepoint, planbuilder.PlanRelease, planbuilder.PlanSRollback: return qre.execSQL(conn, qre.query, true) - case planbuilder.PlanSelect, planbuilder.PlanSelectLock, planbuilder.PlanSelectImpossible: + case planbuilder.PlanSelect, planbuilder.PlanSelectLock, planbuilder.PlanSelectImpossible, planbuilder.PlanShowTables: maxrows := qre.getSelectLimit() qre.bindVars["#maxLimit"] = sqltypes.Int64BindVariable(maxrows + 1) qr, err := qre.txFetch(conn, false) From cb937899f26c7a86762b94991dc349a226b67af0 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 24 Aug 2020 15:36:07 +0530 Subject: [PATCH 4/5] added nil check before applying colname replace Signed-off-by: Harshit Gangal --- .../tabletserver/planbuilder/builder.go | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/go/vt/vttablet/tabletserver/planbuilder/builder.go b/go/vt/vttablet/tabletserver/planbuilder/builder.go index 4779d2119d8..54eb4c11653 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/builder.go +++ b/go/vt/vttablet/tabletserver/planbuilder/builder.go @@ -130,17 +130,19 @@ func analyzeInsert(ins *sqlparser.Insert, tables map[string]*schema.Table) (plan func analyzeShowTables(show *sqlparser.Show, dbName string) { // rewrite WHERE clause if it exists // `where Tables_in_Keyspace` => `where Tables_in_DbName` - filter := show.ShowTablesOpt.Filter.Filter - if filter != nil { - sqlparser.Rewrite(filter, func(cursor *sqlparser.Cursor) bool { - switch n := cursor.Node().(type) { - case *sqlparser.ColName: - if n.Qualifier.IsEmpty() && strings.HasPrefix(n.Name.Lowered(), "tables_in_") { - cursor.Replace(sqlparser.NewColName("Tables_in_" + dbName)) + if show.ShowTablesOpt != nil && show.ShowTablesOpt.Filter != nil { + filter := show.ShowTablesOpt.Filter.Filter + if filter != nil { + sqlparser.Rewrite(filter, func(cursor *sqlparser.Cursor) bool { + switch n := cursor.Node().(type) { + case *sqlparser.ColName: + if n.Qualifier.IsEmpty() && strings.HasPrefix(n.Name.Lowered(), "tables_in_") { + cursor.Replace(sqlparser.NewColName("Tables_in_" + dbName)) + } } - } - return true - }, nil) + return true + }, nil) + } } } From b38f847b0fe5f00467d77d2f086a51dd2e4c6ed7 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 26 Aug 2020 19:44:51 +0530 Subject: [PATCH 5/5] revert executor changes Signed-off-by: Harshit Gangal --- go/vt/vtgate/executor.go | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index f9eb7be1dc0..22fb205e2dd 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -630,7 +630,14 @@ func (e *Executor) handleShow(ctx context.Context, safeSession *SafeSession, sql } sql = sqlparser.String(show) case sqlparser.KeywordString(sqlparser.TABLES): - destKeyspace, sql = handleShowTables(show, destKeyspace) + if show.ShowTablesOpt != nil && show.ShowTablesOpt.DbName != "" { + if destKeyspace == "" { + // Change "show tables from " to "show tables" directed to that keyspace. + destKeyspace = show.ShowTablesOpt.DbName + } + show.ShowTablesOpt.DbName = "" + } + sql = sqlparser.String(show) case sqlparser.KeywordString(sqlparser.DATABASES), "vitess_keyspaces", "keyspaces": keyspaces, err := e.resolver.resolver.GetAllKeyspaces(ctx) if err != nil { @@ -820,19 +827,6 @@ func (e *Executor) handleShow(ctx context.Context, safeSession *SafeSession, sql return e.handleOther(ctx, safeSession, sql, bindVars, dest, destKeyspace, destTabletType, logStats, ignoreMaxMemoryRows) } -func handleShowTables(show *sqlparser.Show, destKeyspace string) (string, string) { - if show.ShowTablesOpt != nil && show.ShowTablesOpt.DbName != "" { - if destKeyspace == "" { - // Change "show tables from " to "show tables" directed to that keyspace. - destKeyspace = show.ShowTablesOpt.DbName - } - show.ShowTablesOpt.DbName = "" - } - - sql := sqlparser.String(show) - return destKeyspace, sql -} - func (e *Executor) showTablets() (*sqltypes.Result, error) { var rows [][]sqltypes.Value if UsingLegacyGateway() {