From 1d67a905777daa5a7913a34aafbca2060bc7816c Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Thu, 25 Jan 2018 06:15:23 -0800 Subject: [PATCH] bug: DML_SUBQUERY should suppress index hints BUG=72403076 Since DML_SUBQUERY rewrites updates to use pk, it should suppress index hints, because those should only be used by the SELECT. --- data/test/tabletserver/exec_cases.txt | 12 ++++++++++++ go/vt/sqlparser/ast.go | 7 +++++++ go/vt/sqlparser/ast_test.go | 16 ++++++++++++++++ go/vt/vttablet/tabletserver/planbuilder/dml.go | 12 +++++++----- .../tabletserver/planbuilder/query_gen.go | 8 ++++---- 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/data/test/tabletserver/exec_cases.txt b/data/test/tabletserver/exec_cases.txt index 133d8df567d..eeb0131ff5b 100644 --- a/data/test/tabletserver/exec_cases.txt +++ b/data/test/tabletserver/exec_cases.txt @@ -908,6 +908,18 @@ options:PassthroughDMLs "WhereClause": " where eid = 1" } +# update with index hint +# note that you won't find a corresponding test for delete because the grammar doesn't allow it. +"update a use index(b) set name = 'foo' where eid=1" +{ + "PlanID": "DML_SUBQUERY", + "TableName": "a", + "FullQuery": "update a use index (b) set name = 'foo' where eid = 1", + "OuterQuery": "update a set name = 'foo' where :#pk", + "Subquery": "select eid, id from a use index (b) where eid = 1 limit :#maxLimit for update", + "WhereClause": " where eid = 1" +} + # delete limit with pk "delete from d where name in ('a', 'b') limit 1" { diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 9be8f2aac3a..4ed6d6d689b 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -1383,6 +1383,13 @@ func (node *AliasedTableExpr) WalkSubtree(visit Visit) error { ) } +// RemoveHints returns a new AliasedTableExpr with the hints removed. +func (node *AliasedTableExpr) RemoveHints() *AliasedTableExpr { + noHints := *node + noHints.Hints = nil + return &noHints +} + // SimpleTableExpr represents a simple table expression. type SimpleTableExpr interface { iSimpleTableExpr() diff --git a/go/vt/sqlparser/ast_test.go b/go/vt/sqlparser/ast_test.go index 3d2eb9311a8..73b25579ec4 100644 --- a/go/vt/sqlparser/ast_test.go +++ b/go/vt/sqlparser/ast_test.go @@ -109,6 +109,22 @@ func TestSelect(t *testing.T) { } } +func TestRemoveHints(t *testing.T) { + tree, err := Parse("select * from t use index (i)") + if err != nil { + t.Fatal(err) + } + sel := tree.(*Select) + sel.From = TableExprs{ + sel.From[0].(*AliasedTableExpr).RemoveHints(), + } + buf := NewTrackedBuffer(nil) + sel.Format(buf) + if got, want := buf.String(), "select * from t"; got != want { + t.Errorf("stripped query: %s, want %s", got, want) + } +} + func TestAddOrder(t *testing.T) { src, err := Parse("select foo, bar from baz order by foo") if err != nil { diff --git a/go/vt/vttablet/tabletserver/planbuilder/dml.go b/go/vt/vttablet/tabletserver/planbuilder/dml.go index 39f4b874411..febf365a665 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/dml.go +++ b/go/vt/vttablet/tabletserver/planbuilder/dml.go @@ -20,10 +20,11 @@ import ( log "github.com/golang/glog" "github.com/youtube/vitess/go/sqltypes" - vtrpcpb "github.com/youtube/vitess/go/vt/proto/vtrpc" "github.com/youtube/vitess/go/vt/sqlparser" "github.com/youtube/vitess/go/vt/vterrors" "github.com/youtube/vitess/go/vt/vttablet/tabletserver/schema" + + vtrpcpb "github.com/youtube/vitess/go/vt/proto/vtrpc" ) func analyzeUpdate(upd *sqlparser.Update, tables map[string]*schema.Table) (plan *Plan, err error) { @@ -77,7 +78,7 @@ func analyzeUpdate(upd *sqlparser.Update, tables map[string]*schema.Table) (plan return nil, err } - plan.OuterQuery = GenerateUpdateOuterQuery(upd, nil) + plan.OuterQuery = GenerateUpdateOuterQuery(upd, aliased, nil) if pkValues := analyzeWhere(upd.Where, table.Indexes[0]); pkValues != nil { // Also, there should be no limit clause. @@ -133,7 +134,7 @@ func analyzeDelete(del *sqlparser.Delete, tables map[string]*schema.Table) (plan return plan, nil } - plan.OuterQuery = GenerateDeleteOuterQuery(del) + plan.OuterQuery = GenerateDeleteOuterQuery(del, aliased) if pkValues := analyzeWhere(del.Where, table.Indexes[0]); pkValues != nil { // Also, there should be no limit clause. @@ -421,15 +422,16 @@ func analyzeInsertNoType(ins *sqlparser.Insert, plan *Plan, table *schema.Table) newins.Ignore = "" newins.OnDup = nil plan.OuterQuery = sqlparser.NewParsedQuery(&newins) + tableAlias := &sqlparser.AliasedTableExpr{Expr: ins.Table} upd := &sqlparser.Update{ Comments: ins.Comments, - TableExprs: sqlparser.TableExprs{&sqlparser.AliasedTableExpr{Expr: ins.Table}}, + TableExprs: sqlparser.TableExprs{tableAlias}, Exprs: sqlparser.UpdateExprs(ins.OnDup), } // We need to replace 'values' expressions with the actual values they reference. var formatErr error - plan.UpsertQuery = GenerateUpdateOuterQuery(upd, func(buf *sqlparser.TrackedBuffer, node sqlparser.SQLNode) { + plan.UpsertQuery = GenerateUpdateOuterQuery(upd, tableAlias, func(buf *sqlparser.TrackedBuffer, node sqlparser.SQLNode) { if node, ok := node.(*sqlparser.ValuesFuncExpr); ok { colnum := ins.Columns.FindColumn(node.Name) if colnum == -1 { diff --git a/go/vt/vttablet/tabletserver/planbuilder/query_gen.go b/go/vt/vttablet/tabletserver/planbuilder/query_gen.go index 90a11a43d60..0cc8fc6f48b 100644 --- a/go/vt/vttablet/tabletserver/planbuilder/query_gen.go +++ b/go/vt/vttablet/tabletserver/planbuilder/query_gen.go @@ -82,16 +82,16 @@ func GenerateInsertOuterQuery(ins *sqlparser.Insert) *sqlparser.ParsedQuery { // GenerateUpdateOuterQuery generates the outer query for updates. // If there is no custom formatting needed, formatter can be nil. -func GenerateUpdateOuterQuery(upd *sqlparser.Update, formatter sqlparser.NodeFormatter) *sqlparser.ParsedQuery { +func GenerateUpdateOuterQuery(upd *sqlparser.Update, aliased *sqlparser.AliasedTableExpr, formatter sqlparser.NodeFormatter) *sqlparser.ParsedQuery { buf := sqlparser.NewTrackedBuffer(formatter) - buf.Myprintf("update %v%v set %v where %a%v", upd.Comments, upd.TableExprs, upd.Exprs, ":#pk", upd.OrderBy) + buf.Myprintf("update %v%v set %v where %a%v", upd.Comments, aliased.RemoveHints(), upd.Exprs, ":#pk", upd.OrderBy) return buf.ParsedQuery() } // GenerateDeleteOuterQuery generates the outer query for deletes. -func GenerateDeleteOuterQuery(del *sqlparser.Delete) *sqlparser.ParsedQuery { +func GenerateDeleteOuterQuery(del *sqlparser.Delete, aliased *sqlparser.AliasedTableExpr) *sqlparser.ParsedQuery { buf := sqlparser.NewTrackedBuffer(nil) - buf.Myprintf("delete %vfrom %v where %a%v", del.Comments, del.TableExprs, ":#pk", del.OrderBy) + buf.Myprintf("delete %vfrom %v where %a%v", del.Comments, aliased.RemoveHints(), ":#pk", del.OrderBy) return buf.ParsedQuery() }