diff --git a/changelog/20.0/20.0.0/summary.md b/changelog/20.0/20.0.0/summary.md index c947ea2a0aa..f1280ba7e76 100644 --- a/changelog/20.0/20.0.0/summary.md +++ b/changelog/20.0/20.0.0/summary.md @@ -3,15 +3,17 @@ ### Table of Contents - **[Major Changes](#major-changes)** - - **[Query Serving](#query-serving)** + - **[Query Compatibility](#query-compatibility)** - [Vindex Hints](#vindex-hints) + - [Update with Limit Support](#update-limit) + - [Update with Multi Table Support](#multi-table-update) - **[Minor Changes](#minor-changes)** ## Major Changes -### Query Serving +### Query Compatibility #### Vindex Hints @@ -25,5 +27,22 @@ Example: For more information about Vindex hints and its usage, please consult the documentation. +#### Update with Limit Support + +Support is added for sharded update with limit. + +Example: `update t1 set t1.foo = 'abc', t1.bar = 23 where t1.baz > 5 limit 1` + +More details about how it works is available in [MySQL Docs](https://dev.mysql.com/doc/refman/8.0/en/update.html) + +#### Update with Multi Table Support + +Support is added for sharded multi-table update with column update on single target table using multiple table join. + +Example: `update t1 join t2 on t1.id = t2.id join t3 on t1.col = t3.col set t1.baz = 'abc', t1.apa = 23 where t3.foo = 5 and t2.bar = 7` + +More details about how it works is available in [MySQL Docs](https://dev.mysql.com/doc/refman/8.0/en/update.html) + + ## Minor Changes diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 3dd376ff228..0e50d71b77c 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -53,16 +53,17 @@ type ( Commented } + OrderAndLimit interface { + AddOrder(*Order) + SetLimit(*Limit) + } + // SelectStatement any SELECT statement. SelectStatement interface { Statement InsertRows + OrderAndLimit iSelectStatement() - AddOrder(*Order) - SetOrderBy(OrderBy) - GetOrderBy() OrderBy - GetLimit() *Limit - SetLimit(*Limit) GetLock() Lock SetLock(lock Lock) SetInto(into *SelectInto) @@ -72,6 +73,9 @@ type ( GetColumns() SelectExprs Commented IsDistinct() bool + GetOrderBy() OrderBy + SetOrderBy(OrderBy) + GetLimit() *Limit } // DDLStatement represents any DDL Statement @@ -712,6 +716,10 @@ type ( IndexType int8 ) +var _ OrderAndLimit = (*Select)(nil) +var _ OrderAndLimit = (*Update)(nil) +var _ OrderAndLimit = (*Delete)(nil) + func (*Union) iStatement() {} func (*Select) iStatement() {} func (*Stream) iStatement() {} diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index ecbfef2ff66..a8d231d6aa1 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -2605,3 +2605,84 @@ func MultiTable(node []TableExpr) bool { _, singleTbl := node[0].(*AliasedTableExpr) return !singleTbl } + +func (node *Update) AddOrder(order *Order) { + node.OrderBy = append(node.OrderBy, order) +} + +func (node *Update) SetLimit(limit *Limit) { + node.Limit = limit +} + +func (node *Delete) AddOrder(order *Order) { + node.OrderBy = append(node.OrderBy, order) +} + +func (node *Delete) SetLimit(limit *Limit) { + node.Limit = limit +} + +func (node *Select) GetFrom() []TableExpr { + return node.From +} + +func (node *Select) SetFrom(exprs []TableExpr) { + node.From = exprs +} + +func (node *Select) GetWherePredicate() Expr { + if node.Where == nil { + return nil + } + return node.Where.Expr +} + +func (node *Select) SetWherePredicate(expr Expr) { + node.Where = &Where{ + Type: WhereClause, + Expr: expr, + } +} +func (node *Delete) GetFrom() []TableExpr { + return node.TableExprs +} + +func (node *Delete) SetFrom(exprs []TableExpr) { + node.TableExprs = exprs +} + +func (node *Delete) GetWherePredicate() Expr { + if node.Where == nil { + return nil + } + return node.Where.Expr +} + +func (node *Delete) SetWherePredicate(expr Expr) { + node.Where = &Where{ + Type: WhereClause, + Expr: expr, + } +} + +func (node *Update) GetFrom() []TableExpr { + return node.TableExprs +} + +func (node *Update) SetFrom(exprs []TableExpr) { + node.TableExprs = exprs +} + +func (node *Update) GetWherePredicate() Expr { + if node.Where == nil { + return nil + } + return node.Where.Expr +} + +func (node *Update) SetWherePredicate(expr Expr) { + node.Where = &Where{ + Type: WhereClause, + Expr: expr, + } +} diff --git a/go/vt/vterrors/code.go b/go/vt/vterrors/code.go index eb2b1c4be6d..574ac7c2cdf 100644 --- a/go/vt/vterrors/code.go +++ b/go/vt/vterrors/code.go @@ -57,6 +57,7 @@ var ( VT03029 = errorWithState("VT03029", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueCountOnRow, "column count does not match value count with the row for vindex '%s'", "The number of columns you want to insert do not match the number of columns of your SELECT query.") VT03030 = errorWithState("VT03030", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueCountOnRow, "lookup column count does not match value count with the row (columns, count): (%v, %d)", "The number of columns you want to insert do not match the number of columns of your SELECT query.") VT03031 = errorWithoutState("VT03031", vtrpcpb.Code_INVALID_ARGUMENT, "EXPLAIN is only supported for single keyspace", "EXPLAIN has to be sent down as a single query to the underlying MySQL, and this is not possible if it uses tables from multiple keyspaces") + VT03032 = errorWithState("VT03031", vtrpcpb.Code_INVALID_ARGUMENT, NonUpdateableTable, "the target table %s of the UPDATE is not updatable", "You cannot update a table that is not a real MySQL table.") VT05001 = errorWithState("VT05001", vtrpcpb.Code_NOT_FOUND, DbDropExists, "cannot drop database '%s'; database does not exists", "The given database does not exist; Vitess cannot drop it.") VT05002 = errorWithState("VT05002", vtrpcpb.Code_NOT_FOUND, BadDb, "cannot alter database '%s'; unknown database", "The given database does not exist; Vitess cannot alter it.") @@ -141,6 +142,7 @@ var ( VT03029, VT03030, VT03031, + VT03032, VT05001, VT05002, VT05003, diff --git a/go/vt/vtgate/planbuilder/operators/SQL_builder.go b/go/vt/vtgate/planbuilder/operators/SQL_builder.go index aeb60cc8d01..88f9d985d81 100644 --- a/go/vt/vtgate/planbuilder/operators/SQL_builder.go +++ b/go/vt/vtgate/planbuilder/operators/SQL_builder.go @@ -40,6 +40,9 @@ type ( func (qb *queryBuilder) asSelectStatement() sqlparser.SelectStatement { return qb.stmt.(sqlparser.SelectStatement) } +func (qb *queryBuilder) asOrderAndLimit() sqlparser.OrderAndLimit { + return qb.stmt.(sqlparser.OrderAndLimit) +} func ToSQL(ctx *plancontext.PlanningContext, op Operator) (_ sqlparser.Statement, _ Operator, err error) { defer PanicHandler(&err) @@ -70,17 +73,15 @@ func (qb *queryBuilder) addTableExpr( if qb.stmt == nil { qb.stmt = &sqlparser.Select{} } - sel := qb.stmt.(*sqlparser.Select) - elems := &sqlparser.AliasedTableExpr{ + tbl := &sqlparser.AliasedTableExpr{ Expr: tblExpr, Partitions: nil, As: sqlparser.NewIdentifierCS(alias), Hints: hints, Columns: columnAliases, } - qb.ctx.SemTable.ReplaceTableSetFor(tableID, elems) - sel.From = append(sel.From, elems) - qb.stmt = sel + qb.ctx.SemTable.ReplaceTableSetFor(tableID, tbl) + qb.stmt.(FromStatement).SetFrom(append(qb.stmt.(FromStatement).GetFrom(), tbl)) qb.tableNames = append(qb.tableNames, tableName) } @@ -201,62 +202,81 @@ func (qb *queryBuilder) unionWith(other *queryBuilder, distinct bool) { } } +type FromStatement interface { + GetFrom() []sqlparser.TableExpr + SetFrom([]sqlparser.TableExpr) + GetWherePredicate() sqlparser.Expr + SetWherePredicate(sqlparser.Expr) +} + +var _ FromStatement = (*sqlparser.Select)(nil) +var _ FromStatement = (*sqlparser.Update)(nil) +var _ FromStatement = (*sqlparser.Delete)(nil) + func (qb *queryBuilder) joinInnerWith(other *queryBuilder, onCondition sqlparser.Expr) { - sel := qb.stmt.(*sqlparser.Select) - otherSel := other.stmt.(*sqlparser.Select) - sel.From = append(sel.From, otherSel.From...) - sel.SelectExprs = append(sel.SelectExprs, otherSel.SelectExprs...) + stmt := qb.stmt.(FromStatement) + otherStmt := other.stmt.(FromStatement) + + if sel, isSel := stmt.(*sqlparser.Select); isSel { + otherSel := otherStmt.(*sqlparser.Select) + sel.SelectExprs = append(sel.SelectExprs, otherSel.SelectExprs...) + } + + newFromClause := append(stmt.GetFrom(), otherStmt.GetFrom()...) + stmt.SetFrom(newFromClause) + qb.mergeWhereClauses(stmt, otherStmt) + qb.addPredicate(onCondition) +} + +func (qb *queryBuilder) joinOuterWith(other *queryBuilder, onCondition sqlparser.Expr) { + stmt := qb.stmt.(FromStatement) + otherStmt := other.stmt.(FromStatement) - var predicate sqlparser.Expr - if sel.Where != nil { - predicate = sel.Where.Expr + if sel, isSel := stmt.(*sqlparser.Select); isSel { + otherSel := otherStmt.(*sqlparser.Select) + sel.SelectExprs = append(sel.SelectExprs, otherSel.SelectExprs...) } - if otherSel.Where != nil { + + newFromClause := []sqlparser.TableExpr{buildOuterJoin(stmt, otherStmt, onCondition)} + stmt.SetFrom(newFromClause) + qb.mergeWhereClauses(stmt, otherStmt) +} + +func (qb *queryBuilder) mergeWhereClauses(stmt, otherStmt FromStatement) { + predicate := stmt.GetWherePredicate() + if otherPredicate := otherStmt.GetWherePredicate(); otherPredicate != nil { predExprs := sqlparser.SplitAndExpression(nil, predicate) - otherExprs := sqlparser.SplitAndExpression(nil, otherSel.Where.Expr) + otherExprs := sqlparser.SplitAndExpression(nil, otherPredicate) predicate = qb.ctx.SemTable.AndExpressions(append(predExprs, otherExprs...)...) } if predicate != nil { - sel.Where = &sqlparser.Where{Type: sqlparser.WhereClause, Expr: predicate} + stmt.SetWherePredicate(predicate) } - - qb.addPredicate(onCondition) } -func (qb *queryBuilder) joinOuterWith(other *queryBuilder, onCondition sqlparser.Expr) { - sel := qb.stmt.(*sqlparser.Select) - otherSel := other.stmt.(*sqlparser.Select) +func buildOuterJoin(stmt FromStatement, otherStmt FromStatement, onCondition sqlparser.Expr) *sqlparser.JoinTableExpr { var lhs sqlparser.TableExpr - if len(sel.From) == 1 { - lhs = sel.From[0] + fromClause := stmt.GetFrom() + if len(fromClause) == 1 { + lhs = fromClause[0] } else { - lhs = &sqlparser.ParenTableExpr{Exprs: sel.From} + lhs = &sqlparser.ParenTableExpr{Exprs: fromClause} } var rhs sqlparser.TableExpr - if len(otherSel.From) == 1 { - rhs = otherSel.From[0] + otherFromClause := otherStmt.GetFrom() + if len(otherFromClause) == 1 { + rhs = otherFromClause[0] } else { - rhs = &sqlparser.ParenTableExpr{Exprs: otherSel.From} + rhs = &sqlparser.ParenTableExpr{Exprs: otherFromClause} } - sel.From = []sqlparser.TableExpr{&sqlparser.JoinTableExpr{ + + return &sqlparser.JoinTableExpr{ LeftExpr: lhs, RightExpr: rhs, Join: sqlparser.LeftJoinType, Condition: &sqlparser.JoinCondition{ On: onCondition, }, - }} - - sel.SelectExprs = append(sel.SelectExprs, otherSel.SelectExprs...) - var predicate sqlparser.Expr - if sel.Where != nil { - predicate = sel.Where.Expr - } - if otherSel.Where != nil { - predicate = qb.ctx.SemTable.AndExpressions(predicate, otherSel.Where.Expr) - } - if predicate != nil { - sel.Where = &sqlparser.Where{Type: sqlparser.WhereClause, Expr: predicate} } } @@ -386,28 +406,27 @@ func buildQuery(op Operator, qb *queryBuilder) { } func buildDelete(op *Delete, qb *queryBuilder) { - buildQuery(op.Source, qb) - // currently the qb builds a select query underneath. - // Will take the `From` and `Where` from this select - // and create a delete statement. - // TODO: change it to directly produce `delete` statement. - sel, ok := qb.stmt.(*sqlparser.Select) - if !ok { - panic(vterrors.VT13001("expected a select here")) + qb.stmt = &sqlparser.Delete{ + Ignore: op.Ignore, + Targets: sqlparser.TableNames{op.Target.Name}, } + buildQuery(op.Source, qb) qb.dmlOperator = op - qb.stmt = &sqlparser.Delete{ - Ignore: op.Ignore, - Targets: sqlparser.TableNames{op.Target.Name}, - TableExprs: sel.From, - Where: sel.Where, - Limit: sel.Limit, - OrderBy: sel.OrderBy, - } } func buildUpdate(op *Update, qb *queryBuilder) { + updExprs := getUpdateExprs(op) + upd := &sqlparser.Update{ + Ignore: op.Ignore, + Exprs: updExprs, + } + qb.stmt = upd + qb.dmlOperator = op + buildQuery(op.Source, qb) +} + +func getUpdateExprs(op *Update) sqlparser.UpdateExprs { updExprs := make(sqlparser.UpdateExprs, 0, len(op.Assignments)) for _, se := range op.Assignments { updExprs = append(updExprs, &sqlparser.UpdateExpr{ @@ -415,26 +434,7 @@ func buildUpdate(op *Update, qb *queryBuilder) { Expr: se.Expr.EvalExpr, }) } - - buildQuery(op.Source, qb) - // currently the qb builds a select query underneath. - // Will take the `From` and `Where` from this select - // and create a update statement. - // TODO: change it to directly produce `update` statement. - sel, ok := qb.stmt.(*sqlparser.Select) - if !ok { - panic(vterrors.VT13001("expected a select here")) - } - - qb.dmlOperator = op - qb.stmt = &sqlparser.Update{ - Ignore: op.Ignore, - TableExprs: sel.From, - Exprs: updExprs, - Where: sel.Where, - Limit: sel.Limit, - OrderBy: sel.OrderBy, - } + return updExprs } type OpWithAST interface { @@ -470,13 +470,13 @@ func buildOrdering(op *Ordering, qb *queryBuilder) { buildQuery(op.Source, qb) for _, order := range op.Order { - qb.asSelectStatement().AddOrder(order.Inner) + qb.asOrderAndLimit().AddOrder(order.Inner) } } func buildLimit(op *Limit, qb *queryBuilder) { buildQuery(op.Source, qb) - qb.asSelectStatement().SetLimit(op.AST) + qb.asOrderAndLimit().SetLimit(op.AST) } func buildTable(op *Table, qb *queryBuilder) { diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index 22b8f1e4281..4bc2f48f4d8 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -220,12 +220,12 @@ func generateOwnedVindexQuery(tblExpr sqlparser.TableExpr, del *sqlparser.Delete var selExprs sqlparser.SelectExprs for _, col := range ksidCols { colName := makeColName(col, table, sqlparser.MultiTable(del.TableExprs)) - selExprs = append(selExprs, sqlparser.NewAliasedExpr(colName, "")) + selExprs = append(selExprs, aeWrap(colName)) } for _, cv := range table.VTable.Owned { for _, col := range cv.Columns { colName := makeColName(col, table, sqlparser.MultiTable(del.TableExprs)) - selExprs = append(selExprs, sqlparser.NewAliasedExpr(colName, "")) + selExprs = append(selExprs, aeWrap(colName)) } } sqlparser.RemoveKeyspaceInTables(tblExpr) diff --git a/go/vt/vtgate/planbuilder/operators/dml_planning.go b/go/vt/vtgate/planbuilder/operators/dml_planning.go index 6dbe7a74ff3..ed777428381 100644 --- a/go/vt/vtgate/planbuilder/operators/dml_planning.go +++ b/go/vt/vtgate/planbuilder/operators/dml_planning.go @@ -109,6 +109,7 @@ func buildChangedVindexesValues( ctx *plancontext.PlanningContext, update *sqlparser.Update, table *vindexes.Table, + ate *sqlparser.AliasedTableExpr, ksidCols []sqlparser.IdentifierCI, assignments []SetExpr, ) (vv map[string]*engine.VindexValues, ownedVindexQuery *sqlparser.Select, subQueriesArgOnChangedVindex []string) { @@ -144,11 +145,7 @@ func buildChangedVindexesValues( return nil, nil, nil } // generate rest of the owned vindex query. - aTblExpr, ok := update.TableExprs[0].(*sqlparser.AliasedTableExpr) - if !ok { - panic(vterrors.VT12001("UPDATE on complex table expression")) - } - tblExpr := &sqlparser.AliasedTableExpr{Expr: sqlparser.TableName{Name: table.Name}, As: aTblExpr.As} + tblExpr := sqlparser.NewAliasedTableExpr(sqlparser.TableName{Name: table.Name}, ate.As.String()) ovq := &sqlparser.Select{ From: []sqlparser.TableExpr{tblExpr}, SelectExprs: selExprs, diff --git a/go/vt/vtgate/planbuilder/operators/phases.go b/go/vt/vtgate/planbuilder/operators/phases.go index b45fbd5c9ad..3937105c494 100644 --- a/go/vt/vtgate/planbuilder/operators/phases.go +++ b/go/vt/vtgate/planbuilder/operators/phases.go @@ -79,7 +79,7 @@ func (p Phase) shouldRun(s semantics.QuerySignature) bool { case subquerySettling: return s.SubQueries case dmlWithInput: - return s.Dml + return s.DML default: return true } diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index a4aafa222fa..5fc7dc0d4a7 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -103,6 +103,8 @@ func (u *Update) ShortDescription() string { } func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update) (op Operator) { + errIfUpdateNotSupported(ctx, updStmt) + var updClone *sqlparser.Update var vTbl *vindexes.Table @@ -134,6 +136,31 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return buildFkOperator(ctx, op, updClone, parentFks, childFks, vTbl) } +func errIfUpdateNotSupported(ctx *plancontext.PlanningContext, stmt *sqlparser.Update) { + var vTbl *vindexes.Table + for _, ue := range stmt.Exprs { + tblInfo, err := ctx.SemTable.TableInfoForExpr(ue.Name) + if err != nil { + panic(err) + } + if _, isATable := tblInfo.(*semantics.RealTable); !isATable { + var tblName string + ate := tblInfo.GetAliasedTableExpr() + if ate != nil { + tblName = sqlparser.String(ate) + } + panic(vterrors.VT03032(tblName)) + } + + if vTbl == nil { + vTbl = tblInfo.GetVindexTable() + } + if vTbl != tblInfo.GetVindexTable() { + panic(vterrors.VT12001("multi-table UPDATE statement with multi-target column update")) + } + } +} + func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update) (Operator, *vindexes.Table, *sqlparser.Update) { op := crossJoin(ctx, updStmt.TableExprs) @@ -148,6 +175,8 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U // If we encounter subqueries, we want to fix the updClone to use the replaced expression, so that the pulled out subquery's // result is used everywhere instead of running the subquery multiple times, which is wasteful. updClone := sqlparser.CloneRefOfUpdate(updStmt) + var tblInfo semantics.TableInfo + var err error for idx, updExpr := range updStmt.Exprs { expr, subqs := sqc.pullOutValueSubqueries(ctx, updExpr.Expr, outerID, true) if len(subqs) == 0 { @@ -164,19 +193,13 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U Name: updExpr.Name, Expr: proj, } + tblInfo, err = ctx.SemTable.TableInfoForExpr(updExpr.Name) + if err != nil { + panic(err) + } } - target := updStmt.TableExprs[0] - atbl, ok := target.(*sqlparser.AliasedTableExpr) - if !ok { - panic(vterrors.VT12001("multi table update")) - } - tblID := ctx.SemTable.TableSetFor(atbl) - tblInfo, err := ctx.SemTable.TableInfoFor(tblID) - if err != nil { - panic(err) - } - + tblID := ctx.SemTable.TableSetFor(tblInfo.GetAliasedTableExpr()) vTbl := tblInfo.GetVindexTable() // Reference table should update the source table. if vTbl.Type == vindexes.TypeReference && vTbl.Source != nil { @@ -194,7 +217,7 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U Name: name, } - _, cvv, ovq, subQueriesArgOnChangedVindex := getUpdateVindexInformation(ctx, updStmt, targetTbl, assignments) + _, cvv, ovq, subQueriesArgOnChangedVindex := getUpdateVindexInformation(ctx, updStmt, targetTbl, tblInfo.GetAliasedTableExpr(), assignments) updOp := &Update{ DMLCommon: &DMLCommon{ @@ -226,6 +249,7 @@ func getUpdateVindexInformation( ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, table TargetTable, + ate *sqlparser.AliasedTableExpr, assignments []SetExpr, ) ([]*VindexPlusPredicates, map[string]*engine.VindexValues, *sqlparser.Select, []string) { if !table.VTable.Keyspace.Sharded { @@ -233,7 +257,7 @@ func getUpdateVindexInformation( } primaryVindex, vindexAndPredicates := getVindexInformation(table.ID, table.VTable) - changedVindexValues, ownedVindexQuery, subQueriesArgOnChangedVindex := buildChangedVindexesValues(ctx, updStmt, table.VTable, primaryVindex.Columns, assignments) + changedVindexValues, ownedVindexQuery, subQueriesArgOnChangedVindex := buildChangedVindexesValues(ctx, updStmt, table.VTable, ate, primaryVindex.Columns, assignments) return vindexAndPredicates, changedVindexValues, ownedVindexQuery, subQueriesArgOnChangedVindex } diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.json b/go/vt/vtgate/planbuilder/testdata/dml_cases.json index 5e4987be8c3..995795fc209 100644 --- a/go/vt/vtgate/planbuilder/testdata/dml_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/dml_cases.json @@ -5660,5 +5660,173 @@ "user.user" ] } + }, + { + "comment": "update with multi table join with single target", + "query": "update user as u, user_extra as ue set u.name = 'foo' where u.id = ue.id", + "plan": { + "QueryType": "UPDATE", + "Original": "update user as u, user_extra as ue set u.name = 'foo' where u.id = ue.id", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "R:0", + "JoinVars": { + "ue_id": 0 + }, + "TableName": "user_extra_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select ue.id from user_extra as ue where 1 != 1", + "Query": "select ue.id from user_extra as ue lock in share mode", + "Table": "user_extra" + }, + { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u.id from `user` as u where 1 != 1", + "Query": "select u.id from `user` as u where u.id = :ue_id lock in share mode", + "Table": "`user`", + "Values": [ + ":ue_id" + ], + "Vindex": "user_index" + } + ] + }, + { + "OperatorType": "Update", + "Variant": "IN", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "ChangedVindexValues": [ + "name_user_map:3" + ], + "KsidLength": 1, + "KsidVindex": "user_index", + "OwnedVindexQuery": "select Id, `Name`, Costly, u.`name` = 'foo' from `user` as u where u.id in ::dml_vals for update", + "Query": "update `user` as u set u.`name` = 'foo' where u.id in ::dml_vals", + "Table": "user", + "Values": [ + "::dml_vals" + ], + "Vindex": "user_index" + } + ] + }, + "TablesUsed": [ + "user.user", + "user.user_extra" + ] + } + }, + { + "comment": "update with multi table join with single target modifying lookup vindex", + "query": "update user join user_extra on user.id = user_extra.id set user.name = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update user join user_extra on user.id = user_extra.id set user.name = 'foo'", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "R:0", + "JoinVars": { + "user_extra_id": 0 + }, + "TableName": "user_extra_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select user_extra.id from user_extra where 1 != 1", + "Query": "select user_extra.id from user_extra lock in share mode", + "Table": "user_extra" + }, + { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.id from `user` where 1 != 1", + "Query": "select `user`.id from `user` where `user`.id = :user_extra_id lock in share mode", + "Table": "`user`", + "Values": [ + ":user_extra_id" + ], + "Vindex": "user_index" + } + ] + }, + { + "OperatorType": "Update", + "Variant": "IN", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "ChangedVindexValues": [ + "name_user_map:3" + ], + "KsidLength": 1, + "KsidVindex": "user_index", + "OwnedVindexQuery": "select Id, `Name`, Costly, `user`.`name` = 'foo' from `user` where `user`.id in ::dml_vals for update", + "Query": "update `user` set `user`.`name` = 'foo' where `user`.id in ::dml_vals", + "Table": "user", + "Values": [ + "::dml_vals" + ], + "Vindex": "user_index" + } + ] + }, + "TablesUsed": [ + "user.user", + "user.user_extra" + ] + } + }, + { + "comment": "update with multi table reference with multi target update on a derived table", + "query": "update ignore (select foo, col, bar from user) u, music m set u.foo = 21, u.bar = 'abc' where u.col = m.col", + "plan": "VT03031: the target table (select foo, col, bar from `user`) as u of the UPDATE is not updatable" + }, + { + "comment": "update with derived table", + "query": "update (select id from user) as u set id = 4", + "plan": "VT03031: the target table (select id from `user`) as u of the UPDATE is not updatable" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json index 59071c7e810..4214a396499 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json @@ -64,21 +64,6 @@ "query": "update user_metadata set email = 'juan@vitess.io' where user_id = 1 limit 10", "plan": "VT12001: unsupported: Vindex update should have ORDER BY clause when using LIMIT" }, - { - "comment": "update with derived table", - "query": "update (select id from user) as u set id = 4", - "plan": "The target table u of the UPDATE is not updatable" - }, - { - "comment": "join in update tables", - "query": "update user join user_extra on user.id = user_extra.id set user.name = 'foo'", - "plan": "VT12001: unsupported: unaliased multiple tables in update" - }, - { - "comment": "multiple tables in update", - "query": "update user as u, user_extra as ue set u.name = 'foo' where u.id = ue.id", - "plan": "VT12001: unsupported: multiple (2) tables in update" - }, { "comment": "unsharded insert, col list does not match values", "query": "insert into unsharded_auto(id, val) values(1)", @@ -408,5 +393,10 @@ "comment": "count and sum distinct on different columns", "query": "SELECT COUNT(DISTINCT col), SUM(DISTINCT id) FROM user", "plan": "VT12001: unsupported: only one DISTINCT aggregation is allowed in a SELECT: sum(distinct id)" + }, + { + "comment": "update with multi table reference with multi target update", + "query": "update ignore user u, music m set u.foo = 21, m.bar = 'abc' where u.col = m.col", + "plan": "VT12001: unsupported: multi-table UPDATE statement with multi-target column update" } ] diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index 95d645d7d9d..f52bd983104 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -328,8 +328,8 @@ func (a *analyzer) noteQuerySignature(node sqlparser.SQLNode) { } case sqlparser.AggrFunc: a.sig.Aggregation = true - case *sqlparser.Delete, *sqlparser.Update: - a.sig.Dml = true + case *sqlparser.Delete, *sqlparser.Update, *sqlparser.Insert: + a.sig.DML = true } } diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 10f85326751..dd8db6f5cd1 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -1589,31 +1589,6 @@ func TestNextErrors(t *testing.T) { } } -func TestUpdateErrors(t *testing.T) { - tests := []struct { - query, expectedError string - }{ - { - query: "update t1, t2 set id = 12", - expectedError: "VT12001: unsupported: multiple (2) tables in update", - }, { - query: "update (select 1 from dual) dt set id = 1", - expectedError: "The target table dt of the UPDATE is not updatable", - }, - } - - for _, test := range tests { - t.Run(test.query, func(t *testing.T) { - parse, err := sqlparser.NewTestParser().Parse(test.query) - require.NoError(t, err) - - _, err = AnalyzeStrict(parse, "d", fakeSchemaInfo()) - - assert.EqualError(t, err, test.expectedError) - }) - } -} - // TestScopingSubQueryJoinClause tests the scoping behavior of a subquery containing a join clause. // The test ensures that the scoping analysis correctly identifies and handles the relationships // between the tables involved in the join operation with the outer query. diff --git a/go/vt/vtgate/semantics/check_invalid.go b/go/vt/vtgate/semantics/check_invalid.go index 38dacefa1f0..45c160e93a2 100644 --- a/go/vt/vtgate/semantics/check_invalid.go +++ b/go/vt/vtgate/semantics/check_invalid.go @@ -24,8 +24,6 @@ import ( func (a *analyzer) checkForInvalidConstructs(cursor *sqlparser.Cursor) error { switch node := cursor.Node().(type) { - case *sqlparser.Update: - return checkUpdate(node) case *sqlparser.Select: return a.checkSelect(cursor, node) case *sqlparser.Nextval: @@ -179,21 +177,6 @@ func (a *analyzer) checkSelect(cursor *sqlparser.Cursor, node *sqlparser.Select) return nil } -func checkUpdate(node *sqlparser.Update) error { - if len(node.TableExprs) != 1 { - return ShardedError{Inner: &UnsupportedMultiTablesInUpdateError{ExprCount: len(node.TableExprs)}} - } - alias, isAlias := node.TableExprs[0].(*sqlparser.AliasedTableExpr) - if !isAlias { - return ShardedError{Inner: &UnsupportedMultiTablesInUpdateError{NotAlias: true}} - } - _, isDerived := alias.Expr.(*sqlparser.DerivedTable) - if isDerived { - return &TableNotUpdatableError{Table: alias.As.String()} - } - return nil -} - // checkAliasedTableExpr checks the validity of AliasedTableExpr. func checkAliasedTableExpr(node *sqlparser.AliasedTableExpr) error { if len(node.Hints) == 0 { diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 949a1b72017..74bd9dd1d69 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -78,7 +78,7 @@ type ( // QuerySignature is used to identify shortcuts in the planning process QuerySignature struct { Aggregation bool - Dml bool + DML bool Distinct bool HashJoin bool SubQueries bool