From c7f8d4f4268f19e5137fb7c4774e58759c9069c6 Mon Sep 17 00:00:00 2001 From: GuptaManan100 Date: Wed, 17 Feb 2021 14:00:24 +0530 Subject: [PATCH 1/3] Added another case to merge routes for information_schema queries Signed-off-by: GuptaManan100 --- go/vt/sqlparser/ast.go | 3 + go/vt/sqlparser/ast_funcs.go | 71 +++++++++++++++++++ go/vt/vtgate/planbuilder/route.go | 26 +++++-- .../planbuilder/testdata/select_cases.txt | 19 +++++ 4 files changed, 115 insertions(+), 4 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 52b8e39e8e4..aa4bdf79055 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -52,6 +52,7 @@ type ( SetLimit(*Limit) SetLock(lock Lock) MakeDistinct() + ContainsOnlyInformationSchema() bool } // DDLStatement represents any DDL Statement @@ -1446,6 +1447,7 @@ type ( // TableExpr represents a table expression. TableExpr interface { iTableExpr() + ContainsOnlyInformationSchema() bool SQLNode } @@ -1484,6 +1486,7 @@ type ( // SimpleTableExpr represents a simple table expression. SimpleTableExpr interface { iSimpleTableExpr() + ContainsOnlyInformationSchema() bool SQLNode } diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index bdd00bce729..4a7adfa117c 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -668,6 +668,13 @@ func (node TableIdent) String() string { return node.v } +// String returns the unescaped table name with qualifier. It must +// not be used for SQL generation. Use sqlparser.String +// instead. +func (node TableName) String() string { + return strings.ToLower(node.Qualifier.String() + "." + node.Name.String()) +} + // CompliantName returns a compliant id name // that can be used for a bind var. func (node TableIdent) CompliantName() string { @@ -764,6 +771,52 @@ func (node *Select) MakeDistinct() { node.Distinct = true } +// ContainsOnlyInformationSchema returns if the select statement only contains information_schema tables +func (node *Select) ContainsOnlyInformationSchema() bool { + for _, expr := range node.From { + if !expr.ContainsOnlyInformationSchema() { + return false + } + } + return true +} + +// ContainsOnlyInformationSchema returns if the expression only contains information_schema tables +func (expr *ParenTableExpr) ContainsOnlyInformationSchema() bool { + for _, tableExpr := range expr.Exprs { + if !tableExpr.ContainsOnlyInformationSchema() { + return false + } + } + return true +} + +// ContainsOnlyInformationSchema returns if the expression only contains information_schema tables +func (expr *JoinTableExpr) ContainsOnlyInformationSchema() bool { + if !expr.LeftExpr.ContainsOnlyInformationSchema() { + return false + } + if !expr.RightExpr.ContainsOnlyInformationSchema() { + return false + } + return true +} + +// ContainsOnlyInformationSchema returns if the expression only contains information_schema tables +func (node *AliasedTableExpr) ContainsOnlyInformationSchema() bool { + return node.Expr.ContainsOnlyInformationSchema() +} + +// ContainsOnlyInformationSchema returns if the table is from information_schema +func (node *DerivedTable) ContainsOnlyInformationSchema() bool { + return node.Select.ContainsOnlyInformationSchema() +} + +// ContainsOnlyInformationSchema returns if the table is from information_schema +func (node TableName) ContainsOnlyInformationSchema() bool { + return strings.ToLower(node.Qualifier.String()) == "information_schema" +} + // AddWhere adds the boolean expression to the // WHERE clause as an AND condition. func (node *Select) AddWhere(expr Expr) { @@ -816,6 +869,11 @@ func (node *ParenSelect) MakeDistinct() { node.Select.MakeDistinct() } +// ContainsOnlyInformationSchema returns if the select statement only contains information_schema tables +func (node *ParenSelect) ContainsOnlyInformationSchema() bool { + return node.Select.ContainsOnlyInformationSchema() +} + // AddWhere adds the boolean expression to the // WHERE clause as an AND condition. func (node *Update) AddWhere(expr Expr) { @@ -852,6 +910,19 @@ func (node *Union) MakeDistinct() { node.UnionSelects[len(node.UnionSelects)-1].Distinct = true } +// ContainsOnlyInformationSchema returns if the union statement only contains information_schema tables +func (node *Union) ContainsOnlyInformationSchema() bool { + if !node.FirstStatement.ContainsOnlyInformationSchema() { + return false + } + for _, unionSelect := range node.UnionSelects { + if !unionSelect.Statement.ContainsOnlyInformationSchema() { + return false + } + } + return true +} + //Unionize returns a UNION, either creating one or adding SELECT to an existing one func Unionize(lhs, rhs SelectStatement, distinct bool, by OrderBy, limit *Limit, lock Lock) *Union { union, isUnion := lhs.(*Union) diff --git a/go/vt/vtgate/planbuilder/route.go b/go/vt/vtgate/planbuilder/route.go index 6998a16b313..6ae22bfed54 100644 --- a/go/vt/vtgate/planbuilder/route.go +++ b/go/vt/vtgate/planbuilder/route.go @@ -440,15 +440,28 @@ func (rb *route) JoinCanMerge(pb *primitiveBuilder, rrb *route, ajoin *sqlparser if where == nil { return true } - hasRuntimeRoutingPredicates := false + var tableWithRoutingPredicates sqlparser.TableNames _ = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { col, ok := node.(*sqlparser.ColName) if ok { - hasRuntimeRoutingPredicates = hasRuntimeRoutingPredicates || isTableNameCol(col) || isDbNameCol(col) + hasRuntimeRoutingPredicates := isTableNameCol(col) || isDbNameCol(col) + if hasRuntimeRoutingPredicates { + for _, table := range tableWithRoutingPredicates { + if table.String() == col.Qualifier.String() { + return true, nil + } + } + tableWithRoutingPredicates = append(tableWithRoutingPredicates, col.Qualifier) + } } - return !hasRuntimeRoutingPredicates, nil + return true, nil }, where) - return !hasRuntimeRoutingPredicates + // Routes can be merged if only tables from information schema are accessed and only 1 table is used in the predicates that are used for routing + // TODO :- Even if more table are present in the routing, we can merge if they agree + if rb.ContainsOnlyInformationSchema() && rrb.ContainsOnlyInformationSchema() && len(tableWithRoutingPredicates) <= 1 { + return true + } + return len(tableWithRoutingPredicates) == 0 } if ajoin == nil { return false @@ -820,3 +833,8 @@ func queryTimeout(d sqlparser.CommentDirectives) int { } return 0 } + +// ContainsOnlyInformationSchema returns if the route only contains information_schema tables +func (rb *route) ContainsOnlyInformationSchema() bool { + return rb.Select.ContainsOnlyInformationSchema() +} diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt index 42ef6bf233f..812805e2e49 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt @@ -1696,3 +1696,22 @@ Gen4 plan same as above ] } } + +# Select from information schema query with two tables that route should be merged +"SELECT DELETE_RULE, UPDATE_RULE FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS KCU INNER JOIN INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS AS RC ON KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME WHERE KCU.TABLE_SCHEMA = 'test' AND KCU.TABLE_NAME = 'data_type_table' AND KCU.COLUMN_NAME = 'id' AND KCU.REFERENCED_TABLE_SCHEMA = 'test' AND KCU.CONSTRAINT_NAME = 'data_type_table_id_fkey' ORDER BY KCU.CONSTRAINT_NAME, KCU.COLUMN_NAME" +{ + "QueryType": "SELECT", + "Original": "SELECT DELETE_RULE, UPDATE_RULE FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS KCU INNER JOIN INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS AS RC ON KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME WHERE KCU.TABLE_SCHEMA = 'test' AND KCU.TABLE_NAME = 'data_type_table' AND KCU.COLUMN_NAME = 'id' AND KCU.REFERENCED_TABLE_SCHEMA = 'test' AND KCU.CONSTRAINT_NAME = 'data_type_table_id_fkey' ORDER BY KCU.CONSTRAINT_NAME, KCU.COLUMN_NAME", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectDBA", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select DELETE_RULE, UPDATE_RULE from INFORMATION_SCHEMA.KEY_COLUMN_USAGE as KCU join INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS as RC on KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME where 1 != 1", + "Query": "select DELETE_RULE, UPDATE_RULE from INFORMATION_SCHEMA.KEY_COLUMN_USAGE as KCU join INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS as RC on KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME where KCU.TABLE_SCHEMA = :__vtschemaname and KCU.TABLE_NAME = :__vttablename and KCU.COLUMN_NAME = 'id' and KCU.REFERENCED_TABLE_SCHEMA = 'test' and KCU.CONSTRAINT_NAME = 'data_type_table_id_fkey' order by KCU.CONSTRAINT_NAME asc, KCU.COLUMN_NAME asc", + "SysTableTableName": "VARBINARY(\"data_type_table\")", + "SysTableTableSchema": "VARBINARY(\"test\")" + } +} From 4cd2a7aa1edebf3e1d1395635dce51fefc5f0888 Mon Sep 17 00:00:00 2001 From: Akilan Selvacoumar Date: Thu, 18 Feb 2021 11:27:54 +0400 Subject: [PATCH 2/3] simplified opt code instead of using AST Signed-off-by: Akilan Selvacoumar Signed-off-by: --- go/vt/sqlparser/ast.go | 3 -- go/vt/sqlparser/ast_funcs.go | 64 ------------------------------ go/vt/sqlparser/cached_size.go | 4 +- go/vt/vtgate/engine/cached_size.go | 2 +- go/vt/vtgate/planbuilder/route.go | 2 +- 5 files changed, 4 insertions(+), 71 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index aa4bdf79055..52b8e39e8e4 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -52,7 +52,6 @@ type ( SetLimit(*Limit) SetLock(lock Lock) MakeDistinct() - ContainsOnlyInformationSchema() bool } // DDLStatement represents any DDL Statement @@ -1447,7 +1446,6 @@ type ( // TableExpr represents a table expression. TableExpr interface { iTableExpr() - ContainsOnlyInformationSchema() bool SQLNode } @@ -1486,7 +1484,6 @@ type ( // SimpleTableExpr represents a simple table expression. SimpleTableExpr interface { iSimpleTableExpr() - ContainsOnlyInformationSchema() bool SQLNode } diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 4a7adfa117c..850013d0cc5 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -771,52 +771,6 @@ func (node *Select) MakeDistinct() { node.Distinct = true } -// ContainsOnlyInformationSchema returns if the select statement only contains information_schema tables -func (node *Select) ContainsOnlyInformationSchema() bool { - for _, expr := range node.From { - if !expr.ContainsOnlyInformationSchema() { - return false - } - } - return true -} - -// ContainsOnlyInformationSchema returns if the expression only contains information_schema tables -func (expr *ParenTableExpr) ContainsOnlyInformationSchema() bool { - for _, tableExpr := range expr.Exprs { - if !tableExpr.ContainsOnlyInformationSchema() { - return false - } - } - return true -} - -// ContainsOnlyInformationSchema returns if the expression only contains information_schema tables -func (expr *JoinTableExpr) ContainsOnlyInformationSchema() bool { - if !expr.LeftExpr.ContainsOnlyInformationSchema() { - return false - } - if !expr.RightExpr.ContainsOnlyInformationSchema() { - return false - } - return true -} - -// ContainsOnlyInformationSchema returns if the expression only contains information_schema tables -func (node *AliasedTableExpr) ContainsOnlyInformationSchema() bool { - return node.Expr.ContainsOnlyInformationSchema() -} - -// ContainsOnlyInformationSchema returns if the table is from information_schema -func (node *DerivedTable) ContainsOnlyInformationSchema() bool { - return node.Select.ContainsOnlyInformationSchema() -} - -// ContainsOnlyInformationSchema returns if the table is from information_schema -func (node TableName) ContainsOnlyInformationSchema() bool { - return strings.ToLower(node.Qualifier.String()) == "information_schema" -} - // AddWhere adds the boolean expression to the // WHERE clause as an AND condition. func (node *Select) AddWhere(expr Expr) { @@ -869,11 +823,6 @@ func (node *ParenSelect) MakeDistinct() { node.Select.MakeDistinct() } -// ContainsOnlyInformationSchema returns if the select statement only contains information_schema tables -func (node *ParenSelect) ContainsOnlyInformationSchema() bool { - return node.Select.ContainsOnlyInformationSchema() -} - // AddWhere adds the boolean expression to the // WHERE clause as an AND condition. func (node *Update) AddWhere(expr Expr) { @@ -910,19 +859,6 @@ func (node *Union) MakeDistinct() { node.UnionSelects[len(node.UnionSelects)-1].Distinct = true } -// ContainsOnlyInformationSchema returns if the union statement only contains information_schema tables -func (node *Union) ContainsOnlyInformationSchema() bool { - if !node.FirstStatement.ContainsOnlyInformationSchema() { - return false - } - for _, unionSelect := range node.UnionSelects { - if !unionSelect.Statement.ContainsOnlyInformationSchema() { - return false - } - } - return true -} - //Unionize returns a UNION, either creating one or adding SELECT to an existing one func Unionize(lhs, rhs SelectStatement, distinct bool, by OrderBy, limit *Limit, lock Lock) *Union { union, isUnion := lhs.(*Union) diff --git a/go/vt/sqlparser/cached_size.go b/go/vt/sqlparser/cached_size.go index ad4cbe32662..30b1b680026 100644 --- a/go/vt/sqlparser/cached_size.go +++ b/go/vt/sqlparser/cached_size.go @@ -553,7 +553,7 @@ func (cached *CreateTable) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(57) + size += int64(65) } // field Table vitess.io/vitess/go/vt/sqlparser.TableName size += cached.Table.CachedSize(false) @@ -666,7 +666,7 @@ func (cached *DropTable) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(25) + size += int64(33) } // field FromTables vitess.io/vitess/go/vt/sqlparser.TableNames { diff --git a/go/vt/vtgate/engine/cached_size.go b/go/vt/vtgate/engine/cached_size.go index 136ce2b84f1..2278127a81c 100644 --- a/go/vt/vtgate/engine/cached_size.go +++ b/go/vt/vtgate/engine/cached_size.go @@ -78,7 +78,7 @@ func (cached *DDL) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(56) + size += int64(57) } // field Keyspace *vitess.io/vitess/go/vt/vtgate/vindexes.Keyspace size += cached.Keyspace.CachedSize(true) diff --git a/go/vt/vtgate/planbuilder/route.go b/go/vt/vtgate/planbuilder/route.go index 6ae22bfed54..b2755337d12 100644 --- a/go/vt/vtgate/planbuilder/route.go +++ b/go/vt/vtgate/planbuilder/route.go @@ -836,5 +836,5 @@ func queryTimeout(d sqlparser.CommentDirectives) int { // ContainsOnlyInformationSchema returns if the route only contains information_schema tables func (rb *route) ContainsOnlyInformationSchema() bool { - return rb.Select.ContainsOnlyInformationSchema() + return rb.eroute.Opcode == engine.SelectDBA } From 41c0f2ffa55dc9d7cf015d38d151b04d2b80e8b2 Mon Sep 17 00:00:00 2001 From: GuptaManan100 Date: Fri, 19 Feb 2021 14:41:02 +0530 Subject: [PATCH 3/3] cleanup and addition of one more test Signed-off-by: GuptaManan100 --- go/vt/sqlparser/ast_funcs.go | 7 ---- go/vt/vtgate/planbuilder/route.go | 20 +++------- .../planbuilder/testdata/select_cases.txt | 39 +++++++++++++++++++ 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 850013d0cc5..bdd00bce729 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -668,13 +668,6 @@ func (node TableIdent) String() string { return node.v } -// String returns the unescaped table name with qualifier. It must -// not be used for SQL generation. Use sqlparser.String -// instead. -func (node TableName) String() string { - return strings.ToLower(node.Qualifier.String() + "." + node.Name.String()) -} - // CompliantName returns a compliant id name // that can be used for a bind var. func (node TableIdent) CompliantName() string { diff --git a/go/vt/vtgate/planbuilder/route.go b/go/vt/vtgate/planbuilder/route.go index b2755337d12..dda0667986c 100644 --- a/go/vt/vtgate/planbuilder/route.go +++ b/go/vt/vtgate/planbuilder/route.go @@ -440,25 +440,20 @@ func (rb *route) JoinCanMerge(pb *primitiveBuilder, rrb *route, ajoin *sqlparser if where == nil { return true } - var tableWithRoutingPredicates sqlparser.TableNames + tableWithRoutingPredicates := make(map[sqlparser.TableName]struct{}) _ = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { col, ok := node.(*sqlparser.ColName) if ok { hasRuntimeRoutingPredicates := isTableNameCol(col) || isDbNameCol(col) - if hasRuntimeRoutingPredicates { - for _, table := range tableWithRoutingPredicates { - if table.String() == col.Qualifier.String() { - return true, nil - } - } - tableWithRoutingPredicates = append(tableWithRoutingPredicates, col.Qualifier) + if hasRuntimeRoutingPredicates && pb.st.tables[col.Qualifier] != nil { + tableWithRoutingPredicates[col.Qualifier] = struct{}{} } } return true, nil }, where) - // Routes can be merged if only tables from information schema are accessed and only 1 table is used in the predicates that are used for routing + // Routes can be merged if only 1 table is used in the predicates that are used for routing // TODO :- Even if more table are present in the routing, we can merge if they agree - if rb.ContainsOnlyInformationSchema() && rrb.ContainsOnlyInformationSchema() && len(tableWithRoutingPredicates) <= 1 { + if len(tableWithRoutingPredicates) <= 1 { return true } return len(tableWithRoutingPredicates) == 0 @@ -833,8 +828,3 @@ func queryTimeout(d sqlparser.CommentDirectives) int { } return 0 } - -// ContainsOnlyInformationSchema returns if the route only contains information_schema tables -func (rb *route) ContainsOnlyInformationSchema() bool { - return rb.eroute.Opcode == engine.SelectDBA -} diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt index 812805e2e49..5df10dc59ae 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt @@ -1715,3 +1715,42 @@ Gen4 plan same as above "SysTableTableSchema": "VARBINARY(\"test\")" } } + +# Select from information schema query with three tables such that route for 2 should be merged but not for the last. +"SELECT KCU.DELETE_RULE, S.UPDATE_RULE FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS KCU INNER JOIN INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS AS RC ON KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME, INFORMATION_SCHEMA.K AS S WHERE KCU.TABLE_SCHEMA = 'test' AND KCU.TABLE_NAME = 'data_type_table' AND KCU.TABLE_NAME = 'data_type_table' AND S.TABLE_SCHEMA = 'test' AND S.TABLE_NAME = 'sc' ORDER BY KCU.CONSTRAINT_NAME, KCU.COLUMN_NAME" +{ + "QueryType": "SELECT", + "Original": "SELECT KCU.DELETE_RULE, S.UPDATE_RULE FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS KCU INNER JOIN INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS AS RC ON KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME, INFORMATION_SCHEMA.K AS S WHERE KCU.TABLE_SCHEMA = 'test' AND KCU.TABLE_NAME = 'data_type_table' AND KCU.TABLE_NAME = 'data_type_table' AND S.TABLE_SCHEMA = 'test' AND S.TABLE_NAME = 'sc' ORDER BY KCU.CONSTRAINT_NAME, KCU.COLUMN_NAME", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-1,1", + "TableName": "_", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectDBA", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select KCU.DELETE_RULE from INFORMATION_SCHEMA.KEY_COLUMN_USAGE as KCU join INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS as RC on KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME where 1 != 1", + "Query": "select KCU.DELETE_RULE from INFORMATION_SCHEMA.KEY_COLUMN_USAGE as KCU join INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS as RC on KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME where KCU.TABLE_SCHEMA = :__vtschemaname and KCU.TABLE_NAME = :__vttablename and KCU.TABLE_NAME = :__vttablename order by KCU.CONSTRAINT_NAME asc, KCU.COLUMN_NAME asc", + "SysTableTableName": "VARBINARY(\"data_type_table\")", + "SysTableTableSchema": "VARBINARY(\"test\")" + }, + { + "OperatorType": "Route", + "Variant": "SelectDBA", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select S.UPDATE_RULE from INFORMATION_SCHEMA.K as S where 1 != 1", + "Query": "select S.UPDATE_RULE from INFORMATION_SCHEMA.K as S where S.TABLE_SCHEMA = :__vtschemaname and S.TABLE_NAME = :__vttablename", + "SysTableTableName": "VARBINARY(\"sc\")", + "SysTableTableSchema": "VARBINARY(\"test\")" + } + ] + } +}