From 23bfeb3e04c62ffd076539d122f7436adf3d6ac0 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Fri, 18 Aug 2023 23:34:40 +0530 Subject: [PATCH 01/31] feat: foreign key cascade planner for delete query Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/delete.go | 15 +-- go/vt/vtgate/planbuilder/insert.go | 20 +--- go/vt/vtgate/planbuilder/operators/ast2op.go | 111 ++++++++++++++++-- .../planbuilder/operators/fk_cascade.go | 69 +++++++++++ .../vtgate/planbuilder/operators/fk_child.go | 58 +++++++++ go/vt/vtgate/planbuilder/plan_test.go | 4 +- .../plancontext/planning_context.go | 20 +++- go/vt/vtgate/planbuilder/select.go | 85 ++++++-------- go/vt/vtgate/planbuilder/update.go | 20 +--- 9 files changed, 297 insertions(+), 105 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/operators/fk_cascade.go create mode 100644 go/vt/vtgate/planbuilder/operators/fk_child.go diff --git a/go/vt/vtgate/planbuilder/delete.go b/go/vt/vtgate/planbuilder/delete.go index 4fae6ae97fa..e12b6c9abac 100644 --- a/go/vt/vtgate/planbuilder/delete.go +++ b/go/vt/vtgate/planbuilder/delete.go @@ -46,23 +46,17 @@ func gen4DeleteStmtPlanner( } } - ksName := "" - if ks, _ := vschema.DefaultKeyspace(); ks != nil { - ksName = ks.Name - } - semTable, err := semantics.Analyze(deleteStmt, ksName, vschema) + ctx, err := plancontext.CreatePlanningContext(deleteStmt, reservedVars, vschema, version) if err != nil { return nil, err } - // record any warning as planner warning. - vschema.PlannerWarning(semTable.Warning) err = rewriteRoutedTables(deleteStmt, vschema) if err != nil { return nil, err } - if ks, tables := semTable.SingleUnshardedKeyspace(); ks != nil { + if ks, tables := ctx.SemTable.SingleUnshardedKeyspace(); ks != nil { if fkManagementNotRequired(vschema, tables) { plan := deleteUnshardedShortcut(deleteStmt, ks, tables) plan = pushCommentDirectivesOnPlan(plan, deleteStmt) @@ -70,16 +64,15 @@ func gen4DeleteStmtPlanner( } } - if err := checkIfDeleteSupported(deleteStmt, semTable); err != nil { + if err := checkIfDeleteSupported(deleteStmt, ctx.SemTable); err != nil { return nil, err } - err = queryRewrite(semTable, reservedVars, deleteStmt) + err = queryRewrite(ctx.SemTable, reservedVars, deleteStmt) if err != nil { return nil, err } - ctx := plancontext.NewPlanningContext(reservedVars, semTable, vschema, version) op, err := operators.PlanQuery(ctx, deleteStmt) if err != nil { return nil, err diff --git a/go/vt/vtgate/planbuilder/insert.go b/go/vt/vtgate/planbuilder/insert.go index 864d1056908..7ddd6919f9e 100644 --- a/go/vt/vtgate/planbuilder/insert.go +++ b/go/vt/vtgate/planbuilder/insert.go @@ -27,16 +27,10 @@ import ( ) func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStmt *sqlparser.Insert, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema) (*planResult, error) { - ksName := "" - if ks, _ := vschema.DefaultKeyspace(); ks != nil { - ksName = ks.Name - } - semTable, err := semantics.Analyze(insStmt, ksName, vschema) + ctx, err := plancontext.CreatePlanningContext(insStmt, reservedVars, vschema, version) if err != nil { return nil, err } - // record any warning as planner warning. - vschema.PlannerWarning(semTable.Warning) err = rewriteRoutedTables(insStmt, vschema) if err != nil { @@ -48,28 +42,26 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm // Check single unsharded. Even if the table is for single unsharded but sequence table is used. // We cannot shortcut here as sequence column needs additional planning. - ks, tables := semTable.SingleUnshardedKeyspace() + ks, tables := ctx.SemTable.SingleUnshardedKeyspace() if ks != nil && tables[0].AutoIncrement == nil { plan := insertUnshardedShortcut(insStmt, ks, tables) plan = pushCommentDirectivesOnPlan(plan, insStmt) return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil } - tblInfo, err := semTable.TableInfoFor(semTable.TableSetFor(insStmt.Table)) + tblInfo, err := ctx.SemTable.TableInfoFor(ctx.SemTable.TableSetFor(insStmt.Table)) if err != nil { return nil, err } - if tblInfo.GetVindexTable().Keyspace.Sharded && semTable.NotUnshardedErr != nil { - return nil, semTable.NotUnshardedErr + if tblInfo.GetVindexTable().Keyspace.Sharded && ctx.SemTable.NotUnshardedErr != nil { + return nil, ctx.SemTable.NotUnshardedErr } - err = queryRewrite(semTable, reservedVars, insStmt) + err = queryRewrite(ctx.SemTable, reservedVars, insStmt) if err != nil { return nil, err } - ctx := plancontext.NewPlanningContext(reservedVars, semTable, vschema, version) - op, err := operators.PlanQuery(ctx, insStmt) if err != nil { return nil, err diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index a4463035d67..f2cb0c035f0 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -215,6 +215,87 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp return nil, err } + delClone := sqlparser.CloneRefOfDelete(deleteStmt) + // Create the delete operator first. + del, err := createDeleteOperator(ctx, deleteStmt, qt, vindexTable, routing) + if err != nil { + return nil, err + } + + // Now we check for the foreign key mode and make changes if required. + ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) + if err != nil { + return nil, err + } + + // Unmanaged foreign-key-mode, we don't need to do anything. + if ksMode != vschemapb.Keyspace_FK_MANAGED { + return del, nil + } + childFks := vindexTable.ChildFKsNeedsHandling(vindexes.DeleteAction) + // If there are no foreign key constraints, then we don't need to do anything. + if len(childFks) == 0 { + return del, nil + } + // If the delete statement has a limit, we don't support it yet. + if deleteStmt.Limit != nil { + return nil, vterrors.VT12001("foreign keys management at vitess with limit") + } + + var fkChildren []ops.Operator + var selectExprs []sqlparser.SelectExpr + for _, fk := range childFks { + if isRestrict(fk.OnDelete) { + return nil, vterrors.VT12002() + } + var cols []int + for _, column := range fk.ParentColumns { + cols = append(cols, len(selectExprs)) + selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) + } + + bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + compExpr := &sqlparser.ComparisonExpr{ + Operator: sqlparser.InOp, + Left: valTuple, + Right: sqlparser.NewListArg(bvName), + } + childDel := &sqlparser.Delete{ + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(sqlparser.NewTableName(fk.Table.String()), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + } + childDelOp, err := createOpFromStmt(ctx, childDel) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, &FkChild{ + BVName: bvName, + Cols: cols, + Op: childDelOp, + }) + } + selectionStmt := &sqlparser.Select{ + SelectExprs: selectExprs, + From: delClone.TableExprs, + Where: delClone.Where, + } + selection, err := createOpFromStmt(ctx, selectionStmt) + if err != nil { + return nil, err + } + + return &FkCascade{ + Selection: selection, + Children: fkChildren, + Parent: del, + }, nil +} + +func createDeleteOperator(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete, qt *QueryTable, vindexTable *vindexes.Table, routing Routing) (ops.Operator, error) { del := &Delete{ QTable: qt, VTable: vindexTable, @@ -225,17 +306,6 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp Routing: routing, } - ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) - if err != nil { - return nil, err - } - if ksMode == vschemapb.Keyspace_FK_MANAGED { - childFks := vindexTable.ChildFKsNeedsHandling(vindexes.DeleteAction) - if len(childFks) > 0 { - return nil, vterrors.VT12003() - } - } - if !vindexTable.Keyspace.Sharded { return route, nil } @@ -282,6 +352,25 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp return subq, nil } +func isRestrict(onDelete sqlparser.ReferenceAction) bool { + switch onDelete { + case sqlparser.Restrict, sqlparser.NoAction, sqlparser.DefaultAction: + return true + default: + return false + } +} + +func createOpFromStmt(ctx *plancontext.PlanningContext, stmt sqlparser.Statement) (ops.Operator, error) { + newCtx, err := plancontext.CreatePlanningContext(stmt, ctx.ReservedVars, ctx.VSchema, ctx.PlannerVersion) + if err != nil { + return nil, err + } + ctx = newCtx + + return PlanQuery(ctx, stmt) +} + func createOperatorFromInsert(ctx *plancontext.PlanningContext, ins *sqlparser.Insert) (ops.Operator, error) { tableInfo, qt, err := createQueryTableForDML(ctx, ins.Table, nil) if err != nil { diff --git a/go/vt/vtgate/planbuilder/operators/fk_cascade.go b/go/vt/vtgate/planbuilder/operators/fk_cascade.go new file mode 100644 index 00000000000..2d5d8693f01 --- /dev/null +++ b/go/vt/vtgate/planbuilder/operators/fk_cascade.go @@ -0,0 +1,69 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operators + +import ( + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" +) + +type FkCascade struct { + Selection ops.Operator + Children []ops.Operator + Parent ops.Operator + + noColumns + noPredicates +} + +var _ ops.Operator = (*FkCascade)(nil) + +func (fkc *FkCascade) Inputs() []ops.Operator { + var inputs []ops.Operator + inputs = append(inputs, fkc.Parent) + inputs = append(inputs, fkc.Selection) + inputs = append(inputs, fkc.Children...) + return inputs +} + +func (fkc *FkCascade) SetInputs(operators []ops.Operator) { + if len(operators) < 2 { + panic("incorrect count of inputs for FkCascade") + } + fkc.Parent = operators[0] + fkc.Selection = operators[1] + fkc.Children = operators[2:] +} + +// Clone implements the Operator interface +func (fkc *FkCascade) Clone(inputs []ops.Operator) ops.Operator { + if len(inputs) < 2 { + panic("incorrect count of inputs for FkCascade") + } + return &FkCascade{ + Parent: inputs[0], + Selection: inputs[1], + Children: inputs[2:], + } +} + +func (fkc *FkCascade) GetOrdering() ([]ops.OrderBy, error) { + return nil, nil +} + +func (fkc *FkCascade) ShortDescription() string { + return "FkCascade" +} diff --git a/go/vt/vtgate/planbuilder/operators/fk_child.go b/go/vt/vtgate/planbuilder/operators/fk_child.go new file mode 100644 index 00000000000..c0fa6c2581b --- /dev/null +++ b/go/vt/vtgate/planbuilder/operators/fk_child.go @@ -0,0 +1,58 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operators + +import ( + "fmt" + + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" +) + +type FkChild struct { + BVName string + Cols []int // indexes + Op ops.Operator + + noColumns + noPredicates +} + +func (f *FkChild) Clone(inputs []ops.Operator) ops.Operator { + return &FkChild{ + BVName: f.BVName, + Cols: f.Cols, + Op: inputs[0], + } +} + +func (f *FkChild) Inputs() []ops.Operator { + return []ops.Operator{f.Op} +} + +func (f *FkChild) SetInputs(operators []ops.Operator) { + f.Op = operators[0] +} + +func (f *FkChild) ShortDescription() string { + return fmt.Sprintf("BvName: %s, Cols: %v", f.BVName, f.Cols) +} + +func (f *FkChild) GetOrdering() ([]ops.OrderBy, error) { + return nil, nil +} + +var _ ops.Operator = (*FkChild)(nil) diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index c548f1c0ca9..df0e3ec0a94 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -183,8 +183,10 @@ func TestOne(t *testing.T) { reset := oprewriters.EnableDebugPrinting() defer reset() + lv := loadSchema(t, "vschemas/schema.json", true) + setFks(t, lv) vschema := &vschemawrapper.VSchemaWrapper{ - V: loadSchema(t, "vschemas/schema.json", true), + V: lv, } testFile(t, "onecase.json", "", vschema, false) diff --git a/go/vt/vtgate/planbuilder/plancontext/planning_context.go b/go/vt/vtgate/planbuilder/plancontext/planning_context.go index 85e72a8c6d2..6fc99232379 100644 --- a/go/vt/vtgate/planbuilder/plancontext/planning_context.go +++ b/go/vt/vtgate/planbuilder/plancontext/planning_context.go @@ -45,8 +45,21 @@ type PlanningContext struct { DelegateAggregation bool } -func NewPlanningContext(reservedVars *sqlparser.ReservedVars, semTable *semantics.SemTable, vschema VSchema, version querypb.ExecuteOptions_PlannerVersion) *PlanningContext { - ctx := &PlanningContext{ +func CreatePlanningContext(stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema VSchema, version querypb.ExecuteOptions_PlannerVersion) (*PlanningContext, error) { + ksName := "" + if ks, _ := vschema.DefaultKeyspace(); ks != nil { + ksName = ks.Name + } + + semTable, err := semantics.Analyze(stmt, ksName, vschema) + if err != nil { + return nil, err + } + + // record any warning as planner warning. + vschema.PlannerWarning(semTable.Warning) + + return &PlanningContext{ ReservedVars: reservedVars, SemTable: semTable, VSchema: vschema, @@ -54,8 +67,7 @@ func NewPlanningContext(reservedVars *sqlparser.ReservedVars, semTable *semantic SkipPredicates: map[sqlparser.Expr]any{}, PlannerVersion: version, ReservedArguments: map[sqlparser.Expr]string{}, - } - return ctx + }, nil } func (c *PlanningContext) IsSubQueryToReplace(e sqlparser.Expr) bool { diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index df4e34e8308..ce5857a01e2 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -26,6 +26,7 @@ import ( "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators" + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" ) @@ -73,18 +74,18 @@ func gen4SelectStmtPlanner( sel.SQLCalcFoundRows = false } - getPlan := func(selStatement sqlparser.SelectStatement) (logicalPlan, *semantics.SemTable, []string, error) { + getPlan := func(selStatement sqlparser.SelectStatement) (logicalPlan, []string, error) { return newBuildSelectPlan(selStatement, reservedVars, vschema, plannerVersion) } - plan, _, tablesUsed, err := getPlan(stmt) + plan, tablesUsed, err := getPlan(stmt) if err != nil { return nil, err } if shouldRetryAfterPredicateRewriting(plan) { // by transforming the predicates to CNF, the planner will sometimes find better plans - plan2, _, tablesUsed := gen4PredicateRewrite(stmt, getPlan) + plan2, tablesUsed := gen4PredicateRewrite(stmt, getPlan) if plan2 != nil { return newPlanResult(plan2.Primitive(), tablesUsed...), nil } @@ -135,7 +136,7 @@ func buildSQLCalcFoundRowsPlan( reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, ) (logicalPlan, []string, error) { - limitPlan, _, err := planSelectGen4(reservedVars, vschema, sel) + limitPlan, _, err := newBuildSelectPlan(sel, reservedVars, vschema, Gen4) if err != nil { return nil, nil, err } @@ -175,33 +176,25 @@ func buildSQLCalcFoundRowsPlan( reservedVars2 := sqlparser.NewReservedVars("vtg", reserved2) - countPlan, tablesUsed, err := planSelectGen4(reservedVars2, vschema, sel2) + countPlan, tablesUsed, err := newBuildSelectPlan(sel2, reservedVars2, vschema, Gen4) if err != nil { return nil, nil, err } return &sqlCalcFoundRows{LimitQuery: limitPlan, CountQuery: countPlan}, tablesUsed, nil } -func planSelectGen4(reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, sel *sqlparser.Select) (logicalPlan, []string, error) { - plan, _, tablesUsed, err := newBuildSelectPlan(sel, reservedVars, vschema, 0) - if err != nil { - return nil, nil, err - } - return plan, tablesUsed, nil -} - -func gen4PredicateRewrite(stmt sqlparser.Statement, getPlan func(selStatement sqlparser.SelectStatement) (logicalPlan, *semantics.SemTable, []string, error)) (logicalPlan, *semantics.SemTable, []string) { +func gen4PredicateRewrite(stmt sqlparser.Statement, getPlan func(selStatement sqlparser.SelectStatement) (logicalPlan, []string, error)) (logicalPlan, []string) { rewritten, isSel := sqlparser.RewritePredicate(stmt).(sqlparser.SelectStatement) if !isSel { // Fail-safe code, should never happen - return nil, nil, nil + return nil, nil } - plan2, st, op, err := getPlan(rewritten) + plan2, op, err := getPlan(rewritten) if err == nil && !shouldRetryAfterPredicateRewriting(plan2) { // we only use this new plan if it's better than the old one we got - return plan2, st, op + return plan2, op } - return nil, nil, nil + return nil, nil } func newBuildSelectPlan( @@ -209,65 +202,57 @@ func newBuildSelectPlan( reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, version querypb.ExecuteOptions_PlannerVersion, -) (plan logicalPlan, semTable *semantics.SemTable, tablesUsed []string, err error) { - ksName := "" - if ks, _ := vschema.DefaultKeyspace(); ks != nil { - ksName = ks.Name - } - semTable, err = semantics.Analyze(selStmt, ksName, vschema) +) (logicalPlan, []string, error) { + ctx, err := plancontext.CreatePlanningContext(selStmt, reservedVars, vschema, version) if err != nil { - return nil, nil, nil, err + return nil, nil, err } - // record any warning as planner warning. - vschema.PlannerWarning(semTable.Warning) - - ctx := plancontext.NewPlanningContext(reservedVars, semTable, vschema, version) - if ks, _ := semTable.SingleUnshardedKeyspace(); ks != nil { - plan, tablesUsed, err = selectUnshardedShortcut(ctx, selStmt, ks) + if ks, _ := ctx.SemTable.SingleUnshardedKeyspace(); ks != nil { + plan, tablesUsed, err := selectUnshardedShortcut(ctx, selStmt, ks) if err != nil { - return nil, nil, nil, err + return nil, nil, err } plan = pushCommentDirectivesOnPlan(plan, selStmt) - return plan, semTable, tablesUsed, err + return plan, tablesUsed, err } // From this point on, we know it is not an unsharded query and return the NotUnshardedErr if there is any - if semTable.NotUnshardedErr != nil { - return nil, nil, nil, semTable.NotUnshardedErr + if ctx.SemTable.NotUnshardedErr != nil { + return nil, nil, ctx.SemTable.NotUnshardedErr } - err = queryRewrite(semTable, reservedVars, selStmt) + op, err := createSelectOperator(ctx, selStmt, reservedVars) if err != nil { - return nil, nil, nil, err - } - - op, err := operators.PlanQuery(ctx, selStmt) - if err != nil { - return nil, nil, nil, err + return nil, nil, err } - plan, err = transformToLogicalPlan(ctx, op, true) + plan, err := transformToLogicalPlan(ctx, op, true) if err != nil { - return nil, nil, nil, err + return nil, nil, err } optimizePlan(plan) - sel, isSel := selStmt.(*sqlparser.Select) - if isSel { + if sel, isSel := selStmt.(*sqlparser.Select); isSel { if err = setMiscFunc(plan, sel); err != nil { - return nil, nil, nil, err + return nil, nil, err } } if err = plan.Wireup(ctx); err != nil { - return nil, nil, nil, err + return nil, nil, err } + return pushCommentDirectivesOnPlan(plan, selStmt), operators.TablesUsed(op), nil +} - plan = pushCommentDirectivesOnPlan(plan, selStmt) +func createSelectOperator(ctx *plancontext.PlanningContext, selStmt sqlparser.SelectStatement, reservedVars *sqlparser.ReservedVars) (ops.Operator, error) { + err := queryRewrite(ctx.SemTable, reservedVars, selStmt) + if err != nil { + return nil, err + } - return plan, semTable, operators.TablesUsed(op), nil + return operators.PlanQuery(ctx, selStmt) } // optimizePlan removes unnecessary simpleProjections that have been created while planning diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index 052a204cc1b..c0f95aa9e73 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -38,41 +38,33 @@ func gen4UpdateStmtPlanner( return nil, vterrors.VT12001("WITH expression in UPDATE statement") } - ksName := "" - if ks, _ := vschema.DefaultKeyspace(); ks != nil { - ksName = ks.Name - } - semTable, err := semantics.Analyze(updStmt, ksName, vschema) + ctx, err := plancontext.CreatePlanningContext(updStmt, reservedVars, vschema, version) if err != nil { return nil, err } - // record any warning as planner warning. - vschema.PlannerWarning(semTable.Warning) err = rewriteRoutedTables(updStmt, vschema) if err != nil { return nil, err } - if ks, tables := semTable.SingleUnshardedKeyspace(); ks != nil { - if fkManagementNotRequiredForUpdate(semTable, vschema, tables, updStmt.Exprs) { + if ks, tables := ctx.SemTable.SingleUnshardedKeyspace(); ks != nil { + if fkManagementNotRequiredForUpdate(ctx.SemTable, vschema, tables, updStmt.Exprs) { plan := updateUnshardedShortcut(updStmt, ks, tables) plan = pushCommentDirectivesOnPlan(plan, updStmt) return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil } } - if semTable.NotUnshardedErr != nil { - return nil, semTable.NotUnshardedErr + if ctx.SemTable.NotUnshardedErr != nil { + return nil, ctx.SemTable.NotUnshardedErr } - err = queryRewrite(semTable, reservedVars, updStmt) + err = queryRewrite(ctx.SemTable, reservedVars, updStmt) if err != nil { return nil, err } - ctx := plancontext.NewPlanningContext(reservedVars, semTable, vschema, version) - op, err := operators.PlanQuery(ctx, updStmt) if err != nil { return nil, err From eefc89c4249297fd1ad38e7119e747b26259b6ce Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 11:12:54 +0530 Subject: [PATCH 02/31] feat: fix operator planning for fk_cascade and add comments Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 11 ++++++++++- go/vt/vtgate/planbuilder/testdata/onecase.json | 8 +++----- go/vt/vtgate/vindexes/vschema.go | 5 +++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index f2cb0c035f0..6b3e5e9e255 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -245,15 +245,21 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp var fkChildren []ops.Operator var selectExprs []sqlparser.SelectExpr for _, fk := range childFks { + // Any RESTRICT type foreign keys that arrive here, + // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. if isRestrict(fk.OnDelete) { return nil, vterrors.VT12002() } + + // We need to select all the parent columns for the foreign key constraint, to use in the deletion in the child. var cols []int for _, column := range fk.ParentColumns { cols = append(cols, len(selectExprs)) selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) } + // We now construct the delete query for the child table. + // The query looks something like this - `DELETE FROM WHERE IN ()` bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") var valTuple sqlparser.ValTuple for _, column := range fk.ChildColumns { @@ -265,7 +271,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp Right: sqlparser.NewListArg(bvName), } childDel := &sqlparser.Delete{ - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(sqlparser.NewTableName(fk.Table.String()), "")}, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, } childDelOp, err := createOpFromStmt(ctx, childDel) @@ -278,6 +284,9 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp Op: childDelOp, }) } + // We now create the selection operator to select the parent columns for the foreign key constraints. + // The Select statement looks something like this - `SELECT FROM WHERE ` + // TODO (@Harshit, @GuptaManan100): Compress the columns in the SELECT statement, if there are multiple foreign key constraints using the same columns. selectionStmt := &sqlparser.Select{ SelectExprs: selectExprs, From: delClone.TableExprs, diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.json b/go/vt/vtgate/planbuilder/testdata/onecase.json index da7543f706a..f303e58ef7e 100644 --- a/go/vt/vtgate/planbuilder/testdata/onecase.json +++ b/go/vt/vtgate/planbuilder/testdata/onecase.json @@ -1,9 +1,7 @@ [ { - "comment": "Add your test case here for debugging and run go test -run=One.", - "query": "", - "plan": { - - } + "comment": "Delete in a table with shard-scoped foreign keys with cascade disallowed", + "query": "delete from tbl5", + "plan": "VT12002: unsupported: foreign keys management at vitess" } ] \ No newline at end of file diff --git a/go/vt/vtgate/vindexes/vschema.go b/go/vt/vtgate/vindexes/vschema.go index 6edd97aeeb5..53c6dba6304 100644 --- a/go/vt/vtgate/vindexes/vschema.go +++ b/go/vt/vtgate/vindexes/vschema.go @@ -120,6 +120,11 @@ type Table struct { ParentForeignKeys []ParentFKInfo `json:"parent_foreign_keys,omitempty"` } +// GetTableName gets the sqlparser.TableName for the vindex Table. +func (t *Table) GetTableName() sqlparser.TableName { + return sqlparser.NewTableNameWithQualifier(t.Name.String(), t.Keyspace.Name) +} + // Keyspace contains the keyspcae info for each Table. type Keyspace struct { Name string From 4a827098761814804fbe08c8363aac2f14afba47 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 12:56:18 +0530 Subject: [PATCH 03/31] feat: add conversion to logical primitives for fk cascading in deletes and update tests Signed-off-by: Manan Gupta --- go/vt/vtgate/engine/fk_cascade.go | 27 ++- go/vt/vtgate/engine/fk_cascade_test.go | 4 +- go/vt/vtgate/planbuilder/delete.go | 2 +- go/vt/vtgate/planbuilder/fk_cascade.go | 85 ++++++++++ go/vt/vtgate/planbuilder/insert.go | 2 +- .../planbuilder/operator_transformers.go | 70 ++++++-- go/vt/vtgate/planbuilder/operators/ast2op.go | 2 +- .../planbuilder/operators/fk_cascade.go | 23 ++- go/vt/vtgate/planbuilder/select.go | 2 +- go/vt/vtgate/planbuilder/subquery_op.go | 8 +- .../testdata/foreignkey_cases.json | 156 +++++++++++++++++- .../vtgate/planbuilder/testdata/onecase.json | 8 +- go/vt/vtgate/planbuilder/update.go | 2 +- go/vt/vtgate/semantics/semantic_state.go | 6 + 14 files changed, 350 insertions(+), 47 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/fk_cascade.go diff --git a/go/vt/vtgate/engine/fk_cascade.go b/go/vt/vtgate/engine/fk_cascade.go index f32767bf2bc..7c8d20f4ab7 100644 --- a/go/vt/vtgate/engine/fk_cascade.go +++ b/go/vt/vtgate/engine/fk_cascade.go @@ -39,7 +39,7 @@ type FkCascade struct { // Selection is the Primitive that is used to find the rows that are going to be modified in the child tables. Selection Primitive // Children is a list of child foreign key Primitives that are executed using rows from the Selection Primitive. - Children []FkChild + Children []*FkChild // Parent is the Primitive that is executed after the children are modified. Parent Primitive @@ -168,17 +168,30 @@ func (fkc *FkCascade) TryStreamExecute(ctx context.Context, vcursor VCursor, bin // Inputs implements the Primitive interface. func (fkc *FkCascade) Inputs() []Primitive { - inputs := []Primitive{fkc.Selection} - for _, child := range fkc.Children { - inputs = append(inputs, child.Exec) - } - inputs = append(inputs, fkc.Parent) - return inputs + return nil } func (fkc *FkCascade) description() PrimitiveDescription { + var childrenDesc []PrimitiveDescription + for _, child := range fkc.Children { + childrenDesc = append(childrenDesc, PrimitiveDescription{ + OperatorType: "FkCascadeChild", + Inputs: []PrimitiveDescription{ + PrimitiveToPlanDescription(child.Exec), + }, + Other: map[string]any{ + "BvName": child.BVName, + "Cols": child.Cols, + }, + }) + } return PrimitiveDescription{ OperatorType: fkc.RouteType(), + Other: map[string]any{ + "Selection": PrimitiveToPlanDescription(fkc.Selection), + "Parent": PrimitiveToPlanDescription(fkc.Parent), + "Children": childrenDesc, + }, } } diff --git a/go/vt/vtgate/engine/fk_cascade_test.go b/go/vt/vtgate/engine/fk_cascade_test.go index a8aa1055c86..6c89feebf95 100644 --- a/go/vt/vtgate/engine/fk_cascade_test.go +++ b/go/vt/vtgate/engine/fk_cascade_test.go @@ -58,7 +58,7 @@ func TestDeleteCascade(t *testing.T) { } fkc := &FkCascade{ Selection: inputP, - Children: []FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}}, + Children: []*FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}}, Parent: parentP, } @@ -119,7 +119,7 @@ func TestUpdateCascade(t *testing.T) { } fkc := &FkCascade{ Selection: inputP, - Children: []FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}}, + Children: []*FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}}, Parent: parentP, } diff --git a/go/vt/vtgate/planbuilder/delete.go b/go/vt/vtgate/planbuilder/delete.go index e12b6c9abac..034d1fa020f 100644 --- a/go/vt/vtgate/planbuilder/delete.go +++ b/go/vt/vtgate/planbuilder/delete.go @@ -78,7 +78,7 @@ func gen4DeleteStmtPlanner( return nil, err } - plan, err := transformToLogicalPlan(ctx, op, true) + plan, err := transformToLogicalPlan(ctx, op) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/fk_cascade.go b/go/vt/vtgate/planbuilder/fk_cascade.go new file mode 100644 index 00000000000..5a709156955 --- /dev/null +++ b/go/vt/vtgate/planbuilder/fk_cascade.go @@ -0,0 +1,85 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package planbuilder + +import ( + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" + "vitess.io/vitess/go/vt/vtgate/semantics" +) + +var _ logicalPlan = (*fkCascade)(nil) + +// fkCascade is the logicalPlan for engine.FkCascade. +type fkCascade struct { + parent logicalPlan + selection logicalPlan + children []*engine.FkChild +} + +// newFkCascade builds a new fkCascade. +func newFkCascade(parent, selection logicalPlan, children []*engine.FkChild) *fkCascade { + return &fkCascade{ + parent: parent, + selection: selection, + children: children, + } +} + +// Primitive implements the logicalPlan interface +func (fkc *fkCascade) Primitive() engine.Primitive { + return &engine.FkCascade{ + Parent: fkc.parent.Primitive(), + Selection: fkc.selection.Primitive(), + Children: fkc.children, + } +} + +// Wireup implements the logicalPlan interface +func (fkc *fkCascade) Wireup(ctx *plancontext.PlanningContext) error { + if err := fkc.parent.Wireup(ctx); err != nil { + return err + } + return fkc.selection.Wireup(ctx) +} + +// Rewrite implements the logicalPlan interface +func (fkc *fkCascade) Rewrite(inputs ...logicalPlan) error { + if len(inputs) != 2 { + return vterrors.VT13001("fkCascade: wrong number of inputs") + } + fkc.parent = inputs[0] + fkc.selection = inputs[1] + return nil +} + +// ContainsTables implements the logicalPlan interface +func (fkc *fkCascade) ContainsTables() semantics.TableSet { + return fkc.parent.ContainsTables() +} + +// Inputs implements the logicalPlan interface +func (fkc *fkCascade) Inputs() []logicalPlan { + return []logicalPlan{fkc.parent, fkc.selection} +} + +// OutputColumns implements the logicalPlan interface +func (fkc *fkCascade) OutputColumns() []sqlparser.SelectExpr { + return nil +} diff --git a/go/vt/vtgate/planbuilder/insert.go b/go/vt/vtgate/planbuilder/insert.go index 7ddd6919f9e..52066e9d1f7 100644 --- a/go/vt/vtgate/planbuilder/insert.go +++ b/go/vt/vtgate/planbuilder/insert.go @@ -67,7 +67,7 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm return nil, err } - plan, err := transformToLogicalPlan(ctx, op, true) + plan, err := transformToLogicalPlan(ctx, op) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/operator_transformers.go b/go/vt/vtgate/planbuilder/operator_transformers.go index 51faea89cf4..283ee147ba4 100644 --- a/go/vt/vtgate/planbuilder/operator_transformers.go +++ b/go/vt/vtgate/planbuilder/operator_transformers.go @@ -37,7 +37,7 @@ import ( "vitess.io/vitess/go/vt/vtgate/vindexes" ) -func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator, isRoot bool) (logicalPlan, error) { +func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator) (logicalPlan, error) { switch op := op.(type) { case *operators.Route: return transformRoutePlan(ctx, op) @@ -54,7 +54,7 @@ func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator, i case *operators.Filter: return transformFilter(ctx, op) case *operators.Horizon: - return transformHorizon(ctx, op, isRoot) + return transformHorizon(ctx, op) case *operators.Projection: return transformProjection(ctx, op) case *operators.Limit: @@ -65,13 +65,53 @@ func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator, i return transformAggregator(ctx, op) case *operators.Distinct: return transformDistinct(ctx, op) + case *operators.FkCascade: + return transformFkCascade(ctx, op) } return nil, vterrors.VT13001(fmt.Sprintf("unknown type encountered: %T (transformToLogicalPlan)", op)) } +// transformFkCascade transforms a FkCascade operator into a logical plan. +func transformFkCascade(ctx *plancontext.PlanningContext, fkc *operators.FkCascade) (logicalPlan, error) { + // We convert the parent operator to a logical plan. + parentLP, err := transformToLogicalPlan(ctx, fkc.Parent) + if err != nil { + return nil, nil + } + + // Once we have the parent logical plan, we can create the selection logical plan and the primitives for the children operators. + // For all of these, we don't need the semTable anymore. We set it to nil, to avoid using an incorrect one. + ctx.SemTable = nil + selLP, err := transformToLogicalPlan(ctx, fkc.Selection) + if err != nil { + return nil, err + } + + // Go over the children and convert them to Primitives too. + var children []*engine.FkChild + for _, child := range fkc.Children { + childLP, err := transformToLogicalPlan(ctx, child.Op) + if err != nil { + return nil, err + } + err = childLP.Wireup(ctx) + if err != nil { + return nil, err + } + childEngine := childLP.Primitive() + children = append(children, &engine.FkChild{ + BVName: child.BVName, + Cols: child.Cols, + Exec: childEngine, + }) + } + + return newFkCascade(parentLP, selLP, children), nil +} + func transformAggregator(ctx *plancontext.PlanningContext, op *operators.Aggregator) (logicalPlan, error) { - plan, err := transformToLogicalPlan(ctx, op.Source, false) + plan, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } @@ -111,7 +151,7 @@ func transformAggregator(ctx *plancontext.PlanningContext, op *operators.Aggrega } func transformDistinct(ctx *plancontext.PlanningContext, op *operators.Distinct) (logicalPlan, error) { - src, err := transformToLogicalPlan(ctx, op.Source, false) + src, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } @@ -119,7 +159,7 @@ func transformDistinct(ctx *plancontext.PlanningContext, op *operators.Distinct) } func transformOrdering(ctx *plancontext.PlanningContext, op *operators.Ordering) (logicalPlan, error) { - plan, err := transformToLogicalPlan(ctx, op.Source, false) + plan, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } @@ -152,7 +192,7 @@ func createMemorySort(ctx *plancontext.PlanningContext, src logicalPlan, orderin } func transformProjection(ctx *plancontext.PlanningContext, op *operators.Projection) (logicalPlan, error) { - src, err := transformToLogicalPlan(ctx, op.Source, false) + src, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } @@ -232,7 +272,7 @@ func elementsMatchIndices(in []int) bool { } func transformFilter(ctx *plancontext.PlanningContext, op *operators.Filter) (logicalPlan, error) { - plan, err := transformToLogicalPlan(ctx, op.Source, false) + plan, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } @@ -262,11 +302,11 @@ func transformFilter(ctx *plancontext.PlanningContext, op *operators.Filter) (lo }, nil } -func transformHorizon(ctx *plancontext.PlanningContext, op *operators.Horizon, isRoot bool) (logicalPlan, error) { +func transformHorizon(ctx *plancontext.PlanningContext, op *operators.Horizon) (logicalPlan, error) { if op.IsDerived() { return transformDerivedPlan(ctx, op) } - source, err := transformToLogicalPlan(ctx, op.Source, isRoot) + source, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } @@ -305,11 +345,11 @@ func transformHorizon(ctx *plancontext.PlanningContext, op *operators.Horizon, i } func transformApplyJoinPlan(ctx *plancontext.PlanningContext, n *operators.ApplyJoin) (logicalPlan, error) { - lhs, err := transformToLogicalPlan(ctx, n.LHS, false) + lhs, err := transformToLogicalPlan(ctx, n.LHS) if err != nil { return nil, err } - rhs, err := transformToLogicalPlan(ctx, n.RHS, false) + rhs, err := transformToLogicalPlan(ctx, n.RHS) if err != nil { return nil, err } @@ -421,7 +461,7 @@ func transformInsertPlan(ctx *plancontext.PlanningContext, op *operators.Route, if ins.Input == nil { eins.Query = generateQuery(ins.AST) } else { - i.source, err = transformToLogicalPlan(ctx, ins.Input, true) + i.source, err = transformToLogicalPlan(ctx, ins.Input) if err != nil { return } @@ -640,7 +680,7 @@ func getAllTableNames(op *operators.Route) ([]string, error) { func transformUnionPlan(ctx *plancontext.PlanningContext, op *operators.Union) (logicalPlan, error) { sources, err := slice.MapWithError(op.Sources, func(src ops.Operator) (logicalPlan, error) { - plan, err := transformToLogicalPlan(ctx, src, false) + plan, err := transformToLogicalPlan(ctx, src) if err != nil { return nil, err } @@ -667,7 +707,7 @@ func transformDerivedPlan(ctx *plancontext.PlanningContext, op *operators.Horizo // expression containing our derived table's inner select and the derived // table's alias. - plan, err := transformToLogicalPlan(ctx, op.Source, false) + plan, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } @@ -707,7 +747,7 @@ func transformDerivedPlan(ctx *plancontext.PlanningContext, op *operators.Horizo } func transformLimit(ctx *plancontext.PlanningContext, op *operators.Limit) (logicalPlan, error) { - plan, err := transformToLogicalPlan(ctx, op.Source, false) + plan, err := transformToLogicalPlan(ctx, op.Source) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 6b3e5e9e255..5e4af7257a1 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -242,7 +242,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp return nil, vterrors.VT12001("foreign keys management at vitess with limit") } - var fkChildren []ops.Operator + var fkChildren []*FkChild var selectExprs []sqlparser.SelectExpr for _, fk := range childFks { // Any RESTRICT type foreign keys that arrive here, diff --git a/go/vt/vtgate/planbuilder/operators/fk_cascade.go b/go/vt/vtgate/planbuilder/operators/fk_cascade.go index 2d5d8693f01..8618dcf8261 100644 --- a/go/vt/vtgate/planbuilder/operators/fk_cascade.go +++ b/go/vt/vtgate/planbuilder/operators/fk_cascade.go @@ -22,7 +22,7 @@ import ( type FkCascade struct { Selection ops.Operator - Children []ops.Operator + Children []*FkChild Parent ops.Operator noColumns @@ -35,7 +35,9 @@ func (fkc *FkCascade) Inputs() []ops.Operator { var inputs []ops.Operator inputs = append(inputs, fkc.Parent) inputs = append(inputs, fkc.Selection) - inputs = append(inputs, fkc.Children...) + for _, child := range fkc.Children { + inputs = append(inputs, child) + } return inputs } @@ -45,7 +47,12 @@ func (fkc *FkCascade) SetInputs(operators []ops.Operator) { } fkc.Parent = operators[0] fkc.Selection = operators[1] - fkc.Children = operators[2:] + for idx, operator := range operators { + if idx < 2 { + continue + } + fkc.Children[idx-2] = operator.(*FkChild) + } } // Clone implements the Operator interface @@ -53,11 +60,17 @@ func (fkc *FkCascade) Clone(inputs []ops.Operator) ops.Operator { if len(inputs) < 2 { panic("incorrect count of inputs for FkCascade") } - return &FkCascade{ + newFkc := &FkCascade{ Parent: inputs[0], Selection: inputs[1], - Children: inputs[2:], } + for idx, operator := range inputs { + if idx < 2 { + continue + } + newFkc.Children = append(newFkc.Children, operator.(*FkChild)) + } + return newFkc } func (fkc *FkCascade) GetOrdering() ([]ops.OrderBy, error) { diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index ce5857a01e2..15f3ba245da 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -227,7 +227,7 @@ func newBuildSelectPlan( return nil, nil, err } - plan, err := transformToLogicalPlan(ctx, op, true) + plan, err := transformToLogicalPlan(ctx, op) if err != nil { return nil, nil, err } diff --git a/go/vt/vtgate/planbuilder/subquery_op.go b/go/vt/vtgate/planbuilder/subquery_op.go index d2fd30c05c3..7faf7291a79 100644 --- a/go/vt/vtgate/planbuilder/subquery_op.go +++ b/go/vt/vtgate/planbuilder/subquery_op.go @@ -25,7 +25,7 @@ import ( ) func transformSubQueryPlan(ctx *plancontext.PlanningContext, op *operators.SubQueryOp) (logicalPlan, error) { - innerPlan, err := transformToLogicalPlan(ctx, op.Inner, false) + innerPlan, err := transformToLogicalPlan(ctx, op.Inner) if err != nil { return nil, err } @@ -36,7 +36,7 @@ func transformSubQueryPlan(ctx *plancontext.PlanningContext, op *operators.SubQu argName := op.Extracted.GetArgName() hasValuesArg := op.Extracted.GetHasValuesArg() - outerPlan, err := transformToLogicalPlan(ctx, op.Outer, false) + outerPlan, err := transformToLogicalPlan(ctx, op.Outer) merged := mergeSubQueryOpPlan(ctx, innerPlan, outerPlan, op) if merged != nil { @@ -51,11 +51,11 @@ func transformSubQueryPlan(ctx *plancontext.PlanningContext, op *operators.SubQu } func transformCorrelatedSubQueryPlan(ctx *plancontext.PlanningContext, op *operators.CorrelatedSubQueryOp) (logicalPlan, error) { - outer, err := transformToLogicalPlan(ctx, op.Outer, false) + outer, err := transformToLogicalPlan(ctx, op.Outer) if err != nil { return nil, err } - inner, err := transformToLogicalPlan(ctx, op.Inner, false) + inner, err := transformToLogicalPlan(ctx, op.Inner) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index f316dd68e92..f525f80eb3d 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -57,22 +57,166 @@ { "comment": "Delete in a table with cross-shard foreign keys disallowed", "query": "delete from tbl1", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": "VT12002: unsupported: cross-shard foreign keys" }, { "comment": "Delete in a table with not all column shard-scoped foreign keys - disallowed", "query": "delete from tbl7", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": "VT12002: unsupported: cross-shard foreign keys" }, { - "comment": "Delete in a table with shard-scoped multiple column foreign key with cascade not allowed", + "comment": "Delete in a table with shard-scoped multiple column foreign key with cascade", "query": "delete from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": { + "QueryType": "DELETE", + "Original": "delete from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Instructions": { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals", + "Cols": [ + 0, + 1, + 2, + 3, + 4 + ], + "Inputs": [ + { + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from multicol_tbl2 where (colb, cola, x, colc, y) in ::fkc_vals", + "Table": "multicol_tbl2" + } + ] + } + ], + "Parent": { + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Table": "multicol_tbl1", + "Values": [ + "INT64(1)", + "INT64(2)", + "INT64(3)" + ], + "Vindex": "multicolIdx" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select colb, cola, y, colc, x from multicol_tbl1 where 1 != 1", + "Query": "select colb, cola, y, colc, x from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3 lock in share mode", + "Table": "multicol_tbl1", + "Values": [ + "INT64(1)", + "INT64(2)", + "INT64(3)" + ], + "Vindex": "multicolIdx" + } + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl1", + "sharded_fk_allow.multicol_tbl2" + ] + } }, { - "comment": "Delete in a table with shard-scoped foreign keys with cascade disallowed", + "comment": "Delete in a table with shard-scoped foreign keys with cascade", "query": "delete from tbl5", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl5", + "Instructions": { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from tbl4 where (col4) in ::fkc_vals", + "Table": "tbl4" + } + ] + }, + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals1", + "Cols": [ + 1 + ], + "Inputs": [ + { + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from tbl4 where (t4col4) in ::fkc_vals1", + "Table": "tbl4" + } + ] + } + ], + "Parent": { + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from tbl5", + "Table": "tbl5" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select col5, t5col5 from tbl5 where 1 != 1", + "Query": "select col5, t5col5 from tbl5 lock in share mode", + "Table": "tbl5" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl5" + ] + } }, { "comment": "update in unsharded table with restrict", diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.json b/go/vt/vtgate/planbuilder/testdata/onecase.json index f303e58ef7e..da7543f706a 100644 --- a/go/vt/vtgate/planbuilder/testdata/onecase.json +++ b/go/vt/vtgate/planbuilder/testdata/onecase.json @@ -1,7 +1,9 @@ [ { - "comment": "Delete in a table with shard-scoped foreign keys with cascade disallowed", - "query": "delete from tbl5", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "comment": "Add your test case here for debugging and run go test -run=One.", + "query": "", + "plan": { + + } } ] \ No newline at end of file diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index c0f95aa9e73..0f18786c3f9 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -70,7 +70,7 @@ func gen4UpdateStmtPlanner( return nil, err } - plan, err := transformToLogicalPlan(ctx, op, true) + plan, err := transformToLogicalPlan(ctx, op) if err != nil { return nil, err } diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 24ca5db5eef..5618f88c807 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -215,6 +215,9 @@ func (st *SemTable) TableSetFor(t *sqlparser.AliasedTableExpr) TableSet { // ReplaceTableSetFor replaces the given single TabletSet with the new *sqlparser.AliasedTableExpr func (st *SemTable) ReplaceTableSetFor(id TableSet, t *sqlparser.AliasedTableExpr) { + if st == nil { + return + } if id.NumberOfTables() != 1 { // This is probably a derived table return @@ -393,6 +396,9 @@ func (st *SemTable) FindSubqueryReference(subquery *sqlparser.Subquery) *sqlpars // GetSubqueryNeedingRewrite returns a list of sub-queries that need to be rewritten func (st *SemTable) GetSubqueryNeedingRewrite() []*sqlparser.ExtractedSubquery { + if st == nil { + return nil + } var res []*sqlparser.ExtractedSubquery for _, extractedSubquery := range st.SubqueryRef { if extractedSubquery.Merged { From 272ec1d34049281798a6b44a50605c4337112af5 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 13:05:46 +0530 Subject: [PATCH 04/31] feat: error out for setNull delete action and fix tests Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 53 ++++++++++--------- go/vt/vtgate/planbuilder/plan_test.go | 7 ++- .../testdata/foreignkey_cases.json | 5 ++ .../planbuilder/testdata/vschemas/schema.json | 8 +++ 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 5e4af7257a1..cfce3383ed2 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -258,31 +258,36 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) } - // We now construct the delete query for the child table. - // The query looks something like this - `DELETE FROM WHERE IN ()` - bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") - var valTuple sqlparser.ValTuple - for _, column := range fk.ChildColumns { - valTuple = append(valTuple, sqlparser.NewColName(column.String())) - } - compExpr := &sqlparser.ComparisonExpr{ - Operator: sqlparser.InOp, - Left: valTuple, - Right: sqlparser.NewListArg(bvName), - } - childDel := &sqlparser.Delete{ - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, - } - childDelOp, err := createOpFromStmt(ctx, childDel) - if err != nil { - return nil, err + switch fk.OnDelete { + case sqlparser.Cascade: + // We now construct the delete query for the child table. + // The query looks something like this - `DELETE FROM WHERE IN ()` + bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + compExpr := &sqlparser.ComparisonExpr{ + Operator: sqlparser.InOp, + Left: valTuple, + Right: sqlparser.NewListArg(bvName), + } + childDel := &sqlparser.Delete{ + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + } + childDelOp, err := createOpFromStmt(ctx, childDel) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, &FkChild{ + BVName: bvName, + Cols: cols, + Op: childDelOp, + }) + default: + return nil, vterrors.VT12003() } - fkChildren = append(fkChildren, &FkChild{ - BVName: bvName, - Cols: cols, - Op: childDelOp, - }) } // We now create the selection operator to select the parent columns for the foreign key constraints. // The Select statement looks something like this - `SELECT FROM WHERE ` diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index df0e3ec0a94..5c704a83e4d 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -125,9 +125,12 @@ func setFks(t *testing.T, vschema *vindexes.VSchema) { // FK from tbl10 referencing tbl3 that is not shard scoped. _ = vschema.AddForeignKey("sharded_fk_allow", "tbl10", createFkDefinition([]string{"col"}, "tbl3", []string{"col"}, sqlparser.Restrict, sqlparser.Restrict)) + // FK from tbl5 referencing tbl8 that is shard scoped of SET-NULL types. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl5", createFkDefinition([]string{"col5"}, "tbl8", []string{"col8"}, sqlparser.SetNull, sqlparser.SetNull)) + // FK from tbl4 referencing tbl5 that is shard scoped. - _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"col4"}, "tbl5", []string{"col5"}, sqlparser.SetNull, sqlparser.SetNull)) - _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"t4col4"}, "tbl5", []string{"t5col5"}, sqlparser.SetNull, sqlparser.SetNull)) + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"col4"}, "tbl5", []string{"col5"}, sqlparser.SetNull, sqlparser.Cascade)) + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"t4col4"}, "tbl5", []string{"t5col5"}, sqlparser.SetNull, sqlparser.Cascade)) // FK from tbl6 referencing tbl7 that is shard scoped. _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"col6"}, "tbl7", []string{"col7"}, sqlparser.NoAction, sqlparser.NoAction)) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index f525f80eb3d..3a8edc4055c 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -218,6 +218,11 @@ ] } }, + { + "comment": "Delete in a table with shard-scoped foreign keys with SET NULL", + "query": "delete from tbl8 where col8 = 1", + "plan": "VT12002: unsupported: foreign keys management at vitess" + }, { "comment": "update in unsharded table with restrict", "query": "update u_tbl5 set col5 = 'foo' where id = 1", diff --git a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json index 0b474163ce5..4b07b36463b 100644 --- a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json +++ b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json @@ -695,6 +695,14 @@ } ] }, + "tbl8": { + "column_vindexes": [ + { + "column": "col8", + "name": "hash_vin" + } + ] + }, "tbl10": { "column_vindexes": [ { From 836c04cbbd0f3d80673f114e3851cb71d0078c63 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 13:13:58 +0530 Subject: [PATCH 05/31] feat: fix e2e tests Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index 531aed2642a..9ba484ed8e2 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -76,11 +76,14 @@ func TestDeleteWithFK(t *testing.T) { // table's child foreign key has cross shard fk, so query will fail at vtgate. _, err = utils.ExecAllowError(t, conn, `delete from t1 where id = 42`) - assert.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)") + assert.ErrorContains(t, err, "VT12002: unsupported: cross-shard foreign keys (errno 1235) (sqlstate 42000)") - // child foreign key is cascade, so query will fail at vtgate. - _, err = utils.ExecAllowError(t, conn, `delete from multicol_tbl1 where cola = 100`) - assert.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)") + // child foreign key is cascade, so this should work as expected. + qr = utils.Exec(t, conn, `delete from multicol_tbl1 where cola = 100`) + assert.EqualValues(t, 1, qr.RowsAffected) + // we also verify that the rows in the child table were deleted. + qr = utils.Exec(t, conn, `select * from multicol_tbl2 where cola = 100`) + assert.Zero(t, qr.Rows) } // TestUpdations tests that update work as expected when foreign key management is enabled in Vitess. From 5f6a3cc030ebb03d19dddb76fa076db9f481c614 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 14:02:41 +0530 Subject: [PATCH 06/31] feat: run sizegen Signed-off-by: Manan Gupta --- go/vt/vtgate/engine/cached_size.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/vt/vtgate/engine/cached_size.go b/go/vt/vtgate/engine/cached_size.go index 19329df5cd1..838b6b5356b 100644 --- a/go/vt/vtgate/engine/cached_size.go +++ b/go/vt/vtgate/engine/cached_size.go @@ -261,11 +261,11 @@ func (cached *FkCascade) CachedSize(alloc bool) int64 { if cc, ok := cached.Selection.(cachedObject); ok { size += cc.CachedSize(true) } - // field Children []vitess.io/vitess/go/vt/vtgate/engine.FkChild + // field Children []*vitess.io/vitess/go/vt/vtgate/engine.FkChild { - size += hack.RuntimeAllocSize(int64(cap(cached.Children)) * int64(56)) + size += hack.RuntimeAllocSize(int64(cap(cached.Children)) * int64(8)) for _, elem := range cached.Children { - size += elem.CachedSize(false) + size += elem.CachedSize(true) } } // field Parent vitess.io/vitess/go/vt/vtgate/engine.Primitive From f6e67895b7869c03be6ac76e7f42c31e728af870 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 21 Aug 2023 15:00:05 +0530 Subject: [PATCH 07/31] feat: added planning for ON DELETE SET NULL Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/operators/ast2op.go | 34 ++++++++++++++++++- .../testdata/foreignkey_cases.json | 2 +- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index cfce3383ed2..270cf22a524 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -285,7 +285,39 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp Cols: cols, Op: childDelOp, }) - default: + case sqlparser.SetNull: + // We now construct the update query for the child table. + // The query looks something like this - `UPDATE SET = NULL WHERE IN ()` + bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") + var valTuple sqlparser.ValTuple + var updExprs sqlparser.UpdateExprs + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + updExprs = append(updExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(column.String()), + Expr: &sqlparser.NullVal{}, + }) + } + compExpr := &sqlparser.ComparisonExpr{ + Operator: sqlparser.InOp, + Left: valTuple, + Right: sqlparser.NewListArg(bvName), + } + childUpd := &sqlparser.Update{ + Exprs: updExprs, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + } + childUpdOp, err := createOpFromStmt(ctx, childUpd) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, &FkChild{ + BVName: bvName, + Cols: cols, + Op: childUpdOp, + }) + case sqlparser.SetDefault: return nil, vterrors.VT12003() } } diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 3a8edc4055c..8eafec4621e 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -221,7 +221,7 @@ { "comment": "Delete in a table with shard-scoped foreign keys with SET NULL", "query": "delete from tbl8 where col8 = 1", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": "VT12001: unsupported: you cannot UPDATE primary vindex columns; invalid update on vindex: hash_vin" }, { "comment": "update in unsharded table with restrict", From c02be0a783d2ac973c4c5243651e8e2df89fe23f Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 16:37:25 +0530 Subject: [PATCH 08/31] feat: add unit test for cascade SET-NULL delete query Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 2 +- go/vt/vtgate/planbuilder/plan_test.go | 2 + .../testdata/foreignkey_cases.json | 59 +++++++++++++++++++ .../planbuilder/testdata/vschemas/schema.json | 13 +++- 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 270cf22a524..114063f0552 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -287,7 +287,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp }) case sqlparser.SetNull: // We now construct the update query for the child table. - // The query looks something like this - `UPDATE SET = NULL WHERE IN ()` + // The query looks something like this - `UPDATE SET = NULL [AND = NULL]... WHERE IN ()` bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") var valTuple sqlparser.ValTuple var updExprs sqlparser.UpdateExprs diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 5c704a83e4d..e5cf6d2058d 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -143,12 +143,14 @@ func setFks(t *testing.T, vschema *vindexes.VSchema) { // u_tbl4(col41) -> u_tbl1(col14) Restrict. // u_tbl4(col4) -> u_tbl3(col3) Restrict. // u_tbl6(col6) -> u_tbl5(col5) Restrict. + // u_tbl8(col8) -> u_tbl9(col9) Cascade Null. _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl2", createFkDefinition([]string{"col2"}, "u_tbl1", []string{"col1"}, sqlparser.Cascade, sqlparser.Cascade)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl3", createFkDefinition([]string{"col3"}, "u_tbl2", []string{"col2"}, sqlparser.SetNull, sqlparser.SetNull)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl4", createFkDefinition([]string{"col41"}, "u_tbl1", []string{"col14"}, sqlparser.NoAction, sqlparser.NoAction)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl4", createFkDefinition([]string{"col4"}, "u_tbl3", []string{"col3"}, sqlparser.Restrict, sqlparser.Restrict)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl6", createFkDefinition([]string{"col6"}, "u_tbl5", []string{"col5"}, sqlparser.DefaultAction, sqlparser.DefaultAction)) + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl8", createFkDefinition([]string{"col8"}, "u_tbl9", []string{"col9"}, sqlparser.SetNull, sqlparser.SetNull)) } } diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 8eafec4621e..0e405ccb696 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -223,6 +223,65 @@ "query": "delete from tbl8 where col8 = 1", "plan": "VT12001: unsupported: you cannot UPDATE primary vindex columns; invalid update on vindex: hash_vin" }, + { + "comment": "Delete in a table with unsharded foreign key with SET NULL", + "query": "delete from u_tbl9 where col9 = 5", + "plan": { + "QueryType": "DELETE", + "Original": "delete from u_tbl9 where col9 = 5", + "Instructions": { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl8 set col8 = null where (col8) in ::fkc_vals", + "Table": "u_tbl8" + } + ] + } + ], + "Parent": { + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from u_tbl9 where col9 = 5", + "Table": "u_tbl9" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col9 from u_tbl9 where 1 != 1", + "Query": "select col9 from u_tbl9 where col9 = 5 lock in share mode", + "Table": "u_tbl9" + } + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl8", + "unsharded_fk_allow.u_tbl9" + ] + } + }, { "comment": "update in unsharded table with restrict", "query": "update u_tbl5 set col5 = 'foo' where id = 1", diff --git a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json index 4b07b36463b..fdba9155707 100644 --- a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json +++ b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json @@ -703,6 +703,14 @@ } ] }, + "tbl9": { + "column_vindexes": [ + { + "column": "col9", + "name": "hash_vin" + } + ] + }, "tbl10": { "column_vindexes": [ { @@ -721,7 +729,10 @@ "u_tbl3": {}, "u_tbl4": {}, "u_tbl5": {}, - "u_tbl6": {} + "u_tbl6": {}, + "u_tbl7": {}, + "u_tbl8": {}, + "u_tbl9": {} } } } From a554bfb7ac947279ffa4491515aca0b57040a756 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 16:58:18 +0530 Subject: [PATCH 09/31] test: augment foreign_key e2e test to verify set null cascading in unsharded keyspace Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 12 ++++++++ .../endtoend/vtgate/foreignkey/main_test.go | 29 +++++++++++++++++-- .../vtgate/foreignkey/unsharded_schema.sql | 15 ++++++++++ .../vtgate/foreignkey/unsharded_vschema.json | 8 +++++ 4 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql create mode 100644 go/test/endtoend/vtgate/foreignkey/unsharded_vschema.json diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index 9ba484ed8e2..be79d3790ae 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -84,6 +84,18 @@ func TestDeleteWithFK(t *testing.T) { // we also verify that the rows in the child table were deleted. qr = utils.Exec(t, conn, `select * from multicol_tbl2 where cola = 100`) assert.Zero(t, qr.Rows) + + // Unsharded keyspace tests + utils.Exec(t, conn, `use uks`) + // insert some data. + utils.Exec(t, conn, `insert into u_t1(id, col1) values (100, 123), (10, 12), (1, 13), (1000, 1234)`) + utils.Exec(t, conn, `insert into u_t2(id, col2) values (342, 123), (19, 1234)`) + + // Delete from u_t1 which has a foreign key constraint to t2 with SET NULL type. + qr = utils.Exec(t, conn, `delete from u_t1 where id = 100`) + assert.EqualValues(t, 1, qr.RowsAffected) + // Verify the result in u_t2 as well + utils.AssertMatches(t, conn, `select * from u_t2`, `[[INT64(342) NULL] [INT64(19) INT64(1234)]]`) } // TestUpdations tests that update work as expected when foreign key management is enabled in Vitess. diff --git a/go/test/endtoend/vtgate/foreignkey/main_test.go b/go/test/endtoend/vtgate/foreignkey/main_test.go index 8cd9cdcadc1..bd1b3391982 100644 --- a/go/test/endtoend/vtgate/foreignkey/main_test.go +++ b/go/test/endtoend/vtgate/foreignkey/main_test.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Vitess Authors. +Copyright 2021 The Vitess Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -35,13 +35,19 @@ var ( clusterInstance *cluster.LocalProcessCluster vtParams mysql.ConnParams shardedKs = "ks" + unshardedKs = "uks" Cell = "test" - //go:embed sharded_schema.sql shardedSchemaSQL string + //go:embed unsharded_schema.sql + unshardedSchemaSQL string + //go:embed sharded_vschema.json shardedVSchema string + + //go:embed unsharded_vschema.json + unshardedVSchema string ) func TestMain(m *testing.M) { @@ -70,6 +76,21 @@ func TestMain(m *testing.M) { return 1 } + uKs := &cluster.Keyspace{ + Name: unshardedKs, + SchemaSQL: unshardedSchemaSQL, + VSchema: unshardedVSchema, + } + err = clusterInstance.StartUnshardedKeyspace(*uKs, 0, false) + if err != nil { + return 1 + } + + err = clusterInstance.VtctlclientProcess.ExecuteCommand("RebuildVSchemaGraph") + if err != nil { + return 1 + } + // Start vtgate err = clusterInstance.StartVtgate() if err != nil { @@ -99,6 +120,10 @@ func start(t *testing.T) (*mysql.Conn, func()) { for _, table := range tables { _ = utils.Exec(t, conn, "delete from "+table) } + _ = utils.Exec(t, conn, "use `uks`") + for _, table := range []string{"u_t1", "u_t2"} { + _ = utils.Exec(t, conn, "delete from "+table) + } _ = utils.Exec(t, conn, "use `ks`") } diff --git a/go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql b/go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql new file mode 100644 index 00000000000..94c5bbcbdfd --- /dev/null +++ b/go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql @@ -0,0 +1,15 @@ +create table u_t1 +( + id bigint, + col1 bigint, + index(col1), + primary key (id) +) Engine = InnoDB; + +create table u_t2 +( + id bigint, + col2 bigint, + primary key (id), + foreign key (col2) references u_t1 (col1) on delete set null on update set null +) Engine = InnoDB; diff --git a/go/test/endtoend/vtgate/foreignkey/unsharded_vschema.json b/go/test/endtoend/vtgate/foreignkey/unsharded_vschema.json new file mode 100644 index 00000000000..f46ecbe83c3 --- /dev/null +++ b/go/test/endtoend/vtgate/foreignkey/unsharded_vschema.json @@ -0,0 +1,8 @@ +{ + "sharded": false, + "foreignKeyMode": "FK_MANAGED", + "tables": { + "u_a": {}, + "u_b": {} + } +} \ No newline at end of file From 2c7fdc7c09060ebb44d73e1f4be6770bef506d87 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 17:32:40 +0530 Subject: [PATCH 10/31] feat:fix the error message for SET DEFAULT type foreign keys in DELETE Signed-off-by: Manan Gupta --- go/mysql/sqlerror/sql_error.go | 1 + go/vt/vterrors/code.go | 2 ++ go/vt/vterrors/state.go | 1 + go/vt/vtgate/planbuilder/operators/ast2op.go | 2 +- 4 files changed, 5 insertions(+), 1 deletion(-) diff --git a/go/mysql/sqlerror/sql_error.go b/go/mysql/sqlerror/sql_error.go index 7d9093661c9..06b0ade9e40 100644 --- a/go/mysql/sqlerror/sql_error.go +++ b/go/mysql/sqlerror/sql_error.go @@ -212,6 +212,7 @@ var stateToMysqlCode = map[vterrors.State]mysqlCode{ vterrors.ServerNotAvailable: {num: ERServerIsntAvailable, state: SSNetError}, vterrors.CantDoThisInTransaction: {num: ERCantDoThisDuringAnTransaction, state: SSCantDoThisDuringAnTransaction}, vterrors.RequiresPrimaryKey: {num: ERRequiresPrimaryKey, state: SSClientError}, + vterrors.RowIsReferenced2: {num: ERRowIsReferenced2, state: SSConstraintViolation}, vterrors.NoSuchSession: {num: ERUnknownComError, state: SSNetError}, vterrors.OperandColumns: {num: EROperandColumns, state: SSWrongNumberOfColumns}, vterrors.WrongValueCountOnRow: {num: ERWrongValueCountOnRow, state: SSWrongValueCountOnRow}, diff --git a/go/vt/vterrors/code.go b/go/vt/vterrors/code.go index 797f69ce342..aeddacc4a1e 100644 --- a/go/vt/vterrors/code.go +++ b/go/vt/vterrors/code.go @@ -76,6 +76,7 @@ var ( VT09013 = errorWithoutState("VT09013", vtrpcpb.Code_FAILED_PRECONDITION, "semi-sync plugins are not loaded", "Durability policy wants Vitess to use semi-sync, but the MySQL instances don't have the semi-sync plugin loaded.") VT09014 = errorWithoutState("VT09014", vtrpcpb.Code_FAILED_PRECONDITION, "vindex cannot be modified", "The vindex cannot be used as table in DML statement") VT09015 = errorWithoutState("VT09015", vtrpcpb.Code_FAILED_PRECONDITION, "schema tracking required", "This query cannot be planned without more information on the SQL schema. Please turn on schema tracking or add authoritative columns information to your VSchema.") + VT09016 = errorWithState("VT09016", vtrpcpb.Code_FAILED_PRECONDITION, RowIsReferenced2, "Cannot delete or update a parent row: a foreign key constraint fails", "SET DEFAULT is not supported by InnoDB") VT10001 = errorWithoutState("VT10001", vtrpcpb.Code_ABORTED, "foreign key constraints are not allowed", "Foreign key constraints are not allowed, see https://vitess.io/blog/2021-06-15-online-ddl-why-no-fk/.") @@ -143,6 +144,7 @@ var ( VT09013, VT09014, VT09015, + VT09016, VT10001, VT12001, VT12002, diff --git a/go/vt/vterrors/state.go b/go/vt/vterrors/state.go index 22ac5c06b8c..ce2792e35f0 100644 --- a/go/vt/vterrors/state.go +++ b/go/vt/vterrors/state.go @@ -55,6 +55,7 @@ const ( CantDoThisInTransaction RequiresPrimaryKey OperandColumns + RowIsReferenced2 UnknownStmtHandler // not found diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 114063f0552..a4cb6ab58d1 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -318,7 +318,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp Op: childUpdOp, }) case sqlparser.SetDefault: - return nil, vterrors.VT12003() + return nil, vterrors.VT09016() } } // We now create the selection operator to select the parent columns for the foreign key constraints. From 06d73ec82b76c520380e36bedb78ddfe666c08d1 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 21 Aug 2023 17:39:01 +0530 Subject: [PATCH 11/31] test: test for SET DEFAULT Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/plan_test.go | 10 +++++++--- .../planbuilder/testdata/foreignkey_cases.json | 5 +++++ .../planbuilder/testdata/vschemas/schema.json | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index e5cf6d2058d..fa1736e7808 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -125,17 +125,21 @@ func setFks(t *testing.T, vschema *vindexes.VSchema) { // FK from tbl10 referencing tbl3 that is not shard scoped. _ = vschema.AddForeignKey("sharded_fk_allow", "tbl10", createFkDefinition([]string{"col"}, "tbl3", []string{"col"}, sqlparser.Restrict, sqlparser.Restrict)) - // FK from tbl5 referencing tbl8 that is shard scoped of SET-NULL types. - _ = vschema.AddForeignKey("sharded_fk_allow", "tbl5", createFkDefinition([]string{"col5"}, "tbl8", []string{"col8"}, sqlparser.SetNull, sqlparser.SetNull)) - // FK from tbl4 referencing tbl5 that is shard scoped. _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"col4"}, "tbl5", []string{"col5"}, sqlparser.SetNull, sqlparser.Cascade)) _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"t4col4"}, "tbl5", []string{"t5col5"}, sqlparser.SetNull, sqlparser.Cascade)) + // FK from tbl5 referencing tbl8 that is shard scoped of SET-NULL types. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl5", createFkDefinition([]string{"col5"}, "tbl8", []string{"col8"}, sqlparser.SetNull, sqlparser.SetNull)) + // FK from tbl6 referencing tbl7 that is shard scoped. _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"col6"}, "tbl7", []string{"col7"}, sqlparser.NoAction, sqlparser.NoAction)) _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"t6col6"}, "tbl7", []string{"t7col7"}, sqlparser.NoAction, sqlparser.NoAction)) _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"t6col62"}, "tbl7", []string{"t7col72"}, sqlparser.NoAction, sqlparser.NoAction)) + + // FK from tblrefDef referencing tbl20 that is shard scoped of SET-Default types. + _ = vschema.AddForeignKey("sharded_fk_allow", "tblrefDef", createFkDefinition([]string{"ref"}, "tbl20", []string{"col"}, sqlparser.SetDefault, sqlparser.SetDefault)) + } if vschema.Keyspaces["unsharded_fk_allow"] != nil { // u_tbl2(col2) -> u_tbl1(col1) Cascade. diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 0e405ccb696..55f744fc7a2 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -399,5 +399,10 @@ "comment": "update table with column's parent foreign key cross shard - disallowed", "query": "update tbl10 set col = 'foo'", "plan": "VT12002: unsupported: foreign keys management at vitess" + }, + { + "comment": "delete table with shard scoped foreign key set default - disallowed", + "query": "delete from tbl20 where col = 'bar'", + "plan": "VT09016: Cannot delete or update a parent row: a foreign key constraint fails" } ] \ No newline at end of file diff --git a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json index fdba9155707..e7f38d88404 100644 --- a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json +++ b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json @@ -718,6 +718,22 @@ "name": "hash_vin" } ] + }, + "tblrefDef": { + "column_vindexes": [ + { + "column": "ref", + "name": "hash_vin" + } + ] + }, + "tbl20": { + "column_vindexes": [ + { + "column": "col", + "name": "hash_vin" + } + ] } } }, From af323bb5bc7ac669e1c3358721f0900efc238e3b Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 21 Aug 2023 23:54:50 +0530 Subject: [PATCH 12/31] feat: add fk on update cascade planner Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/operators/ast2op.go | 200 +++++++++++++++--- go/vt/vtgate/planbuilder/plan_test.go | 3 + .../testdata/foreignkey_cases.json | 73 ++++++- 3 files changed, 249 insertions(+), 27 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index a4cb6ab58d1..767386151e5 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -115,16 +115,135 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return nil, err } - assignments := make(map[string]sqlparser.Expr) - for _, set := range updStmt.Exprs { - assignments[set.Name.Name.String()] = set.Expr + vindexTable, routing, err := buildVindexTableForDML(ctx, tableInfo, qt, "update") + if err != nil { + return nil, err } - vindexTable, routing, err := buildVindexTableForDML(ctx, tableInfo, qt, "update") + updClone := sqlparser.CloneRefOfUpdate(updStmt) + updOp, err := createUpdateOperator(ctx, updStmt, vindexTable, qt, routing) if err != nil { return nil, err } + ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) + if err != nil { + return nil, err + } + // Unmanaged foreign-key-mode, we don't need to do anything. + if ksMode != vschemapb.Keyspace_FK_MANAGED { + return updOp, nil + } + + parentFKs := vindexTable.ParentFKsNeedsHandling() + childFks := vindexTable.ChildFKsNeedsHandling(vindexes.UpdateAction) + if len(childFks) == 0 && len(parentFKs) == 0 { + return updOp, nil + } + + parentFKs, childFks = fkNeedsHandling(updStmt.Exprs, parentFKs, childFks) + if len(parentFKs) > 0 { + return nil, vterrors.VT12003() + } + + // If there are no foreign key constraints, then we don't need to do anything. + if len(childFks) == 0 { + return updOp, nil + } + + // If the delete statement has a limit, we don't support it yet. + if updStmt.Limit != nil { + return nil, vterrors.VT12001("foreign keys management at vitess with limit") + } + + var fkChildren []*FkChild + var selectExprs []sqlparser.SelectExpr + for _, fk := range childFks { + // Any RESTRICT type foreign keys that arrive here, + // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. + if isRestrict(fk.OnUpdate) { + return nil, vterrors.VT12002() + } + + // We need to select all the parent columns for the foreign key constraint, to use in the update of the child table. + var cols []int + for _, column := range fk.ParentColumns { + cols = append(cols, len(selectExprs)) + selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) + } + var childUpdateExprs sqlparser.UpdateExprs + for _, updateExpr := range updClone.Exprs { + colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx >= 0 { + childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(fk.ParentColumns[colIdx].String()), + Expr: updateExpr.Expr, + }) + } + } + + switch fk.OnUpdate { + case sqlparser.Cascade: + // We now construct the update query for the child table. + // The query looks something like this - `UPDATE SET WHERE IN ()` + bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + compExpr := &sqlparser.ComparisonExpr{ + Operator: sqlparser.InOp, + Left: valTuple, + Right: sqlparser.NewListArg(bvName), + } + childUpd := &sqlparser.Update{ + Exprs: childUpdateExprs, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + } + childDelOp, err := createOpFromStmt(ctx, childUpd) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, &FkChild{ + BVName: bvName, + Cols: cols, + Op: childDelOp, + }) + + case sqlparser.SetNull: + return nil, vterrors.VT12003() + case sqlparser.SetDefault: + return nil, vterrors.VT09016() + } + } + + // We now create the selection operator to select the parent columns for the foreign key constraints. + // The Select statement looks something like this - `SELECT FROM WHERE ` + // TODO (@Harshit, @GuptaManan100): Compress the columns in the SELECT statement, if there are multiple foreign key constraints using the same columns. + selectionStmt := &sqlparser.Select{ + SelectExprs: selectExprs, + From: updClone.TableExprs, + Where: updClone.Where, + } + selection, err := createOpFromStmt(ctx, selectionStmt) + if err != nil { + return nil, err + } + + return &FkCascade{ + Selection: selection, + Children: fkChildren, + Parent: updOp, + }, nil +} + +func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, vindexTable *vindexes.Table, qt *QueryTable, routing Routing) (ops.Operator, error) { + assignments := make(map[string]sqlparser.Expr) + for _, set := range updStmt.Exprs { + assignments[set.Name.Name.String()] = set.Expr + } + vp, cvv, ovq, err := getUpdateVindexInformation(updStmt, vindexTable, qt.ID, qt.Predicates) if err != nil { return nil, err @@ -136,7 +255,6 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars } for _, predicate := range qt.Predicates { - var err error routing, err = UpdateRoutingLogic(ctx, predicate, routing) if err != nil { return nil, err @@ -160,21 +278,6 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars Routing: routing, } - ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) - if err != nil { - return nil, err - } - if ksMode == vschemapb.Keyspace_FK_MANAGED { - parentFKs := vindexTable.ParentFKsNeedsHandling() - childFks := vindexTable.ChildFKsNeedsHandling(vindexes.UpdateAction) - if (len(childFks) > 0 || len(parentFKs) > 0) && - ColumnModified(updStmt.Exprs, func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { - return parentFKs, childFks - }) { - return nil, vterrors.VT12003() - } - } - subq, err := createSubqueryFromStatement(ctx, updStmt) if err != nil { return nil, err @@ -186,6 +289,51 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return subq, nil } +func fkNeedsHandling(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.ParentFKInfo, childFks []vindexes.ChildFKInfo) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { + pFksRequiredMap := map[*vindexes.ParentFKInfo]bool{} + cFksRequiredMap := map[*vindexes.ChildFKInfo]bool{} + for _, updateExpr := range updateExprs { + for _, childFk := range childFks { + if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { + cFksRequiredMap[&childFk] = true + } + } + for _, parentFk := range parentFks { + _, isNull := updateExpr.Expr.(*sqlparser.NullVal) + if isNull { + continue + } + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + pFksRequiredMap[&parentFk] = true + } + } + } + for _, parentFk := range parentFks { + for _, updateExpr := range updateExprs { + _, isNull := updateExpr.Expr.(*sqlparser.NullVal) + if isNull { + continue + } + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + pFksRequiredMap[&parentFk] = false + } + } + } + var pFksNeedsHandling []vindexes.ParentFKInfo + var cFksNeedsHandling []vindexes.ChildFKInfo + for parentFk, required := range pFksRequiredMap { + if required { + pFksNeedsHandling = append(pFksNeedsHandling, *parentFk) + } + } + for childFk, required := range cFksRequiredMap { + if required { + cFksNeedsHandling = append(cFksNeedsHandling, *childFk) + } + } + return pFksNeedsHandling, cFksNeedsHandling +} + // ColumnModified checks if any column in the parent table is being updated which has a child foreign key. func ColumnModified(exprs sqlparser.UpdateExprs, getFks func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo)) bool { for _, updateExpr := range exprs { @@ -196,6 +344,10 @@ func ColumnModified(exprs sqlparser.UpdateExprs, getFks func(expr *sqlparser.Upd } } for _, parentFk := range parentFKs { + _, isNull := updateExpr.Expr.(*sqlparser.NullVal) + if isNull { + continue + } if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { return true } @@ -217,7 +369,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp delClone := sqlparser.CloneRefOfDelete(deleteStmt) // Create the delete operator first. - del, err := createDeleteOperator(ctx, deleteStmt, qt, vindexTable, routing) + delOp, err := createDeleteOperator(ctx, deleteStmt, qt, vindexTable, routing) if err != nil { return nil, err } @@ -230,12 +382,12 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp // Unmanaged foreign-key-mode, we don't need to do anything. if ksMode != vschemapb.Keyspace_FK_MANAGED { - return del, nil + return delOp, nil } childFks := vindexTable.ChildFKsNeedsHandling(vindexes.DeleteAction) // If there are no foreign key constraints, then we don't need to do anything. if len(childFks) == 0 { - return del, nil + return delOp, nil } // If the delete statement has a limit, we don't support it yet. if deleteStmt.Limit != nil { @@ -337,7 +489,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp return &FkCascade{ Selection: selection, Children: fkChildren, - Parent: del, + Parent: delOp, }, nil } diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index fa1736e7808..b55203af874 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -132,6 +132,9 @@ func setFks(t *testing.T, vschema *vindexes.VSchema) { // FK from tbl5 referencing tbl8 that is shard scoped of SET-NULL types. _ = vschema.AddForeignKey("sharded_fk_allow", "tbl5", createFkDefinition([]string{"col5"}, "tbl8", []string{"col8"}, sqlparser.SetNull, sqlparser.SetNull)) + // FK from tbl4 referencing tbl9 that is not shard scoped of SET-NULL types. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"col_ref"}, "tbl9", []string{"col9"}, sqlparser.SetNull, sqlparser.SetNull)) + // FK from tbl6 referencing tbl7 that is shard scoped. _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"col6"}, "tbl7", []string{"col7"}, sqlparser.NoAction, sqlparser.NoAction)) _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"t6col6"}, "tbl7", []string{"t7col7"}, sqlparser.NoAction, sqlparser.NoAction)) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 55f744fc7a2..8025fe7499f 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -334,7 +334,7 @@ { "comment": "Update in a table with cross-shard foreign keys disallowed", "query": "update tbl1 set t1col1 = 'foo' where col1 = 1", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": "VT12002: unsupported: cross-shard foreign keys" }, { "comment": "Update in a table with cross-shard foreign keys, column not in update expression - allowed", @@ -361,7 +361,7 @@ { "comment": "Update in a table with column modified not shard-scoped foreign key whereas other column referencing same table is - disallowed", "query": "update tbl7 set t7col7 = 'foo', t7col72 = 42", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": "VT12002: unsupported: cross-shard foreign keys" }, { "comment": "Update in a table with shard-scoped foreign keys with cascade disallowed", @@ -404,5 +404,72 @@ "comment": "delete table with shard scoped foreign key set default - disallowed", "query": "delete from tbl20 where col = 'bar'", "plan": "VT09016: Cannot delete or update a parent row: a foreign key constraint fails" + }, + { + "comment": "Delete table with cross-shard foreign key with set null - should be eventually allowed", + "query": "delete from tbl9 where col9 = 34", + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl9 where col9 = 34", + "Instructions": { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update tbl4 set col_ref = null where (col_ref) in ::fkc_vals", + "Table": "tbl4" + } + ] + } + ], + "Parent": { + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from tbl9 where col9 = 34", + "Table": "tbl9", + "Values": [ + "INT64(34)" + ], + "Vindex": "hash_vin" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select col9 from tbl9 where 1 != 1", + "Query": "select col9 from tbl9 where col9 = 34 lock in share mode", + "Table": "tbl9", + "Values": [ + "INT64(34)" + ], + "Vindex": "hash_vin" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl9" + ] + } } -] \ No newline at end of file +] From 124b458d15696fae9a70496fcb300fced8317615 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Tue, 22 Aug 2023 00:45:19 +0530 Subject: [PATCH 13/31] feat: add fk on update set null planner Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/operators/ast2op.go | 83 ++++-- go/vt/vtgate/planbuilder/plan_test.go | 6 +- .../testdata/foreignkey_cases.json | 268 +++++++++++++++++- go/vt/vtgate/vindexes/foreign_keys.go | 25 ++ 4 files changed, 350 insertions(+), 32 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 767386151e5..84938217f67 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -171,16 +171,6 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars cols = append(cols, len(selectExprs)) selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) } - var childUpdateExprs sqlparser.UpdateExprs - for _, updateExpr := range updClone.Exprs { - colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) - if colIdx >= 0 { - childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName(fk.ParentColumns[colIdx].String()), - Expr: updateExpr.Expr, - }) - } - } switch fk.OnUpdate { case sqlparser.Cascade: @@ -196,6 +186,16 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars Left: valTuple, Right: sqlparser.NewListArg(bvName), } + var childUpdateExprs sqlparser.UpdateExprs + for _, updateExpr := range updClone.Exprs { + colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx >= 0 { + childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(fk.ChildColumns[colIdx].String()), + Expr: updateExpr.Expr, + }) + } + } childUpd := &sqlparser.Update{ Exprs: childUpdateExprs, TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, @@ -210,9 +210,40 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars Cols: cols, Op: childDelOp, }) - case sqlparser.SetNull: - return nil, vterrors.VT12003() + // We now construct the update query for the child table. + // The query looks something like this - `UPDATE SET WHERE IN ()` + bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + compExpr := &sqlparser.ComparisonExpr{ + Operator: sqlparser.InOp, + Left: valTuple, + Right: sqlparser.NewListArg(bvName), + } + var childUpdateExprs sqlparser.UpdateExprs + for _, column := range fk.ChildColumns { + childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(column.String()), + Expr: &sqlparser.NullVal{}, + }) + } + childUpd := &sqlparser.Update{ + Exprs: childUpdateExprs, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + } + childDelOp, err := createOpFromStmt(ctx, childUpd) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, &FkChild{ + BVName: bvName, + Cols: cols, + Op: childDelOp, + }) case sqlparser.SetDefault: return nil, vterrors.VT09016() } @@ -290,46 +321,44 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U } func fkNeedsHandling(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.ParentFKInfo, childFks []vindexes.ChildFKInfo) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { - pFksRequiredMap := map[*vindexes.ParentFKInfo]bool{} - cFksRequiredMap := map[*vindexes.ChildFKInfo]bool{} + pFksRequiredMap := map[string]*vindexes.ParentFKInfo{} + cFksRequiredMap := map[string]*vindexes.ChildFKInfo{} for _, updateExpr := range updateExprs { - for _, childFk := range childFks { + for idx, childFk := range childFks { if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { - cFksRequiredMap[&childFk] = true + cFksRequiredMap[childFk.String()] = &childFks[idx] } } - for _, parentFk := range parentFks { + for idx, parentFk := range parentFks { _, isNull := updateExpr.Expr.(*sqlparser.NullVal) if isNull { continue } if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequiredMap[&parentFk] = true + pFksRequiredMap[parentFk.String()] = &parentFks[idx] } } } - for _, parentFk := range parentFks { + for _, parentFk := range pFksRequiredMap { for _, updateExpr := range updateExprs { _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if isNull { + if !isNull { continue } if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequiredMap[&parentFk] = false + pFksRequiredMap[parentFk.String()] = nil } } } var pFksNeedsHandling []vindexes.ParentFKInfo var cFksNeedsHandling []vindexes.ChildFKInfo - for parentFk, required := range pFksRequiredMap { - if required { + for _, parentFk := range pFksRequiredMap { + if parentFk != nil { pFksNeedsHandling = append(pFksNeedsHandling, *parentFk) } } - for childFk, required := range cFksRequiredMap { - if required { - cFksNeedsHandling = append(cFksNeedsHandling, *childFk) - } + for _, childFk := range cFksRequiredMap { + cFksNeedsHandling = append(cFksNeedsHandling, *childFk) } return pFksNeedsHandling, cFksNeedsHandling } diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index b55203af874..a83442bc907 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -146,15 +146,17 @@ func setFks(t *testing.T, vschema *vindexes.VSchema) { } if vschema.Keyspaces["unsharded_fk_allow"] != nil { // u_tbl2(col2) -> u_tbl1(col1) Cascade. - // u_tbl3(col2) -> u_tbl2(col2) Cascade Null. // u_tbl4(col41) -> u_tbl1(col14) Restrict. + // u_tbl9(col9) -> u_tbl1(col1) Cascade Null. + // u_tbl3(col2) -> u_tbl2(col2) Cascade Null. // u_tbl4(col4) -> u_tbl3(col3) Restrict. // u_tbl6(col6) -> u_tbl5(col5) Restrict. // u_tbl8(col8) -> u_tbl9(col9) Cascade Null. _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl2", createFkDefinition([]string{"col2"}, "u_tbl1", []string{"col1"}, sqlparser.Cascade, sqlparser.Cascade)) - _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl3", createFkDefinition([]string{"col3"}, "u_tbl2", []string{"col2"}, sqlparser.SetNull, sqlparser.SetNull)) + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl9", createFkDefinition([]string{"col9"}, "u_tbl1", []string{"col1"}, sqlparser.SetNull, sqlparser.NoAction)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl4", createFkDefinition([]string{"col41"}, "u_tbl1", []string{"col14"}, sqlparser.NoAction, sqlparser.NoAction)) + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl3", createFkDefinition([]string{"col3"}, "u_tbl2", []string{"col2"}, sqlparser.SetNull, sqlparser.SetNull)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl4", createFkDefinition([]string{"col4"}, "u_tbl3", []string{"col3"}, sqlparser.Restrict, sqlparser.Restrict)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl6", createFkDefinition([]string{"col6"}, "u_tbl5", []string{"col5"}, sqlparser.DefaultAction, sqlparser.DefaultAction)) _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl8", createFkDefinition([]string{"col8"}, "u_tbl9", []string{"col9"}, sqlparser.SetNull, sqlparser.SetNull)) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 8025fe7499f..3c7de15193c 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -307,7 +307,61 @@ { "comment": "update in unsharded table with cascade", "query": "update u_tbl2 set col2 = 'bar' where id = 1", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set col2 = 'bar' where id = 1", + "Instructions": { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals", + "Table": "u_tbl3" + } + ] + } + ], + "Parent": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl2 set col2 = 'bar' where id = 1", + "Table": "u_tbl2" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2 from u_tbl2 where 1 != 1", + "Query": "select col2 from u_tbl2 where id = 1 lock in share mode", + "Table": "u_tbl2" + } + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } }, { "comment": "update in unsharded table with cascade - on non-referenced column", @@ -364,9 +418,63 @@ "plan": "VT12002: unsupported: cross-shard foreign keys" }, { - "comment": "Update in a table with shard-scoped foreign keys with cascade disallowed", + "comment": "Update in a table with shard-scoped foreign keys with cascade", "query": "update tbl5 set t5col5 = 'foo'", - "plan": "VT12002: unsupported: foreign keys management at vitess" + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl5 set t5col5 = 'foo'", + "Instructions": { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update tbl4 set t4col4 = null where (t4col4) in ::fkc_vals", + "Table": "tbl4" + } + ] + } + ], + "Parent": { + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update tbl5 set t5col5 = 'foo'", + "Table": "tbl5" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select t5col5 from tbl5 where 1 != 1", + "Query": "select t5col5 from tbl5 lock in share mode", + "Table": "tbl5" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl5" + ] + } }, { "comment": "Insertion in a table with 2 foreign keys constraint with same table on different columns - both are not shard scoped - disallowed", @@ -471,5 +579,159 @@ "sharded_fk_allow.tbl9" ] } + }, + { + "comment": "update table with same column having reference to different tables, one with on update cascade other with on update set null - child table have further reference", + "query": "update u_tbl1 set col1 = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl1 set col1 = 'foo'", + "Instructions": { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals1", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1", + "Table": "u_tbl3" + } + ] + } + ], + "Parent": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", + "Table": "u_tbl2" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2 from u_tbl2 where 1 != 1", + "Query": "select col2 from u_tbl2 where (col2) in ::fkc_vals", + "Table": "u_tbl2" + } + } + ] + }, + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals2", + "Cols": [ + 1 + ], + "Inputs": [ + { + "OperatorType": "FkCascade", + "Children": [ + { + "OperatorType": "FkCascadeChild", + "BvName": "fkc_vals3", + "Cols": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl8 set col8 = null where (col8) in ::fkc_vals3", + "Table": "u_tbl8" + } + ] + } + ], + "Parent": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl9 set col9 = null where (col9) in ::fkc_vals2", + "Table": "u_tbl9" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col9 from u_tbl9 where 1 != 1", + "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2", + "Table": "u_tbl9" + } + } + ] + } + ], + "Parent": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl1 set col1 = 'foo'", + "Table": "u_tbl1" + }, + "Selection": { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col1, col1 from u_tbl1 where 1 != 1", + "Query": "select col1, col1 from u_tbl1 lock in share mode", + "Table": "u_tbl1" + } + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl1", + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3", + "unsharded_fk_allow.u_tbl8", + "unsharded_fk_allow.u_tbl9" + ] + } } ] diff --git a/go/vt/vtgate/vindexes/foreign_keys.go b/go/vt/vtgate/vindexes/foreign_keys.go index 5029c1f7fe1..2d111d7e18e 100644 --- a/go/vt/vtgate/vindexes/foreign_keys.go +++ b/go/vt/vtgate/vindexes/foreign_keys.go @@ -19,6 +19,7 @@ package vindexes import ( "encoding/json" "fmt" + "strings" "vitess.io/vitess/go/vt/sqlparser" ) @@ -43,6 +44,18 @@ func (fk *ParentFKInfo) MarshalJSON() ([]byte, error) { }) } +func (fk *ParentFKInfo) String() string { + var str strings.Builder + str.WriteString(fk.Table.Name.String()) + for _, column := range fk.ChildColumns { + str.WriteString(column.String()) + } + for _, column := range fk.ParentColumns { + str.WriteString(column.String()) + } + return str.String() +} + // NewParentFkInfo creates a new ParentFKInfo. func NewParentFkInfo(parentTbl *Table, fkDef *sqlparser.ForeignKeyDefinition) ParentFKInfo { return ParentFKInfo{ @@ -75,6 +88,18 @@ func (fk *ChildFKInfo) MarshalJSON() ([]byte, error) { }) } +func (fk *ChildFKInfo) String() string { + var str strings.Builder + str.WriteString(fk.Table.Name.String()) + for _, column := range fk.ChildColumns { + str.WriteString(column.String()) + } + for _, column := range fk.ParentColumns { + str.WriteString(column.String()) + } + return str.String() +} + // NewChildFkInfo creates a new ChildFKInfo. func NewChildFkInfo(childTbl *Table, fkDef *sqlparser.ForeignKeyDefinition) ChildFKInfo { return ChildFKInfo{ From 8d255469c40df7dac766b5cc7bc99673c718bdf1 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 22 Aug 2023 09:20:40 +0530 Subject: [PATCH 14/31] feat: fix e2e tests Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index be79d3790ae..047590ca7ac 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -121,9 +121,9 @@ func TestUpdateWithFK(t *testing.T) { qr := utils.Exec(t, conn, `update t4 set col = 20 where id = 1`) assert.EqualValues(t, 1, qr.RowsAffected) - // child table have cascade which is cross shard. Query will fail at vtgate. - _, err = utils.ExecAllowError(t, conn, `update t2 set col = 125 where id = 100`) - assert.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)") + // child table have cascade which is cross shard. + qr = utils.Exec(t, conn, `update t2 set col = 126 where id = 100`) + assert.EqualValues(t, 1, qr.RowsAffected) // updating column which does not have foreign key constraint, so query will succeed. _ = utils.Exec(t, conn, `update t2 set mycol = 'baz' where id = 100`) From 22797fd7a636e163900def9fec96e9e097ca0c5d Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 22 Aug 2023 10:06:16 +0530 Subject: [PATCH 15/31] refactor: perf improvement Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 84938217f67..f60c66d67ce 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -201,14 +201,14 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, } - childDelOp, err := createOpFromStmt(ctx, childUpd) + childUpdOp, err := createOpFromStmt(ctx, childUpd) if err != nil { return nil, err } fkChildren = append(fkChildren, &FkChild{ BVName: bvName, Cols: cols, - Op: childDelOp, + Op: childUpdOp, }) case sqlparser.SetNull: // We now construct the update query for the child table. @@ -235,14 +235,14 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, } - childDelOp, err := createOpFromStmt(ctx, childUpd) + childUpdOp, err := createOpFromStmt(ctx, childUpd) if err != nil { return nil, err } fkChildren = append(fkChildren, &FkChild{ BVName: bvName, Cols: cols, - Op: childDelOp, + Op: childUpdOp, }) case sqlparser.SetDefault: return nil, vterrors.VT09016() @@ -329,11 +329,11 @@ func fkNeedsHandling(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.Par cFksRequiredMap[childFk.String()] = &childFks[idx] } } + _, isNull := updateExpr.Expr.(*sqlparser.NullVal) + if isNull { + continue + } for idx, parentFk := range parentFks { - _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if isNull { - continue - } if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { pFksRequiredMap[parentFk.String()] = &parentFks[idx] } @@ -372,11 +372,11 @@ func ColumnModified(exprs sqlparser.UpdateExprs, getFks func(expr *sqlparser.Upd return true } } + _, isNull := updateExpr.Expr.(*sqlparser.NullVal) + if isNull { + continue + } for _, parentFk := range parentFKs { - _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if isNull { - continue - } if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { return true } From 91ec17f76971b3fc544d04799757e895f89aa35c Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 22 Aug 2023 16:39:51 +0530 Subject: [PATCH 16/31] refactor: add comments to various places Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 16 ++++++++++++++-- go/vt/vtgate/planbuilder/operators/fk_cascade.go | 7 +++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index f60c66d67ce..6dcf63c05c8 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -141,7 +141,7 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return updOp, nil } - parentFKs, childFks = fkNeedsHandling(updStmt.Exprs, parentFKs, childFks) + parentFKs, childFks = fkNeedsHandlingForUpdates(updStmt.Exprs, parentFKs, childFks) if len(parentFKs) > 0 { return nil, vterrors.VT12003() } @@ -320,25 +320,36 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U return subq, nil } -func fkNeedsHandling(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.ParentFKInfo, childFks []vindexes.ChildFKInfo) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { +// fkNeedsHandlingForUpdates returns what all foreign keys need handling in Vitess for the given set of update expressions. +// Here we return all the parent and child foreign key constraints that could require any handling. +func fkNeedsHandlingForUpdates(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.ParentFKInfo, childFks []vindexes.ChildFKInfo) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { pFksRequiredMap := map[string]*vindexes.ParentFKInfo{} cFksRequiredMap := map[string]*vindexes.ChildFKInfo{} + // Go over all the update expressions for _, updateExpr := range updateExprs { + // Any foreign key to a child table for a column that has been updated + // will require the cascade operations to happen, so we include all such foreign keys. for idx, childFk := range childFks { if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { cFksRequiredMap[childFk.String()] = &childFks[idx] } } + // If we are setting a column to NULL, then we don't need to verify the existance of an + // equivalent row in the parent table, even if this column was part of a foreign key to a parent table. _, isNull := updateExpr.Expr.(*sqlparser.NullVal) if isNull { continue } + // We add all the possible parent foreign key constraints that need verification that an equivalent row + // exists, given that this column has changed. for idx, parentFk := range parentFks { if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { pFksRequiredMap[parentFk.String()] = &parentFks[idx] } } } + // For the parent foreign keys, if any of the columns part of the fk is set to NULL, + // then, we don't care for the existance of an equivalent row in the parent table. for _, parentFk := range pFksRequiredMap { for _, updateExpr := range updateExprs { _, isNull := updateExpr.Expr.(*sqlparser.NullVal) @@ -350,6 +361,7 @@ func fkNeedsHandling(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.Par } } } + // Convert the maps to lists and return them. var pFksNeedsHandling []vindexes.ParentFKInfo var cFksNeedsHandling []vindexes.ChildFKInfo for _, parentFk := range pFksRequiredMap { diff --git a/go/vt/vtgate/planbuilder/operators/fk_cascade.go b/go/vt/vtgate/planbuilder/operators/fk_cascade.go index 8618dcf8261..6e6e781bb99 100644 --- a/go/vt/vtgate/planbuilder/operators/fk_cascade.go +++ b/go/vt/vtgate/planbuilder/operators/fk_cascade.go @@ -20,6 +20,9 @@ import ( "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" ) +// FkCascade is used to represent a foreign key cascade operation +// as an operator. This operator is created for DML queries that require +// cascades (for example, ON DELETE CASCADE). type FkCascade struct { Selection ops.Operator Children []*FkChild @@ -31,6 +34,7 @@ type FkCascade struct { var _ ops.Operator = (*FkCascade)(nil) +// Inputs implements the Operator interface func (fkc *FkCascade) Inputs() []ops.Operator { var inputs []ops.Operator inputs = append(inputs, fkc.Parent) @@ -41,6 +45,7 @@ func (fkc *FkCascade) Inputs() []ops.Operator { return inputs } +// SetInputs implements the Operator interface func (fkc *FkCascade) SetInputs(operators []ops.Operator) { if len(operators) < 2 { panic("incorrect count of inputs for FkCascade") @@ -73,10 +78,12 @@ func (fkc *FkCascade) Clone(inputs []ops.Operator) ops.Operator { return newFkc } +// GetOrdering implements the Operator interface func (fkc *FkCascade) GetOrdering() ([]ops.OrderBy, error) { return nil, nil } +// ShortDescription implements the Operator interface func (fkc *FkCascade) ShortDescription() string { return "FkCascade" } From effb8d48358fe8bb37d0339e0b3386bf6516b21f Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 22 Aug 2023 17:20:56 +0530 Subject: [PATCH 17/31] feat: add more e2e tests Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index 047590ca7ac..a3f3ccb1a9d 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -106,7 +106,7 @@ func TestUpdateWithFK(t *testing.T) { // insert some data. utils.Exec(t, conn, `insert into t1(id, col) values (100, 123),(10, 12),(1, 13),(1000, 1234)`) utils.Exec(t, conn, `insert into t2(id, col, mycol) values (100, 125, 'foo'), (1, 132, 'bar')`) - utils.Exec(t, conn, `insert into t4(id, col, t2_mycol) values (1, 321, 'bar')`) + utils.Exec(t, conn, `insert into t4(id, col, t2_col, t2_mycol) values (1, 321, 132, 'bar')`) utils.Exec(t, conn, `insert into t5(pk, sk, col1) values (1, 1, 1),(2, 1, 1),(3, 1, 10),(4, 1, 20),(5, 1, 30),(6, 1, 40)`) utils.Exec(t, conn, `insert into t6(pk, sk, col1) values (10, 1, 1), (20, 1, 20)`) @@ -122,8 +122,10 @@ func TestUpdateWithFK(t *testing.T) { assert.EqualValues(t, 1, qr.RowsAffected) // child table have cascade which is cross shard. - qr = utils.Exec(t, conn, `update t2 set col = 126 where id = 100`) + qr = utils.Exec(t, conn, `update t2 set col = 126 where id = 1`) assert.EqualValues(t, 1, qr.RowsAffected) + // Check that the child table is also updated. + utils.AssertMatches(t, conn, `select * from t4`, `[[INT64(1) INT64(321) INT64(126) VARCHAR("bar")]]`) // updating column which does not have foreign key constraint, so query will succeed. _ = utils.Exec(t, conn, `update t2 set mycol = 'baz' where id = 100`) @@ -132,4 +134,23 @@ func TestUpdateWithFK(t *testing.T) { // child table have restrict in shard scoped and value exists in parent table. _ = utils.Exec(t, conn, `update t6 set col1 = 40 where pk = 20`) assert.EqualValues(t, 1, qr.RowsAffected) + + // Unsharded keyspace tests + utils.Exec(t, conn, `use uks`) + // insert some data. + utils.Exec(t, conn, `insert into u_t1(id, col1) values (100, 123), (10, 12), (1, 13), (1000, 1234)`) + utils.Exec(t, conn, `insert into u_t2(id, col2) values (342, 123), (19, 1234)`) + + // Update u_t1 which has a foreign key constraint to t2 with SET NULL type. + qr = utils.Exec(t, conn, `update u_t1 set col1 = 2 where id = 100`) + assert.EqualValues(t, 1, qr.RowsAffected) + // Verify the result in u_t2 as well + utils.AssertMatches(t, conn, `select * from u_t2`, `[[INT64(342) NULL] [INT64(19) INT64(1234)]]`) + + // Update u_t1 which has a foreign key constraint to t2 with SET NULL type. + // This update however doesn't change the table. + qr = utils.Exec(t, conn, `update u_t1 set col1 = 1234 where id = 1000`) + assert.EqualValues(t, 0, qr.RowsAffected) + // Verify the result in u_t2 as well + utils.AssertMatches(t, conn, `select * from u_t2`, `[[INT64(342) NULL] [INT64(19) INT64(1234)]]`) } From 20ac58ab4d76848156d9ac6b57f1d7613693e2fa Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 23 Aug 2023 11:59:34 +0530 Subject: [PATCH 18/31] feat: fix set null implementation for update Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 47 +++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 6dcf63c05c8..2cd413f94ef 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -172,6 +172,19 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) } + // We only support literals in update queries, which trigger foreign key updates. + for _, expr := range updStmt.Exprs { + colIdx := fk.ParentColumns.FindColumn(expr.Name.Name) + if colIdx == 0 { + continue + } + switch expr.Expr.(type) { + case *sqlparser.Argument, *sqlparser.NullVal, sqlparser.BoolVal: + default: + return nil, vterrors.VT12001("foreign keys management at vitess with non-literal values") + } + } + switch fk.OnUpdate { case sqlparser.Cascade: // We now construct the update query for the child table. @@ -212,7 +225,11 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars }) case sqlparser.SetNull: // We now construct the update query for the child table. - // The query looks something like this - `UPDATE SET WHERE IN ()` + // The query looks something like this - + // `UPDATE SET + // WHERE IN () + // [AND NOT IN ()]` + // If all the columns of the foreign key in the parent don't actually end up getting updated, then we shouldn't be setting the child columns to NULL. bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") var valTuple sqlparser.ValTuple for _, column := range fk.ChildColumns { @@ -230,10 +247,36 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars Expr: &sqlparser.NullVal{}, }) } + + var secondValTuple sqlparser.ValTuple + var somethingIsNull bool + for _, updateExpr := range updClone.Exprs { + colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx >= 0 { + _, isNull := updateExpr.Expr.(*sqlparser.NullVal) + if isNull { + somethingIsNull = true + break + } else { + secondValTuple = append(secondValTuple, updateExpr.Expr) + } + } + } + var whereExpr sqlparser.Expr = compExpr + if !somethingIsNull { + whereExpr = &sqlparser.AndExpr{ + Left: compExpr, + Right: &sqlparser.ComparisonExpr{ + Operator: sqlparser.NotInOp, + Left: valTuple, + Right: secondValTuple, + }, + } + } childUpd := &sqlparser.Update{ Exprs: childUpdateExprs, TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: whereExpr}, } childUpdOp, err := createOpFromStmt(ctx, childUpd) if err != nil { From dac40bfc567ab9e596e1bdc0e24d876954b31a19 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 23 Aug 2023 13:25:41 +0530 Subject: [PATCH 19/31] feat: augment the e2e tests Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 28 ++++++++++--------- .../vtgate/foreignkey/unsharded_schema.sql | 8 ++++++ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index a3f3ccb1a9d..147bd6f4d83 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -121,12 +121,6 @@ func TestUpdateWithFK(t *testing.T) { qr := utils.Exec(t, conn, `update t4 set col = 20 where id = 1`) assert.EqualValues(t, 1, qr.RowsAffected) - // child table have cascade which is cross shard. - qr = utils.Exec(t, conn, `update t2 set col = 126 where id = 1`) - assert.EqualValues(t, 1, qr.RowsAffected) - // Check that the child table is also updated. - utils.AssertMatches(t, conn, `select * from t4`, `[[INT64(1) INT64(321) INT64(126) VARCHAR("bar")]]`) - // updating column which does not have foreign key constraint, so query will succeed. _ = utils.Exec(t, conn, `update t2 set mycol = 'baz' where id = 100`) assert.EqualValues(t, 1, qr.RowsAffected) @@ -140,17 +134,25 @@ func TestUpdateWithFK(t *testing.T) { // insert some data. utils.Exec(t, conn, `insert into u_t1(id, col1) values (100, 123), (10, 12), (1, 13), (1000, 1234)`) utils.Exec(t, conn, `insert into u_t2(id, col2) values (342, 123), (19, 1234)`) + utils.Exec(t, conn, `insert into u_t3(id, col3) values (32, 123), (1, 12)`) - // Update u_t1 which has a foreign key constraint to t2 with SET NULL type. - qr = utils.Exec(t, conn, `update u_t1 set col1 = 2 where id = 100`) + t.Run("Cascade update with a new value", func(t *testing.T) { + t.Skip("This doesn't work right now. We are able to only cascade updates for which the data already exists in the parent table") + _ = utils.Exec(t, conn, `update u_t1 set col1 = 2 where id = 100`) + }) + + // Update u_t1 which has a foreign key constraint to u_t2 with SET NULL type, and to u_t3 with CASCADE type. + qr = utils.Exec(t, conn, `update u_t1 set col1 = 13 where id = 100`) assert.EqualValues(t, 1, qr.RowsAffected) - // Verify the result in u_t2 as well - utils.AssertMatches(t, conn, `select * from u_t2`, `[[INT64(342) NULL] [INT64(19) INT64(1234)]]`) + // Verify the result in u_t2 and u_t3 as well. + utils.AssertMatches(t, conn, `select * from u_t2 order by id`, `[[INT64(19) INT64(1234)] [INT64(342) NULL]]`) + utils.AssertMatches(t, conn, `select * from u_t3 order by id`, `[[INT64(1) INT64(12)] [INT64(32) INT64(13)]]`) - // Update u_t1 which has a foreign key constraint to t2 with SET NULL type. + // Update u_t1 which has a foreign key constraint to u_t2 with SET NULL type, and to u_t3 with CASCADE type. // This update however doesn't change the table. qr = utils.Exec(t, conn, `update u_t1 set col1 = 1234 where id = 1000`) assert.EqualValues(t, 0, qr.RowsAffected) - // Verify the result in u_t2 as well - utils.AssertMatches(t, conn, `select * from u_t2`, `[[INT64(342) NULL] [INT64(19) INT64(1234)]]`) + // Verify the result in u_t2 and u_t3 as well. + utils.AssertMatches(t, conn, `select * from u_t2 order by id`, `[[INT64(19) INT64(1234)] [INT64(342) NULL]]`) + utils.AssertMatches(t, conn, `select * from u_t3 order by id`, `[[INT64(1) INT64(12)] [INT64(32) INT64(13)]]`) } diff --git a/go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql b/go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql index 94c5bbcbdfd..9c21510046b 100644 --- a/go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql +++ b/go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql @@ -13,3 +13,11 @@ create table u_t2 primary key (id), foreign key (col2) references u_t1 (col1) on delete set null on update set null ) Engine = InnoDB; + +create table u_t3 +( + id bigint, + col3 bigint, + primary key (id), + foreign key (col3) references u_t1 (col1) on delete cascade on update cascade +) Engine = InnoDB; \ No newline at end of file From 20eb4cfb19dbb267208240cc508f40a2fa96bedf Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 23 Aug 2023 14:36:46 +0530 Subject: [PATCH 20/31] test: update test expectations Signed-off-by: Manan Gupta --- .../vtgate/planbuilder/testdata/foreignkey_cases.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 3c7de15193c..16426dc97c5 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -328,7 +328,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals and (col3) not in ('bar')", "Table": "u_tbl3" } ] @@ -441,7 +441,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update tbl4 set t4col4 = null where (t4col4) in ::fkc_vals", + "Query": "update tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (t4col4) not in ('foo')", "Table": "tbl4" } ] @@ -614,7 +614,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (col3) not in ('foo')", "Table": "u_tbl3" } ] @@ -684,7 +684,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_tbl9 set col9 = null where (col9) in ::fkc_vals2", + "Query": "update u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (col9) not in ('foo')", "Table": "u_tbl9" }, "Selection": { @@ -695,7 +695,7 @@ "Sharded": false }, "FieldQuery": "select col9 from u_tbl9 where 1 != 1", - "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2", + "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (col9) not in ('foo')", "Table": "u_tbl9" } } From d1b97566216a8b9d20da941a17be7cdccd2b1ec5 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 23 Aug 2023 15:03:44 +0530 Subject: [PATCH 21/31] test: add tests to increase test coverage Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 4 ++-- go/vt/vtgate/planbuilder/plan_test.go | 2 +- .../testdata/foreignkey_cases.json | 20 +++++++++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 2cd413f94ef..363de74b233 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -175,11 +175,11 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars // We only support literals in update queries, which trigger foreign key updates. for _, expr := range updStmt.Exprs { colIdx := fk.ParentColumns.FindColumn(expr.Name.Name) - if colIdx == 0 { + if colIdx < 0 { continue } switch expr.Expr.(type) { - case *sqlparser.Argument, *sqlparser.NullVal, sqlparser.BoolVal: + case *sqlparser.Argument, *sqlparser.NullVal, sqlparser.BoolVal, *sqlparser.Literal: default: return nil, vterrors.VT12001("foreign keys management at vitess with non-literal values") } diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index a83442bc907..84400ed6042 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -141,7 +141,7 @@ func setFks(t *testing.T, vschema *vindexes.VSchema) { _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"t6col62"}, "tbl7", []string{"t7col72"}, sqlparser.NoAction, sqlparser.NoAction)) // FK from tblrefDef referencing tbl20 that is shard scoped of SET-Default types. - _ = vschema.AddForeignKey("sharded_fk_allow", "tblrefDef", createFkDefinition([]string{"ref"}, "tbl20", []string{"col"}, sqlparser.SetDefault, sqlparser.SetDefault)) + _ = vschema.AddForeignKey("sharded_fk_allow", "tblrefDef", createFkDefinition([]string{"ref"}, "tbl20", []string{"col2"}, sqlparser.SetDefault, sqlparser.SetDefault)) } if vschema.Keyspaces["unsharded_fk_allow"] != nil { diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 16426dc97c5..35e591ef0a6 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -733,5 +733,25 @@ "unsharded_fk_allow.u_tbl9" ] } + }, + { + "comment": "update in a table with limit - disallowed", + "query": "update u_tbl2 set col2 = 'bar' limit 2", + "plan": "VT12001: unsupported: foreign keys management at vitess with limit" + }, + { + "comment": "update in a table with non-literal value - disallowed", + "query": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1", + "plan": "VT12001: unsupported: foreign keys management at vitess with non-literal values" + }, + { + "comment": "update in a table with a child table having SET DEFAULT constraint - disallowed", + "query": "update tbl20 set col2 = 'bar'", + "plan": "VT09016: Cannot delete or update a parent row: a foreign key constraint fails" + }, + { + "comment": "delete in a table with limit - disallowed", + "query": "delete from u_tbl2 limit 2", + "plan": "VT12001: unsupported: foreign keys management at vitess with limit" } ] From 4182593a71e708158bb36e9212b35e2c771969df Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 23 Aug 2023 15:41:29 +0530 Subject: [PATCH 22/31] test: add testing for fkNeedsHandlingForUpdates Signed-off-by: Manan Gupta --- .../planbuilder/operators/ast2op_test.go | 189 ++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 go/vt/vtgate/planbuilder/operators/ast2op_test.go diff --git a/go/vt/vtgate/planbuilder/operators/ast2op_test.go b/go/vt/vtgate/planbuilder/operators/ast2op_test.go new file mode 100644 index 00000000000..4e60ac28c59 --- /dev/null +++ b/go/vt/vtgate/planbuilder/operators/ast2op_test.go @@ -0,0 +1,189 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operators + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/vindexes" +) + +// Test_fkNeedsHandlingForUpdates tests the functionality of the function fkNeedsHandlingForUpdates. +// It verifies the different cases in which foreign key handling is required on vtgate level. +func Test_fkNeedsHandlingForUpdates(t *testing.T) { + t1 := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS("t1"), + } + + tests := []struct { + name string + updateExprs sqlparser.UpdateExprs + parentFks []vindexes.ParentFKInfo + childFks []vindexes.ChildFKInfo + parentFKsWanted []bool + childFKsWanted []bool + }{ + { + name: "No Fks filtered", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName("a"), + Expr: sqlparser.NewIntLiteral("1"), + }, + }, + childFks: []vindexes.ChildFKInfo{ + { + Table: t1, + ParentColumns: sqlparser.MakeColumns("a", "b", "c"), + }, + }, + parentFks: []vindexes.ParentFKInfo{ + { + Table: t1, + ChildColumns: sqlparser.MakeColumns("a", "b", "c"), + }, + }, + parentFKsWanted: []bool{true}, + childFKsWanted: []bool{true}, + }, { + name: "Child Fks filtering", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName("a"), + Expr: sqlparser.NewIntLiteral("1"), + }, + }, + childFks: []vindexes.ChildFKInfo{ + { + Table: t1, + ParentColumns: sqlparser.MakeColumns("b", "a", "c"), + }, { + Table: t1, + ParentColumns: sqlparser.MakeColumns("d", "c"), + }, + }, + parentFks: []vindexes.ParentFKInfo{ + { + Table: t1, + ChildColumns: sqlparser.MakeColumns("a", "b", "c"), + }, + }, + parentFKsWanted: []bool{true}, + childFKsWanted: []bool{true, false}, + }, { + name: "Parent Fks filtered based on columns", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName("a"), + Expr: sqlparser.NewIntLiteral("1"), + }, + }, + childFks: []vindexes.ChildFKInfo{ + { + Table: t1, + ParentColumns: sqlparser.MakeColumns("a", "b", "c"), + }, + }, + parentFks: []vindexes.ParentFKInfo{ + { + Table: t1, + ChildColumns: sqlparser.MakeColumns("b", "a", "c"), + }, { + Table: t1, + ChildColumns: sqlparser.MakeColumns("d", "b"), + }, + }, + parentFKsWanted: []bool{true, false}, + childFKsWanted: []bool{true}, + }, { + name: "Parent Fks filtered because all null values", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName("a"), + Expr: &sqlparser.NullVal{}, + }, + }, + childFks: []vindexes.ChildFKInfo{ + { + Table: t1, + ParentColumns: sqlparser.MakeColumns("a", "b", "c"), + }, + }, + parentFks: []vindexes.ParentFKInfo{ + { + Table: t1, + ChildColumns: sqlparser.MakeColumns("b", "a", "c"), + }, { + Table: t1, + ChildColumns: sqlparser.MakeColumns("a", "b"), + }, + }, + parentFKsWanted: []bool{false, false}, + childFKsWanted: []bool{true}, + }, { + name: "Parent Fks filtered because some column has null values", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName("a"), + Expr: sqlparser.NewIntLiteral("1"), + }, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName("c"), + Expr: &sqlparser.NullVal{}, + }, + }, + childFks: []vindexes.ChildFKInfo{ + { + Table: t1, + ParentColumns: sqlparser.MakeColumns("a", "b", "c"), + }, + }, + parentFks: []vindexes.ParentFKInfo{ + { + Table: t1, + ChildColumns: sqlparser.MakeColumns("b", "a", "c"), + }, { + Table: t1, + ChildColumns: sqlparser.MakeColumns("a", "b"), + }, + }, + parentFKsWanted: []bool{false, true}, + childFKsWanted: []bool{true}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parentFksGot, childFksGot := fkNeedsHandlingForUpdates(tt.updateExprs, tt.parentFks, tt.childFks) + var pFks []vindexes.ParentFKInfo + for idx, b := range tt.parentFKsWanted { + if b { + pFks = append(pFks, tt.parentFks[idx]) + } + } + var cFks []vindexes.ChildFKInfo + for idx, b := range tt.childFKsWanted { + if b { + cFks = append(cFks, tt.childFks[idx]) + } + } + require.EqualValues(t, pFks, parentFksGot) + require.EqualValues(t, cFks, childFksGot) + }) + } +} From c70dc24901a3b1226c2683f35e6d824fd59df56a Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 23 Aug 2023 15:48:58 +0530 Subject: [PATCH 23/31] refactor: improve implementation of fkNeedsHandlingForUpdates to not use a map indexed by strings Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 26 +++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 363de74b233..b0e793b51aa 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -366,15 +366,15 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U // fkNeedsHandlingForUpdates returns what all foreign keys need handling in Vitess for the given set of update expressions. // Here we return all the parent and child foreign key constraints that could require any handling. func fkNeedsHandlingForUpdates(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.ParentFKInfo, childFks []vindexes.ChildFKInfo) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { - pFksRequiredMap := map[string]*vindexes.ParentFKInfo{} - cFksRequiredMap := map[string]*vindexes.ChildFKInfo{} + pFksRequired := make([]bool, len(parentFks)) + cFksRequired := make([]bool, len(childFks)) // Go over all the update expressions for _, updateExpr := range updateExprs { // Any foreign key to a child table for a column that has been updated // will require the cascade operations to happen, so we include all such foreign keys. for idx, childFk := range childFks { if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { - cFksRequiredMap[childFk.String()] = &childFks[idx] + cFksRequired[idx] = true } } // If we are setting a column to NULL, then we don't need to verify the existance of an @@ -387,33 +387,35 @@ func fkNeedsHandlingForUpdates(updateExprs sqlparser.UpdateExprs, parentFks []vi // exists, given that this column has changed. for idx, parentFk := range parentFks { if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequiredMap[parentFk.String()] = &parentFks[idx] + pFksRequired[idx] = true } } } // For the parent foreign keys, if any of the columns part of the fk is set to NULL, // then, we don't care for the existance of an equivalent row in the parent table. - for _, parentFk := range pFksRequiredMap { + for idx, parentFk := range parentFks { for _, updateExpr := range updateExprs { _, isNull := updateExpr.Expr.(*sqlparser.NullVal) if !isNull { continue } if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequiredMap[parentFk.String()] = nil + pFksRequired[idx] = false } } } - // Convert the maps to lists and return them. + // Get the filtered lists and return them. var pFksNeedsHandling []vindexes.ParentFKInfo var cFksNeedsHandling []vindexes.ChildFKInfo - for _, parentFk := range pFksRequiredMap { - if parentFk != nil { - pFksNeedsHandling = append(pFksNeedsHandling, *parentFk) + for idx, parentFk := range parentFks { + if pFksRequired[idx] { + pFksNeedsHandling = append(pFksNeedsHandling, parentFk) } } - for _, childFk := range cFksRequiredMap { - cFksNeedsHandling = append(cFksNeedsHandling, *childFk) + for idx, childFk := range childFks { + if cFksRequired[idx] { + cFksNeedsHandling = append(cFksNeedsHandling, childFk) + } } return pFksNeedsHandling, cFksNeedsHandling } From 7e589eda7ee6f8da7b4b83939edca84e07901acf Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 23 Aug 2023 16:16:43 +0530 Subject: [PATCH 24/31] refactor: refactor createOperatorFromUpdate code to extract out common code Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/ast2op.go | 128 +++++++++---------- 1 file changed, 62 insertions(+), 66 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index b0e793b51aa..a3f23721b51 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -185,21 +185,42 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars } } + // We now construct the update query for the child table - + // For CASCADE type constraint, the query looks like this - + // `UPDATE SET WHERE IN ()` + // For SET NULL type constraint, the query looks like this - + // `UPDATE SET + // WHERE IN () + // [AND NOT IN ()]` + // If all the columns of the foreign key in the parent don't actually end up getting updated, then we shouldn't be setting the child columns to NULL. + + // Some operations are common for both the cases. + // 1. building the bind variable name for the output from the SELECT query. + bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") + // 2. ValTuple having the column names of the foreign key constraint in the child table. + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + // 3. Use the above 2 to create the comparison expression we use in the where condition of the UPDATE. + compExpr := &sqlparser.ComparisonExpr{ + Operator: sqlparser.InOp, + Left: valTuple, + Right: sqlparser.NewListArg(bvName), + } + + // For both the cases, 2 things are different. The where condition and the update expressions. + // For CASCADE type constraint, the where condition is the same as the comparison expression above. + // For SET NULL type constraint, the where condition is the same as the comparison expression above, with an additional condition (that condition is optional). + + // We create the variables for these 2 things, and populate them in the switch case below. + var childWhereExpr sqlparser.Expr = compExpr + var childUpdateExprs sqlparser.UpdateExprs + switch fk.OnUpdate { case sqlparser.Cascade: - // We now construct the update query for the child table. - // The query looks something like this - `UPDATE SET WHERE IN ()` - bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") - var valTuple sqlparser.ValTuple - for _, column := range fk.ChildColumns { - valTuple = append(valTuple, sqlparser.NewColName(column.String())) - } - compExpr := &sqlparser.ComparisonExpr{ - Operator: sqlparser.InOp, - Left: valTuple, - Right: sqlparser.NewListArg(bvName), - } - var childUpdateExprs sqlparser.UpdateExprs + // For the CASCADE type constraint, the update expressions are the same as the update expressions in the parent update query. + // We need to replace the column names in the update expressions with the corresponding child column names. for _, updateExpr := range updClone.Exprs { colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) if colIdx >= 0 { @@ -209,38 +230,8 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars }) } } - childUpd := &sqlparser.Update{ - Exprs: childUpdateExprs, - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, - } - childUpdOp, err := createOpFromStmt(ctx, childUpd) - if err != nil { - return nil, err - } - fkChildren = append(fkChildren, &FkChild{ - BVName: bvName, - Cols: cols, - Op: childUpdOp, - }) case sqlparser.SetNull: - // We now construct the update query for the child table. - // The query looks something like this - - // `UPDATE SET - // WHERE IN () - // [AND NOT IN ()]` - // If all the columns of the foreign key in the parent don't actually end up getting updated, then we shouldn't be setting the child columns to NULL. - bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") - var valTuple sqlparser.ValTuple - for _, column := range fk.ChildColumns { - valTuple = append(valTuple, sqlparser.NewColName(column.String())) - } - compExpr := &sqlparser.ComparisonExpr{ - Operator: sqlparser.InOp, - Left: valTuple, - Right: sqlparser.NewListArg(bvName), - } - var childUpdateExprs sqlparser.UpdateExprs + // For the SET NULL type constraint, we need to set all the child columns to NULL. for _, column := range fk.ChildColumns { childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ Name: sqlparser.NewColName(column.String()), @@ -248,48 +239,53 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars }) } - var secondValTuple sqlparser.ValTuple - var somethingIsNull bool + // There is a special case in this constraint though. + // If a user updates the parent columns with values that are exactly what it already has, such that none of the parent columns actually get updated, + // then we shouldn't be setting the children columns to NULL. + // We need to add an additional condition to the where clause to handle this case. + // The additional condition looks like [AND NOT IN ()]. + // If any of the parent columns is being set to NULL, then we don't need this condition. + var updateValues sqlparser.ValTuple + var somethingIsSetNull bool for _, updateExpr := range updClone.Exprs { colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) if colIdx >= 0 { _, isNull := updateExpr.Expr.(*sqlparser.NullVal) if isNull { - somethingIsNull = true + somethingIsSetNull = true break } else { - secondValTuple = append(secondValTuple, updateExpr.Expr) + updateValues = append(updateValues, updateExpr.Expr) } } } - var whereExpr sqlparser.Expr = compExpr - if !somethingIsNull { - whereExpr = &sqlparser.AndExpr{ + if !somethingIsSetNull { + childWhereExpr = &sqlparser.AndExpr{ Left: compExpr, Right: &sqlparser.ComparisonExpr{ Operator: sqlparser.NotInOp, Left: valTuple, - Right: secondValTuple, + Right: updateValues, }, } } - childUpd := &sqlparser.Update{ - Exprs: childUpdateExprs, - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: whereExpr}, - } - childUpdOp, err := createOpFromStmt(ctx, childUpd) - if err != nil { - return nil, err - } - fkChildren = append(fkChildren, &FkChild{ - BVName: bvName, - Cols: cols, - Op: childUpdOp, - }) case sqlparser.SetDefault: return nil, vterrors.VT09016() } + childUpd := &sqlparser.Update{ + Exprs: childUpdateExprs, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: childWhereExpr}, + } + childUpdOp, err := createOpFromStmt(ctx, childUpd) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, &FkChild{ + BVName: bvName, + Cols: cols, + Op: childUpdOp, + }) } // We now create the selection operator to select the parent columns for the foreign key constraints. From 21f7aa5ae3f4c834421305e5e623df2200853a87 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Fri, 25 Aug 2023 09:33:59 +0530 Subject: [PATCH 25/31] feat: fix vschema updation with foreign keys and add tests Signed-off-by: Manan Gupta --- go/vt/vtgate/vschema_manager.go | 11 +- go/vt/vtgate/vschema_manager_test.go | 154 +++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/vschema_manager.go b/go/vt/vtgate/vschema_manager.go index 80c3209405f..6af61b154fe 100644 --- a/go/vt/vtgate/vschema_manager.go +++ b/go/vt/vtgate/vschema_manager.go @@ -198,9 +198,16 @@ func (vm *VSchemaManager) buildAndEnhanceVSchema(v *vschemapb.SrvVSchema) *vinde func (vm *VSchemaManager) updateFromSchema(vschema *vindexes.VSchema) { for ksName, ks := range vschema.Keyspaces { m := vm.schema.Tables(ksName) + // Before we add the foreign key definitions in the tables, we need to make sure that all the tables + // are created in the Vschema, so that later when we try to find the routed tables, we don't end up + // getting dummy tables. + for tblName, tblInfo := range m { + setColumns(ks, tblName, tblInfo.Columns) + } + // Now that we have ensured that all the tables are created, we can start populating the foreign keys + // in the tables. for tblName, tblInfo := range m { - vTbl := setColumns(ks, tblName, tblInfo.Columns) for _, fkDef := range tblInfo.ForeignKeys { parentTbl, err := vschema.FindRoutedTable(ksName, fkDef.ReferenceDefinition.ReferencedTable.Name.String(), topodatapb.TabletType_PRIMARY) if err != nil { @@ -212,7 +219,7 @@ func (vm *VSchemaManager) updateFromSchema(vschema *vindexes.VSchema) { log.Errorf("error finding child table %s: %v", tblName, err) continue } - vTbl.ParentForeignKeys = append(vTbl.ParentForeignKeys, vindexes.NewParentFkInfo(parentTbl, fkDef)) + childTbl.ParentForeignKeys = append(childTbl.ParentForeignKeys, vindexes.NewParentFkInfo(parentTbl, fkDef)) parentTbl.ChildForeignKeys = append(parentTbl.ChildForeignKeys, vindexes.NewChildFkInfo(childTbl, fkDef)) } } diff --git a/go/vt/vtgate/vschema_manager_test.go b/go/vt/vtgate/vschema_manager_test.go index 4c2649040ce..5d7af6fe5c6 100644 --- a/go/vt/vtgate/vschema_manager_test.go +++ b/go/vt/vtgate/vschema_manager_test.go @@ -29,6 +29,58 @@ func TestVSchemaUpdate(t *testing.T) { tblCol2 := &vindexes.Table{Name: sqlparser.NewIdentifierCS("tbl"), Keyspace: ks, Columns: cols2, ColumnListAuthoritative: true} tblCol2NA := &vindexes.Table{Name: sqlparser.NewIdentifierCS("tbl"), Keyspace: ks, Columns: cols2} + vindexTable_multicol_t1 := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS("multicol_t1"), + Keyspace: ks, + Columns: cols2, + ColumnListAuthoritative: true, + } + vindexTable_multicol_t2 := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS("multicol_t2"), + Keyspace: ks, + Columns: cols2, + ColumnListAuthoritative: true, + } + vindexTable_t1 := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS("t1"), + Keyspace: ks, + Columns: cols1, + ColumnListAuthoritative: true, + } + vindexTable_t2 := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS("t2"), + Keyspace: ks, + Columns: cols1, + ColumnListAuthoritative: true, + } + sqlparserCols1 := sqlparser.MakeColumns("id") + sqlparserCols2 := sqlparser.MakeColumns("uid", "name") + + vindexTable_multicol_t1.ChildForeignKeys = append(vindexTable_multicol_t1.ChildForeignKeys, vindexes.ChildFKInfo{ + Table: vindexTable_multicol_t2, + ChildColumns: sqlparserCols2, + ParentColumns: sqlparserCols2, + OnDelete: sqlparser.NoAction, + OnUpdate: sqlparser.Restrict, + }) + vindexTable_multicol_t2.ParentForeignKeys = append(vindexTable_multicol_t2.ParentForeignKeys, vindexes.ParentFKInfo{ + Table: vindexTable_multicol_t1, + ChildColumns: sqlparserCols2, + ParentColumns: sqlparserCols2, + }) + vindexTable_t1.ChildForeignKeys = append(vindexTable_t1.ChildForeignKeys, vindexes.ChildFKInfo{ + Table: vindexTable_t2, + ChildColumns: sqlparserCols1, + ParentColumns: sqlparserCols1, + OnDelete: sqlparser.SetNull, + OnUpdate: sqlparser.Cascade, + }) + vindexTable_t2.ParentForeignKeys = append(vindexTable_t2.ParentForeignKeys, vindexes.ParentFKInfo{ + Table: vindexTable_t1, + ChildColumns: sqlparserCols1, + ParentColumns: sqlparserCols1, + }) + tcases := []struct { name string srvVschema *vschemapb.SrvVSchema @@ -94,6 +146,108 @@ func TestVSchemaUpdate(t *testing.T) { currentVSchema: &vindexes.VSchema{}, schema: map[string]*vindexes.TableInfo{"tbl": {Columns: cols1}}, expected: &vindexes.VSchema{}, + }, { + name: "foreign keys in schema", + currentVSchema: &vindexes.VSchema{}, + schema: map[string]*vindexes.TableInfo{ + "t1": { + Columns: cols1, + }, + "t2": { + Columns: cols1, + ForeignKeys: []*sqlparser.ForeignKeyDefinition{ + { + Source: sqlparser.MakeColumns("id"), + ReferenceDefinition: &sqlparser.ReferenceDefinition{ + ReferencedTable: sqlparser.NewTableName("t1"), + ReferencedColumns: sqlparserCols1, + OnUpdate: sqlparser.Cascade, + OnDelete: sqlparser.SetNull, + }, + }, + }, + }, + "multicol_t1": { + Columns: cols2, + }, + "multicol_t2": { + Columns: cols2, + ForeignKeys: []*sqlparser.ForeignKeyDefinition{ + { + Source: sqlparser.MakeColumns("uid", "name"), + ReferenceDefinition: &sqlparser.ReferenceDefinition{ + ReferencedTable: sqlparser.NewTableName("multicol_t1"), + ReferencedColumns: sqlparserCols2, + OnUpdate: sqlparser.Restrict, + OnDelete: sqlparser.NoAction, + }, + }, + }, + }, + }, + srvVschema: &vschemapb.SrvVSchema{ + Keyspaces: map[string]*vschemapb.Keyspace{ + "ks": { + Sharded: false, + ForeignKeyMode: vschemapb.Keyspace_FK_MANAGED, + Tables: map[string]*vschemapb.Table{ + "t1": { + Columns: []*vschemapb.Column{ + { + Name: "id", + Type: querypb.Type_INT64, + }, + }, + }, + "t2": { + Columns: []*vschemapb.Column{ + { + Name: "id", + Type: querypb.Type_INT64, + }, + }, + }, + "multicol_t1": { + Columns: []*vschemapb.Column{ + { + Name: "uid", + Type: querypb.Type_INT64, + }, { + Name: "name", + Type: querypb.Type_VARCHAR, + }, + }, + }, "multicol_t2": { + Columns: []*vschemapb.Column{ + { + Name: "uid", + Type: querypb.Type_INT64, + }, { + Name: "name", + Type: querypb.Type_VARCHAR, + }, + }, + }, + }, + }, + }, + }, + expected: &vindexes.VSchema{ + RoutingRules: map[string]*vindexes.RoutingRule{}, + Keyspaces: map[string]*vindexes.KeyspaceSchema{ + "ks": { + Keyspace: ks, + ForeignKeyMode: vschemapb.Keyspace_FK_MANAGED, + Vindexes: map[string]vindexes.Vindex{}, + Tables: map[string]*vindexes.Table{ + "t1": vindexTable_t1, + "t2": vindexTable_t2, + "multicol_t1": vindexTable_multicol_t1, + "multicol_t2": vindexTable_multicol_t2, + }, + }, + }, + }, }} vm := &VSchemaManager{} From 4220c512999680e2fc6380c73f1c174b3b26b86e Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Fri, 25 Aug 2023 10:26:57 +0530 Subject: [PATCH 26/31] refactor: fix license header Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/foreignkey/main_test.go b/go/test/endtoend/vtgate/foreignkey/main_test.go index bd1b3391982..9e1a799c2a1 100644 --- a/go/test/endtoend/vtgate/foreignkey/main_test.go +++ b/go/test/endtoend/vtgate/foreignkey/main_test.go @@ -1,5 +1,5 @@ /* -Copyright 2021 The Vitess Authors. +Copyright 2023 The Vitess Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 928c1aea95ba6346be1e4b79027a650037db46c3 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 28 Aug 2023 13:08:57 +0530 Subject: [PATCH 27/31] refactor fk cascade planning logic Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/operators/ast2op.go | 361 +++++++++--------- .../planbuilder/operators/ast2op_test.go | 39 +- .../testdata/foreignkey_cases.json | 7 +- go/vt/vtgate/planbuilder/update.go | 24 +- 4 files changed, 231 insertions(+), 200 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index a3f23721b51..408b6e10d78 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -135,14 +135,12 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return updOp, nil } - parentFKs := vindexTable.ParentFKsNeedsHandling() - childFks := vindexTable.ChildFKsNeedsHandling(vindexes.UpdateAction) - if len(childFks) == 0 && len(parentFKs) == 0 { + parentFks, childFks := getFKRequirementsForUpdate(updStmt.Exprs, vindexTable) + if len(childFks) == 0 && len(parentFks) == 0 { return updOp, nil } - parentFKs, childFks = fkNeedsHandlingForUpdates(updStmt.Exprs, parentFKs, childFks) - if len(parentFKs) > 0 { + if len(parentFks) > 0 { return nil, vterrors.VT12003() } @@ -156,156 +154,7 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return nil, vterrors.VT12001("foreign keys management at vitess with limit") } - var fkChildren []*FkChild - var selectExprs []sqlparser.SelectExpr - for _, fk := range childFks { - // Any RESTRICT type foreign keys that arrive here, - // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. - if isRestrict(fk.OnUpdate) { - return nil, vterrors.VT12002() - } - - // We need to select all the parent columns for the foreign key constraint, to use in the update of the child table. - var cols []int - for _, column := range fk.ParentColumns { - cols = append(cols, len(selectExprs)) - selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) - } - - // We only support literals in update queries, which trigger foreign key updates. - for _, expr := range updStmt.Exprs { - colIdx := fk.ParentColumns.FindColumn(expr.Name.Name) - if colIdx < 0 { - continue - } - switch expr.Expr.(type) { - case *sqlparser.Argument, *sqlparser.NullVal, sqlparser.BoolVal, *sqlparser.Literal: - default: - return nil, vterrors.VT12001("foreign keys management at vitess with non-literal values") - } - } - - // We now construct the update query for the child table - - // For CASCADE type constraint, the query looks like this - - // `UPDATE SET WHERE IN ()` - // For SET NULL type constraint, the query looks like this - - // `UPDATE SET - // WHERE IN () - // [AND NOT IN ()]` - // If all the columns of the foreign key in the parent don't actually end up getting updated, then we shouldn't be setting the child columns to NULL. - - // Some operations are common for both the cases. - // 1. building the bind variable name for the output from the SELECT query. - bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") - // 2. ValTuple having the column names of the foreign key constraint in the child table. - var valTuple sqlparser.ValTuple - for _, column := range fk.ChildColumns { - valTuple = append(valTuple, sqlparser.NewColName(column.String())) - } - // 3. Use the above 2 to create the comparison expression we use in the where condition of the UPDATE. - compExpr := &sqlparser.ComparisonExpr{ - Operator: sqlparser.InOp, - Left: valTuple, - Right: sqlparser.NewListArg(bvName), - } - - // For both the cases, 2 things are different. The where condition and the update expressions. - // For CASCADE type constraint, the where condition is the same as the comparison expression above. - // For SET NULL type constraint, the where condition is the same as the comparison expression above, with an additional condition (that condition is optional). - - // We create the variables for these 2 things, and populate them in the switch case below. - var childWhereExpr sqlparser.Expr = compExpr - var childUpdateExprs sqlparser.UpdateExprs - - switch fk.OnUpdate { - case sqlparser.Cascade: - // For the CASCADE type constraint, the update expressions are the same as the update expressions in the parent update query. - // We need to replace the column names in the update expressions with the corresponding child column names. - for _, updateExpr := range updClone.Exprs { - colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) - if colIdx >= 0 { - childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName(fk.ChildColumns[colIdx].String()), - Expr: updateExpr.Expr, - }) - } - } - case sqlparser.SetNull: - // For the SET NULL type constraint, we need to set all the child columns to NULL. - for _, column := range fk.ChildColumns { - childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName(column.String()), - Expr: &sqlparser.NullVal{}, - }) - } - - // There is a special case in this constraint though. - // If a user updates the parent columns with values that are exactly what it already has, such that none of the parent columns actually get updated, - // then we shouldn't be setting the children columns to NULL. - // We need to add an additional condition to the where clause to handle this case. - // The additional condition looks like [AND NOT IN ()]. - // If any of the parent columns is being set to NULL, then we don't need this condition. - var updateValues sqlparser.ValTuple - var somethingIsSetNull bool - for _, updateExpr := range updClone.Exprs { - colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) - if colIdx >= 0 { - _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if isNull { - somethingIsSetNull = true - break - } else { - updateValues = append(updateValues, updateExpr.Expr) - } - } - } - if !somethingIsSetNull { - childWhereExpr = &sqlparser.AndExpr{ - Left: compExpr, - Right: &sqlparser.ComparisonExpr{ - Operator: sqlparser.NotInOp, - Left: valTuple, - Right: updateValues, - }, - } - } - case sqlparser.SetDefault: - return nil, vterrors.VT09016() - } - childUpd := &sqlparser.Update{ - Exprs: childUpdateExprs, - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: childWhereExpr}, - } - childUpdOp, err := createOpFromStmt(ctx, childUpd) - if err != nil { - return nil, err - } - fkChildren = append(fkChildren, &FkChild{ - BVName: bvName, - Cols: cols, - Op: childUpdOp, - }) - } - - // We now create the selection operator to select the parent columns for the foreign key constraints. - // The Select statement looks something like this - `SELECT FROM WHERE ` - // TODO (@Harshit, @GuptaManan100): Compress the columns in the SELECT statement, if there are multiple foreign key constraints using the same columns. - selectionStmt := &sqlparser.Select{ - SelectExprs: selectExprs, - From: updClone.TableExprs, - Where: updClone.Where, - } - selection, err := createOpFromStmt(ctx, selectionStmt) - if err != nil { - return nil, err - } - - return &FkCascade{ - Selection: selection, - Children: fkChildren, - Parent: updOp, - }, nil + return createFKCascadeOp(ctx, updOp, updClone, childFks) } func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, vindexTable *vindexes.Table, qt *QueryTable, routing Routing) (ops.Operator, error) { @@ -359,9 +208,15 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U return subq, nil } -// fkNeedsHandlingForUpdates returns what all foreign keys need handling in Vitess for the given set of update expressions. -// Here we return all the parent and child foreign key constraints that could require any handling. -func fkNeedsHandlingForUpdates(updateExprs sqlparser.UpdateExprs, parentFks []vindexes.ParentFKInfo, childFks []vindexes.ChildFKInfo) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { +// getFKRequirementsForUpdate analyzes update expressions to determine which foreign key constraints needs management at the VTGate. +// It identifies parent and child foreign keys that require verification or cascade operations due to column updates. +func getFKRequirementsForUpdate(updateExprs sqlparser.UpdateExprs, vindexTable *vindexes.Table) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { + parentFks := vindexTable.ParentFKsNeedsHandling() + childFks := vindexTable.ChildFKsNeedsHandling(vindexes.UpdateAction) + if len(childFks) == 0 && len(parentFks) == 0 { + return nil, nil + } + pFksRequired := make([]bool, len(parentFks)) cFksRequired := make([]bool, len(childFks)) // Go over all the update expressions @@ -416,26 +271,176 @@ func fkNeedsHandlingForUpdates(updateExprs sqlparser.UpdateExprs, parentFks []vi return pFksNeedsHandling, cFksNeedsHandling } -// ColumnModified checks if any column in the parent table is being updated which has a child foreign key. -func ColumnModified(exprs sqlparser.UpdateExprs, getFks func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo)) bool { - for _, updateExpr := range exprs { - parentFKs, childFks := getFks(updateExpr) - for _, childFk := range childFks { - if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { - return true +func createFKCascadeOp(ctx *plancontext.PlanningContext, parentOp ops.Operator, updStmt *sqlparser.Update, childFks []vindexes.ChildFKInfo) (ops.Operator, error) { + // We only support simple expressions in update queries with cascade. + for _, updateExpr := range updStmt.Exprs { + switch updateExpr.Expr.(type) { + case *sqlparser.Argument, *sqlparser.NullVal, sqlparser.BoolVal, *sqlparser.Literal: + default: + return nil, vterrors.VT12001("foreign keys management at vitess with non-literal values") + } + } + + var fkChildren []*FkChild + var selectExprs []sqlparser.SelectExpr + + for _, fk := range childFks { + // Any RESTRICT type foreign keys that arrive here, + // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. + if isRestrict(fk.OnUpdate) { + return nil, vterrors.VT12002() + } + + // We need to select all the parent columns for the foreign key constraint, to use in the update of the child table. + cols, exprs := selectParentColumns(fk, len(selectExprs)) + selectExprs = append(selectExprs, exprs...) + + childOp, err := createFkChildOp(ctx, fk, updStmt, cols) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, childOp) + } + + selectionOp, err := createSelectionOp(ctx, selectExprs, updStmt) + if err != nil { + return nil, err + } + + return &FkCascade{ + Selection: selectionOp, + Children: fkChildren, + Parent: parentOp, + }, nil +} + +// createFkChildOp creates the update query operator for the child table based on the foreign key constraints. +// For CASCADE type constraint, the query looks like this - +// +// `UPDATE SET WHERE IN ()` +// +// For SET NULL type constraint, the query looks like this - +// +// `UPDATE SET +// WHERE IN () +// [AND NOT IN ()]` +// If all the columns of the foreign key in the parent don't actually end up getting updated, then we shouldn't be setting the child columns to NULL. +func createFkChildOp(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, updStmt *sqlparser.Update, cols []int) (*FkChild, error) { + // Reserve a bind variable name + bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") + + // Create child update operator + childUpdOp, err := createChildUpdateOp(ctx, fk, bvName, updStmt) + if err != nil { + return nil, err + } + + return &FkChild{ + BVName: bvName, + Cols: cols, + Op: childUpdOp, + }, nil +} + +func createChildUpdateOp(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, bvName string, updStmt *sqlparser.Update) (ops.Operator, error) { + // Create a ValTuple of child column names + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + + // Create a comparison expression for WHERE clause + compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) + + // For both the cases, 2 things are different. The where condition and the update expressions. + // For CASCADE type constraint, the where condition is the same as the comparison expression above. + // For SET NULL type constraint, the where condition is the same as the comparison expression above, with an additional condition (that condition is optional). + + // We create the variables for these 2 things, and populate them in the switch case below. + var childWhereExpr sqlparser.Expr = compExpr + var childUpdateExprs sqlparser.UpdateExprs + + switch fk.OnUpdate { + case sqlparser.Cascade: + // For the CASCADE type constraint, the update expressions are the same as the update expressions in the parent update query. + // We need to replace the column names in the update expressions with the corresponding child column names. + for _, updateExpr := range updStmt.Exprs { + colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx == -1 { + continue } + + childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(fk.ChildColumns[colIdx].String()), + Expr: updateExpr.Expr, + }) } - _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if isNull { - continue + case sqlparser.SetNull: + // For the SET NULL type constraint, we need to set all the child columns to NULL. + for _, column := range fk.ChildColumns { + childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(column.String()), + Expr: &sqlparser.NullVal{}, + }) } - for _, parentFk := range parentFKs { - if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - return true + + // There is a special case in this constraint though. + // If a user updates the parent columns with values that are exactly what it already has, such that none of the parent columns actually get updated, + // then we shouldn't be setting the children columns to NULL. + // We need to add a condition to the where clause to handle this case. + // The additional condition looks like [AND NOT IN ()]. + // If any of the parent columns is being set to NULL, then we don't need this condition. + var updateValues sqlparser.ValTuple + colSetToNull := false + for _, updateExpr := range updStmt.Exprs { + colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx >= 0 { + _, colSetToNull = updateExpr.Expr.(*sqlparser.NullVal) + if colSetToNull { + break + } + updateValues = append(updateValues, updateExpr.Expr) } } + if !colSetToNull { + childWhereExpr = &sqlparser.AndExpr{ + Left: compExpr, + Right: sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, updateValues, nil), + } + } + case sqlparser.SetDefault: + return nil, vterrors.VT09016() } - return false + + childUpd := &sqlparser.Update{ + Exprs: childUpdateExprs, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: childWhereExpr}, + } + return createOpFromStmt(ctx, childUpd) +} + +// createSelectionOp creates the selection operator to select the parent columns for the foreign key constraints. +// The Select statement looks something like this - `SELECT FROM WHERE ` +// TODO (@Harshit, @GuptaManan100): Compress the columns in the SELECT statement, if there are multiple foreign key constraints using the same columns. +func createSelectionOp(ctx *plancontext.PlanningContext, selectExprs []sqlparser.SelectExpr, updStmt *sqlparser.Update) (ops.Operator, error) { + selectionStmt := &sqlparser.Select{ + SelectExprs: selectExprs, + From: updStmt.TableExprs, + Where: updStmt.Where, + } + return createOpFromStmt(ctx, selectionStmt) +} + +func selectParentColumns(fk vindexes.ChildFKInfo, lastOffset int) ([]int, []sqlparser.SelectExpr) { + var cols []int + var exprs []sqlparser.SelectExpr + for _, column := range fk.ParentColumns { + cols = append(cols, lastOffset) + exprs = append(exprs, aeWrap(sqlparser.NewColName(column.String()))) + lastOffset++ + } + return cols, exprs } func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete) (ops.Operator, error) { @@ -501,11 +506,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp for _, column := range fk.ChildColumns { valTuple = append(valTuple, sqlparser.NewColName(column.String())) } - compExpr := &sqlparser.ComparisonExpr{ - Operator: sqlparser.InOp, - Left: valTuple, - Right: sqlparser.NewListArg(bvName), - } + compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) childDel := &sqlparser.Delete{ TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, @@ -532,11 +533,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp Expr: &sqlparser.NullVal{}, }) } - compExpr := &sqlparser.ComparisonExpr{ - Operator: sqlparser.InOp, - Left: valTuple, - Right: sqlparser.NewListArg(bvName), - } + compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) childUpd := &sqlparser.Update{ Exprs: updExprs, TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, diff --git a/go/vt/vtgate/planbuilder/operators/ast2op_test.go b/go/vt/vtgate/planbuilder/operators/ast2op_test.go index 4e60ac28c59..29040812b27 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op_test.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op_test.go @@ -29,7 +29,12 @@ import ( // It verifies the different cases in which foreign key handling is required on vtgate level. func Test_fkNeedsHandlingForUpdates(t *testing.T) { t1 := &vindexes.Table{ - Name: sqlparser.NewIdentifierCS("t1"), + Name: sqlparser.NewIdentifierCS("t1"), + Keyspace: &vindexes.Keyspace{Name: "ks"}, + } + t2 := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS("t2"), + Keyspace: &vindexes.Keyspace{Name: "ks2"}, } tests := []struct { @@ -50,13 +55,13 @@ func Test_fkNeedsHandlingForUpdates(t *testing.T) { }, childFks: []vindexes.ChildFKInfo{ { - Table: t1, + Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c"), }, }, parentFks: []vindexes.ParentFKInfo{ { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b", "c"), }, }, @@ -72,16 +77,16 @@ func Test_fkNeedsHandlingForUpdates(t *testing.T) { }, childFks: []vindexes.ChildFKInfo{ { - Table: t1, + Table: t2, ParentColumns: sqlparser.MakeColumns("b", "a", "c"), }, { - Table: t1, + Table: t2, ParentColumns: sqlparser.MakeColumns("d", "c"), }, }, parentFks: []vindexes.ParentFKInfo{ { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b", "c"), }, }, @@ -97,16 +102,16 @@ func Test_fkNeedsHandlingForUpdates(t *testing.T) { }, childFks: []vindexes.ChildFKInfo{ { - Table: t1, + Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c"), }, }, parentFks: []vindexes.ParentFKInfo{ { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c"), }, { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("d", "b"), }, }, @@ -122,16 +127,16 @@ func Test_fkNeedsHandlingForUpdates(t *testing.T) { }, childFks: []vindexes.ChildFKInfo{ { - Table: t1, + Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c"), }, }, parentFks: []vindexes.ParentFKInfo{ { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c"), }, { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b"), }, }, @@ -150,16 +155,16 @@ func Test_fkNeedsHandlingForUpdates(t *testing.T) { }, childFks: []vindexes.ChildFKInfo{ { - Table: t1, + Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c"), }, }, parentFks: []vindexes.ParentFKInfo{ { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c"), }, { - Table: t1, + Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b"), }, }, @@ -169,7 +174,9 @@ func Test_fkNeedsHandlingForUpdates(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - parentFksGot, childFksGot := fkNeedsHandlingForUpdates(tt.updateExprs, tt.parentFks, tt.childFks) + t1.ParentForeignKeys = tt.parentFks + t1.ChildForeignKeys = tt.childFks + parentFksGot, childFksGot := getFKRequirementsForUpdate(tt.updateExprs, t1) var pFks []vindexes.ParentFKInfo for idx, b := range tt.parentFKsWanted { if b { diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 35e591ef0a6..1bfa14d2fc5 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -740,10 +740,15 @@ "plan": "VT12001: unsupported: foreign keys management at vitess with limit" }, { - "comment": "update in a table with non-literal value - disallowed", + "comment": "update in a table with non-literal value - set null fail due to child update where condition", "query": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1", "plan": "VT12001: unsupported: foreign keys management at vitess with non-literal values" }, + { + "comment": "update in a table with non-literal value - with cascade fail as the cascade value is not known", + "query": "update u_tbl1 set m = 2, col1 = x + 'bar' where id = 1", + "plan": "VT12001: unsupported: foreign keys management at vitess with non-literal values" + }, { "comment": "update in a table with a child table having SET DEFAULT constraint - disallowed", "query": "update tbl20 set col2 = 'bar'", diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index 0f18786c3f9..4eafc597f20 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -115,7 +115,29 @@ func fkManagementNotRequiredForUpdate(semTable *semantics.SemTable, vschema plan } // Check if any column in the parent table is being updated which has a child foreign key. - return !operators.ColumnModified(updateExprs, getFKInfo) + return !columnModified(updateExprs, getFKInfo) +} + +// columnModified checks if any column in the parent table is being updated which has a child foreign key. +func columnModified(exprs sqlparser.UpdateExprs, getFks func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo)) bool { + for _, updateExpr := range exprs { + parentFKs, childFks := getFks(updateExpr) + for _, childFk := range childFks { + if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { + return true + } + } + _, isNull := updateExpr.Expr.(*sqlparser.NullVal) + if isNull { + continue + } + for _, parentFk := range parentFKs { + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + return true + } + } + } + return false } func updateUnshardedShortcut(stmt *sqlparser.Update, ks *vindexes.Keyspace, tables []*vindexes.Table) logicalPlan { From caefacabe169eca1c7c7da2933d2d0bdc8bc5f35 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 28 Aug 2023 13:52:41 +0530 Subject: [PATCH 28/31] test: refactor Signed-off-by: Harshit Gangal --- .../planbuilder/operators/ast2op_test.go | 205 +++++++----------- 1 file changed, 74 insertions(+), 131 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/ast2op_test.go b/go/vt/vtgate/planbuilder/operators/ast2op_test.go index 29040812b27..0e5f86fc8bc 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op_test.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op_test.go @@ -44,148 +44,91 @@ func Test_fkNeedsHandlingForUpdates(t *testing.T) { childFks []vindexes.ChildFKInfo parentFKsWanted []bool childFKsWanted []bool - }{ - { - name: "No Fks filtered", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName("a"), - Expr: sqlparser.NewIntLiteral("1"), - }, - }, - childFks: []vindexes.ChildFKInfo{ - { - Table: t2, - ParentColumns: sqlparser.MakeColumns("a", "b", "c"), - }, - }, - parentFks: []vindexes.ParentFKInfo{ - { - Table: t2, - ChildColumns: sqlparser.MakeColumns("a", "b", "c"), - }, - }, - parentFKsWanted: []bool{true}, - childFKsWanted: []bool{true}, - }, { - name: "Child Fks filtering", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName("a"), - Expr: sqlparser.NewIntLiteral("1"), - }, - }, - childFks: []vindexes.ChildFKInfo{ - { - Table: t2, - ParentColumns: sqlparser.MakeColumns("b", "a", "c"), - }, { - Table: t2, - ParentColumns: sqlparser.MakeColumns("d", "c"), - }, - }, - parentFks: []vindexes.ParentFKInfo{ - { - Table: t2, - ChildColumns: sqlparser.MakeColumns("a", "b", "c"), - }, - }, - parentFKsWanted: []bool{true}, - childFKsWanted: []bool{true, false}, - }, { - name: "Parent Fks filtered based on columns", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName("a"), - Expr: sqlparser.NewIntLiteral("1"), - }, - }, - childFks: []vindexes.ChildFKInfo{ - { - Table: t2, - ParentColumns: sqlparser.MakeColumns("a", "b", "c"), - }, - }, - parentFks: []vindexes.ParentFKInfo{ - { - Table: t2, - ChildColumns: sqlparser.MakeColumns("b", "a", "c"), - }, { - Table: t2, - ChildColumns: sqlparser.MakeColumns("d", "b"), - }, - }, - parentFKsWanted: []bool{true, false}, - childFKsWanted: []bool{true}, - }, { - name: "Parent Fks filtered because all null values", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName("a"), - Expr: &sqlparser.NullVal{}, - }, - }, - childFks: []vindexes.ChildFKInfo{ - { - Table: t2, - ParentColumns: sqlparser.MakeColumns("a", "b", "c"), - }, - }, - parentFks: []vindexes.ParentFKInfo{ - { - Table: t2, - ChildColumns: sqlparser.MakeColumns("b", "a", "c"), - }, { - Table: t2, - ChildColumns: sqlparser.MakeColumns("a", "b"), - }, - }, - parentFKsWanted: []bool{false, false}, - childFKsWanted: []bool{true}, - }, { - name: "Parent Fks filtered because some column has null values", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName("a"), - Expr: sqlparser.NewIntLiteral("1"), - }, &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName("c"), - Expr: &sqlparser.NullVal{}, - }, - }, - childFks: []vindexes.ChildFKInfo{ - { - Table: t2, - ParentColumns: sqlparser.MakeColumns("a", "b", "c"), - }, - }, - parentFks: []vindexes.ParentFKInfo{ - { - Table: t2, - ChildColumns: sqlparser.MakeColumns("b", "a", "c"), - }, { - Table: t2, - ChildColumns: sqlparser.MakeColumns("a", "b"), - }, - }, - parentFKsWanted: []bool{false, true}, - childFKsWanted: []bool{true}, + }{{ + name: "No Fks filtered", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, }, - } + childFks: []vindexes.ChildFKInfo{ + {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, + }, + parentFks: []vindexes.ParentFKInfo{ + {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b", "c")}, + }, + parentFKsWanted: []bool{true}, + childFKsWanted: []bool{true}, + }, { + name: "Child Fks filtering", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, + }, + childFks: []vindexes.ChildFKInfo{ + {Table: t2, ParentColumns: sqlparser.MakeColumns("b", "a", "c")}, + {Table: t2, ParentColumns: sqlparser.MakeColumns("d", "c")}, + }, + parentFks: []vindexes.ParentFKInfo{ + {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b", "c")}, + }, + parentFKsWanted: []bool{true}, + childFKsWanted: []bool{true, false}, + }, { + name: "Parent Fks filtered based on columns", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, + }, + childFks: []vindexes.ChildFKInfo{ + {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, + }, + parentFks: []vindexes.ParentFKInfo{ + {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, + {Table: t2, ChildColumns: sqlparser.MakeColumns("d", "b")}, + }, + parentFKsWanted: []bool{true, false}, + childFKsWanted: []bool{true}, + }, { + name: "Parent Fks filtered because all null values", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: &sqlparser.NullVal{}}, + }, + childFks: []vindexes.ChildFKInfo{ + {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, + }, + parentFks: []vindexes.ParentFKInfo{ + {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, + {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b")}, + }, + parentFKsWanted: []bool{false, false}, + childFKsWanted: []bool{true}, + }, { + name: "Parent Fks filtered because some column has null values", + updateExprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, + &sqlparser.UpdateExpr{Name: sqlparser.NewColName("c"), Expr: &sqlparser.NullVal{}}, + }, + childFks: []vindexes.ChildFKInfo{ + {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, + }, + parentFks: []vindexes.ParentFKInfo{ + {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, + {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b")}, + }, + parentFKsWanted: []bool{false, true}, + childFKsWanted: []bool{true}, + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t1.ParentForeignKeys = tt.parentFks t1.ChildForeignKeys = tt.childFks parentFksGot, childFksGot := getFKRequirementsForUpdate(tt.updateExprs, t1) var pFks []vindexes.ParentFKInfo - for idx, b := range tt.parentFKsWanted { - if b { + for idx, expected := range tt.parentFKsWanted { + if expected { pFks = append(pFks, tt.parentFks[idx]) } } var cFks []vindexes.ChildFKInfo - for idx, b := range tt.childFKsWanted { - if b { + for idx, expected := range tt.childFKsWanted { + if expected { cFks = append(cFks, tt.childFks[idx]) } } From f15fb52c8f3ccf0f4189ba4b4c54b394fa0a1cab Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 28 Aug 2023 15:22:57 +0530 Subject: [PATCH 29/31] refactor: FkChild shouldn't be an operator Signed-off-by: Manan Gupta --- .../planbuilder/operators/fk_cascade.go | 22 ++++++- .../vtgate/planbuilder/operators/fk_child.go | 58 ------------------- 2 files changed, 19 insertions(+), 61 deletions(-) delete mode 100644 go/vt/vtgate/planbuilder/operators/fk_child.go diff --git a/go/vt/vtgate/planbuilder/operators/fk_cascade.go b/go/vt/vtgate/planbuilder/operators/fk_cascade.go index 6e6e781bb99..4650bc3e1e5 100644 --- a/go/vt/vtgate/planbuilder/operators/fk_cascade.go +++ b/go/vt/vtgate/planbuilder/operators/fk_cascade.go @@ -17,9 +17,20 @@ limitations under the License. package operators import ( + "golang.org/x/exp/slices" + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" ) +type FkChild struct { + BVName string + Cols []int // indexes + Op ops.Operator + + noColumns + noPredicates +} + // FkCascade is used to represent a foreign key cascade operation // as an operator. This operator is created for DML queries that require // cascades (for example, ON DELETE CASCADE). @@ -40,7 +51,7 @@ func (fkc *FkCascade) Inputs() []ops.Operator { inputs = append(inputs, fkc.Parent) inputs = append(inputs, fkc.Selection) for _, child := range fkc.Children { - inputs = append(inputs, child) + inputs = append(inputs, child.Op) } return inputs } @@ -56,7 +67,7 @@ func (fkc *FkCascade) SetInputs(operators []ops.Operator) { if idx < 2 { continue } - fkc.Children[idx-2] = operator.(*FkChild) + fkc.Children[idx-2].Op = operator } } @@ -73,7 +84,12 @@ func (fkc *FkCascade) Clone(inputs []ops.Operator) ops.Operator { if idx < 2 { continue } - newFkc.Children = append(newFkc.Children, operator.(*FkChild)) + + newFkc.Children = append(newFkc.Children, &FkChild{ + BVName: fkc.Children[idx-2].BVName, + Cols: slices.Clone(fkc.Children[idx-2].Cols), + Op: operator, + }) } return newFkc } diff --git a/go/vt/vtgate/planbuilder/operators/fk_child.go b/go/vt/vtgate/planbuilder/operators/fk_child.go deleted file mode 100644 index c0fa6c2581b..00000000000 --- a/go/vt/vtgate/planbuilder/operators/fk_child.go +++ /dev/null @@ -1,58 +0,0 @@ -/* -Copyright 2023 The Vitess Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package operators - -import ( - "fmt" - - "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" -) - -type FkChild struct { - BVName string - Cols []int // indexes - Op ops.Operator - - noColumns - noPredicates -} - -func (f *FkChild) Clone(inputs []ops.Operator) ops.Operator { - return &FkChild{ - BVName: f.BVName, - Cols: f.Cols, - Op: inputs[0], - } -} - -func (f *FkChild) Inputs() []ops.Operator { - return []ops.Operator{f.Op} -} - -func (f *FkChild) SetInputs(operators []ops.Operator) { - f.Op = operators[0] -} - -func (f *FkChild) ShortDescription() string { - return fmt.Sprintf("BvName: %s, Cols: %v", f.BVName, f.Cols) -} - -func (f *FkChild) GetOrdering() ([]ops.OrderBy, error) { - return nil, nil -} - -var _ ops.Operator = (*FkChild)(nil) From c81f64e2a0ec38d57b4fcc63adf84372191e3c9f Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 28 Aug 2023 15:56:47 +0530 Subject: [PATCH 30/31] fix build Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/operators/fk_cascade.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/operators/fk_cascade.go b/go/vt/vtgate/planbuilder/operators/fk_cascade.go index 4650bc3e1e5..e3a2ef3d38d 100644 --- a/go/vt/vtgate/planbuilder/operators/fk_cascade.go +++ b/go/vt/vtgate/planbuilder/operators/fk_cascade.go @@ -17,7 +17,7 @@ limitations under the License. package operators import ( - "golang.org/x/exp/slices" + "slices" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" ) From 6a722e99d8d0e7d2d55e59ac541c9b2dcec7172d Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 28 Aug 2023 21:57:53 +0530 Subject: [PATCH 31/31] refactor of cascade op Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/operators/ast2op.go | 563 ++---------------- .../planbuilder/operators/ast_to_delete_op.go | 209 +++++++ .../planbuilder/operators/ast_to_update_op.go | 327 ++++++++++ .../planbuilder/operators/fk_cascade.go | 1 + go/vt/vtgate/planbuilder/select.go | 6 +- go/vt/vtgate/planbuilder/update.go | 3 +- 6 files changed, 575 insertions(+), 534 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/operators/ast_to_delete_op.go create mode 100644 go/vt/vtgate/planbuilder/operators/ast_to_update_op.go diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 408b6e10d78..8e1e76c65c6 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -31,6 +31,8 @@ import ( "vitess.io/vitess/go/vt/vtgate/vindexes" ) +const foriegnKeyContraintValues = "fkc_vals" + // translateQueryToOp creates an operator tree that represents the input SELECT or UNION query func translateQueryToOp(ctx *plancontext.PlanningContext, selStmt sqlparser.Statement) (op ops.Operator, err error) { switch node := selStmt.(type) { @@ -109,535 +111,6 @@ func createOperatorFromUnion(ctx *plancontext.PlanningContext, node *sqlparser.U return &Horizon{Source: union, Query: node}, nil } -func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update) (ops.Operator, error) { - tableInfo, qt, err := createQueryTableForDML(ctx, updStmt.TableExprs[0], updStmt.Where) - if err != nil { - return nil, err - } - - vindexTable, routing, err := buildVindexTableForDML(ctx, tableInfo, qt, "update") - if err != nil { - return nil, err - } - - updClone := sqlparser.CloneRefOfUpdate(updStmt) - updOp, err := createUpdateOperator(ctx, updStmt, vindexTable, qt, routing) - if err != nil { - return nil, err - } - - ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) - if err != nil { - return nil, err - } - // Unmanaged foreign-key-mode, we don't need to do anything. - if ksMode != vschemapb.Keyspace_FK_MANAGED { - return updOp, nil - } - - parentFks, childFks := getFKRequirementsForUpdate(updStmt.Exprs, vindexTable) - if len(childFks) == 0 && len(parentFks) == 0 { - return updOp, nil - } - - if len(parentFks) > 0 { - return nil, vterrors.VT12003() - } - - // If there are no foreign key constraints, then we don't need to do anything. - if len(childFks) == 0 { - return updOp, nil - } - - // If the delete statement has a limit, we don't support it yet. - if updStmt.Limit != nil { - return nil, vterrors.VT12001("foreign keys management at vitess with limit") - } - - return createFKCascadeOp(ctx, updOp, updClone, childFks) -} - -func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, vindexTable *vindexes.Table, qt *QueryTable, routing Routing) (ops.Operator, error) { - assignments := make(map[string]sqlparser.Expr) - for _, set := range updStmt.Exprs { - assignments[set.Name.Name.String()] = set.Expr - } - - vp, cvv, ovq, err := getUpdateVindexInformation(updStmt, vindexTable, qt.ID, qt.Predicates) - if err != nil { - return nil, err - } - - tr, ok := routing.(*ShardedRouting) - if ok { - tr.VindexPreds = vp - } - - for _, predicate := range qt.Predicates { - routing, err = UpdateRoutingLogic(ctx, predicate, routing) - if err != nil { - return nil, err - } - } - - if routing.OpCode() == engine.Scatter && updStmt.Limit != nil { - // TODO systay: we should probably check for other op code types - IN could also hit multiple shards (2022-04-07) - return nil, vterrors.VT12001("multi shard UPDATE with LIMIT") - } - - r := &Route{ - Source: &Update{ - QTable: qt, - VTable: vindexTable, - Assignments: assignments, - ChangedVindexValues: cvv, - OwnedVindexQuery: ovq, - AST: updStmt, - }, - Routing: routing, - } - - subq, err := createSubqueryFromStatement(ctx, updStmt) - if err != nil { - return nil, err - } - if subq == nil { - return r, nil - } - subq.Outer = r - return subq, nil -} - -// getFKRequirementsForUpdate analyzes update expressions to determine which foreign key constraints needs management at the VTGate. -// It identifies parent and child foreign keys that require verification or cascade operations due to column updates. -func getFKRequirementsForUpdate(updateExprs sqlparser.UpdateExprs, vindexTable *vindexes.Table) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { - parentFks := vindexTable.ParentFKsNeedsHandling() - childFks := vindexTable.ChildFKsNeedsHandling(vindexes.UpdateAction) - if len(childFks) == 0 && len(parentFks) == 0 { - return nil, nil - } - - pFksRequired := make([]bool, len(parentFks)) - cFksRequired := make([]bool, len(childFks)) - // Go over all the update expressions - for _, updateExpr := range updateExprs { - // Any foreign key to a child table for a column that has been updated - // will require the cascade operations to happen, so we include all such foreign keys. - for idx, childFk := range childFks { - if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { - cFksRequired[idx] = true - } - } - // If we are setting a column to NULL, then we don't need to verify the existance of an - // equivalent row in the parent table, even if this column was part of a foreign key to a parent table. - _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if isNull { - continue - } - // We add all the possible parent foreign key constraints that need verification that an equivalent row - // exists, given that this column has changed. - for idx, parentFk := range parentFks { - if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequired[idx] = true - } - } - } - // For the parent foreign keys, if any of the columns part of the fk is set to NULL, - // then, we don't care for the existance of an equivalent row in the parent table. - for idx, parentFk := range parentFks { - for _, updateExpr := range updateExprs { - _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if !isNull { - continue - } - if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequired[idx] = false - } - } - } - // Get the filtered lists and return them. - var pFksNeedsHandling []vindexes.ParentFKInfo - var cFksNeedsHandling []vindexes.ChildFKInfo - for idx, parentFk := range parentFks { - if pFksRequired[idx] { - pFksNeedsHandling = append(pFksNeedsHandling, parentFk) - } - } - for idx, childFk := range childFks { - if cFksRequired[idx] { - cFksNeedsHandling = append(cFksNeedsHandling, childFk) - } - } - return pFksNeedsHandling, cFksNeedsHandling -} - -func createFKCascadeOp(ctx *plancontext.PlanningContext, parentOp ops.Operator, updStmt *sqlparser.Update, childFks []vindexes.ChildFKInfo) (ops.Operator, error) { - // We only support simple expressions in update queries with cascade. - for _, updateExpr := range updStmt.Exprs { - switch updateExpr.Expr.(type) { - case *sqlparser.Argument, *sqlparser.NullVal, sqlparser.BoolVal, *sqlparser.Literal: - default: - return nil, vterrors.VT12001("foreign keys management at vitess with non-literal values") - } - } - - var fkChildren []*FkChild - var selectExprs []sqlparser.SelectExpr - - for _, fk := range childFks { - // Any RESTRICT type foreign keys that arrive here, - // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. - if isRestrict(fk.OnUpdate) { - return nil, vterrors.VT12002() - } - - // We need to select all the parent columns for the foreign key constraint, to use in the update of the child table. - cols, exprs := selectParentColumns(fk, len(selectExprs)) - selectExprs = append(selectExprs, exprs...) - - childOp, err := createFkChildOp(ctx, fk, updStmt, cols) - if err != nil { - return nil, err - } - fkChildren = append(fkChildren, childOp) - } - - selectionOp, err := createSelectionOp(ctx, selectExprs, updStmt) - if err != nil { - return nil, err - } - - return &FkCascade{ - Selection: selectionOp, - Children: fkChildren, - Parent: parentOp, - }, nil -} - -// createFkChildOp creates the update query operator for the child table based on the foreign key constraints. -// For CASCADE type constraint, the query looks like this - -// -// `UPDATE SET WHERE IN ()` -// -// For SET NULL type constraint, the query looks like this - -// -// `UPDATE SET -// WHERE IN () -// [AND NOT IN ()]` -// If all the columns of the foreign key in the parent don't actually end up getting updated, then we shouldn't be setting the child columns to NULL. -func createFkChildOp(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, updStmt *sqlparser.Update, cols []int) (*FkChild, error) { - // Reserve a bind variable name - bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") - - // Create child update operator - childUpdOp, err := createChildUpdateOp(ctx, fk, bvName, updStmt) - if err != nil { - return nil, err - } - - return &FkChild{ - BVName: bvName, - Cols: cols, - Op: childUpdOp, - }, nil -} - -func createChildUpdateOp(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, bvName string, updStmt *sqlparser.Update) (ops.Operator, error) { - // Create a ValTuple of child column names - var valTuple sqlparser.ValTuple - for _, column := range fk.ChildColumns { - valTuple = append(valTuple, sqlparser.NewColName(column.String())) - } - - // Create a comparison expression for WHERE clause - compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) - - // For both the cases, 2 things are different. The where condition and the update expressions. - // For CASCADE type constraint, the where condition is the same as the comparison expression above. - // For SET NULL type constraint, the where condition is the same as the comparison expression above, with an additional condition (that condition is optional). - - // We create the variables for these 2 things, and populate them in the switch case below. - var childWhereExpr sqlparser.Expr = compExpr - var childUpdateExprs sqlparser.UpdateExprs - - switch fk.OnUpdate { - case sqlparser.Cascade: - // For the CASCADE type constraint, the update expressions are the same as the update expressions in the parent update query. - // We need to replace the column names in the update expressions with the corresponding child column names. - for _, updateExpr := range updStmt.Exprs { - colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) - if colIdx == -1 { - continue - } - - childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName(fk.ChildColumns[colIdx].String()), - Expr: updateExpr.Expr, - }) - } - case sqlparser.SetNull: - // For the SET NULL type constraint, we need to set all the child columns to NULL. - for _, column := range fk.ChildColumns { - childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName(column.String()), - Expr: &sqlparser.NullVal{}, - }) - } - - // There is a special case in this constraint though. - // If a user updates the parent columns with values that are exactly what it already has, such that none of the parent columns actually get updated, - // then we shouldn't be setting the children columns to NULL. - // We need to add a condition to the where clause to handle this case. - // The additional condition looks like [AND NOT IN ()]. - // If any of the parent columns is being set to NULL, then we don't need this condition. - var updateValues sqlparser.ValTuple - colSetToNull := false - for _, updateExpr := range updStmt.Exprs { - colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) - if colIdx >= 0 { - _, colSetToNull = updateExpr.Expr.(*sqlparser.NullVal) - if colSetToNull { - break - } - updateValues = append(updateValues, updateExpr.Expr) - } - } - if !colSetToNull { - childWhereExpr = &sqlparser.AndExpr{ - Left: compExpr, - Right: sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, updateValues, nil), - } - } - case sqlparser.SetDefault: - return nil, vterrors.VT09016() - } - - childUpd := &sqlparser.Update{ - Exprs: childUpdateExprs, - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: childWhereExpr}, - } - return createOpFromStmt(ctx, childUpd) -} - -// createSelectionOp creates the selection operator to select the parent columns for the foreign key constraints. -// The Select statement looks something like this - `SELECT FROM WHERE ` -// TODO (@Harshit, @GuptaManan100): Compress the columns in the SELECT statement, if there are multiple foreign key constraints using the same columns. -func createSelectionOp(ctx *plancontext.PlanningContext, selectExprs []sqlparser.SelectExpr, updStmt *sqlparser.Update) (ops.Operator, error) { - selectionStmt := &sqlparser.Select{ - SelectExprs: selectExprs, - From: updStmt.TableExprs, - Where: updStmt.Where, - } - return createOpFromStmt(ctx, selectionStmt) -} - -func selectParentColumns(fk vindexes.ChildFKInfo, lastOffset int) ([]int, []sqlparser.SelectExpr) { - var cols []int - var exprs []sqlparser.SelectExpr - for _, column := range fk.ParentColumns { - cols = append(cols, lastOffset) - exprs = append(exprs, aeWrap(sqlparser.NewColName(column.String()))) - lastOffset++ - } - return cols, exprs -} - -func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete) (ops.Operator, error) { - tableInfo, qt, err := createQueryTableForDML(ctx, deleteStmt.TableExprs[0], deleteStmt.Where) - if err != nil { - return nil, err - } - - vindexTable, routing, err := buildVindexTableForDML(ctx, tableInfo, qt, "delete") - if err != nil { - return nil, err - } - - delClone := sqlparser.CloneRefOfDelete(deleteStmt) - // Create the delete operator first. - delOp, err := createDeleteOperator(ctx, deleteStmt, qt, vindexTable, routing) - if err != nil { - return nil, err - } - - // Now we check for the foreign key mode and make changes if required. - ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) - if err != nil { - return nil, err - } - - // Unmanaged foreign-key-mode, we don't need to do anything. - if ksMode != vschemapb.Keyspace_FK_MANAGED { - return delOp, nil - } - childFks := vindexTable.ChildFKsNeedsHandling(vindexes.DeleteAction) - // If there are no foreign key constraints, then we don't need to do anything. - if len(childFks) == 0 { - return delOp, nil - } - // If the delete statement has a limit, we don't support it yet. - if deleteStmt.Limit != nil { - return nil, vterrors.VT12001("foreign keys management at vitess with limit") - } - - var fkChildren []*FkChild - var selectExprs []sqlparser.SelectExpr - for _, fk := range childFks { - // Any RESTRICT type foreign keys that arrive here, - // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. - if isRestrict(fk.OnDelete) { - return nil, vterrors.VT12002() - } - - // We need to select all the parent columns for the foreign key constraint, to use in the deletion in the child. - var cols []int - for _, column := range fk.ParentColumns { - cols = append(cols, len(selectExprs)) - selectExprs = append(selectExprs, aeWrap(sqlparser.NewColName(column.String()))) - } - - switch fk.OnDelete { - case sqlparser.Cascade: - // We now construct the delete query for the child table. - // The query looks something like this - `DELETE FROM WHERE IN ()` - bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") - var valTuple sqlparser.ValTuple - for _, column := range fk.ChildColumns { - valTuple = append(valTuple, sqlparser.NewColName(column.String())) - } - compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) - childDel := &sqlparser.Delete{ - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, - } - childDelOp, err := createOpFromStmt(ctx, childDel) - if err != nil { - return nil, err - } - fkChildren = append(fkChildren, &FkChild{ - BVName: bvName, - Cols: cols, - Op: childDelOp, - }) - case sqlparser.SetNull: - // We now construct the update query for the child table. - // The query looks something like this - `UPDATE SET = NULL [AND = NULL]... WHERE IN ()` - bvName := ctx.ReservedVars.ReserveVariable("fkc_vals") - var valTuple sqlparser.ValTuple - var updExprs sqlparser.UpdateExprs - for _, column := range fk.ChildColumns { - valTuple = append(valTuple, sqlparser.NewColName(column.String())) - updExprs = append(updExprs, &sqlparser.UpdateExpr{ - Name: sqlparser.NewColName(column.String()), - Expr: &sqlparser.NullVal{}, - }) - } - compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) - childUpd := &sqlparser.Update{ - Exprs: updExprs, - TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, - Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, - } - childUpdOp, err := createOpFromStmt(ctx, childUpd) - if err != nil { - return nil, err - } - fkChildren = append(fkChildren, &FkChild{ - BVName: bvName, - Cols: cols, - Op: childUpdOp, - }) - case sqlparser.SetDefault: - return nil, vterrors.VT09016() - } - } - // We now create the selection operator to select the parent columns for the foreign key constraints. - // The Select statement looks something like this - `SELECT FROM WHERE ` - // TODO (@Harshit, @GuptaManan100): Compress the columns in the SELECT statement, if there are multiple foreign key constraints using the same columns. - selectionStmt := &sqlparser.Select{ - SelectExprs: selectExprs, - From: delClone.TableExprs, - Where: delClone.Where, - } - selection, err := createOpFromStmt(ctx, selectionStmt) - if err != nil { - return nil, err - } - - return &FkCascade{ - Selection: selection, - Children: fkChildren, - Parent: delOp, - }, nil -} - -func createDeleteOperator(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete, qt *QueryTable, vindexTable *vindexes.Table, routing Routing) (ops.Operator, error) { - del := &Delete{ - QTable: qt, - VTable: vindexTable, - AST: deleteStmt, - } - route := &Route{ - Source: del, - Routing: routing, - } - - if !vindexTable.Keyspace.Sharded { - return route, nil - } - - primaryVindex, vindexAndPredicates, err := getVindexInformation(qt.ID, qt.Predicates, vindexTable) - if err != nil { - return nil, err - } - - tr, ok := routing.(*ShardedRouting) - if ok { - tr.VindexPreds = vindexAndPredicates - } - - var ovq string - if len(vindexTable.Owned) > 0 { - tblExpr := &sqlparser.AliasedTableExpr{Expr: sqlparser.TableName{Name: vindexTable.Name}, As: qt.Alias.As} - ovq = generateOwnedVindexQuery(tblExpr, deleteStmt, vindexTable, primaryVindex.Columns) - } - - del.OwnedVindexQuery = ovq - - for _, predicate := range qt.Predicates { - var err error - route.Routing, err = UpdateRoutingLogic(ctx, predicate, route.Routing) - if err != nil { - return nil, err - } - } - - if routing.OpCode() == engine.Scatter && deleteStmt.Limit != nil { - // TODO systay: we should probably check for other op code types - IN could also hit multiple shards (2022-04-07) - return nil, vterrors.VT12001("multi shard DELETE with LIMIT") - } - - subq, err := createSubqueryFromStatement(ctx, deleteStmt) - if err != nil { - return nil, err - } - if subq == nil { - return route, nil - } - subq.Outer = route - return subq, nil -} - -func isRestrict(onDelete sqlparser.ReferenceAction) bool { - switch onDelete { - case sqlparser.Restrict, sqlparser.NoAction, sqlparser.DefaultAction: - return true - default: - return false - } -} - func createOpFromStmt(ctx *plancontext.PlanningContext, stmt sqlparser.Statement) (ops.Operator, error) { newCtx, err := plancontext.CreatePlanningContext(stmt, ctx.ReservedVars, ctx.VSchema, ctx.PlannerVersion) if err != nil { @@ -1106,3 +579,35 @@ func addColumnEquality(ctx *plancontext.PlanningContext, expr sqlparser.Expr) { } } } + +func isRestrict(onDelete sqlparser.ReferenceAction) bool { + switch onDelete { + case sqlparser.Restrict, sqlparser.NoAction, sqlparser.DefaultAction: + return true + default: + return false + } +} + +// createSelectionOp creates the selection operator to select the parent columns for the foreign key constraints. +// The Select statement looks something like this - `SELECT FROM WHERE ` +// TODO (@Harshit, @GuptaManan100): Compress the columns in the SELECT statement, if there are multiple foreign key constraints using the same columns. +func createSelectionOp(ctx *plancontext.PlanningContext, selectExprs []sqlparser.SelectExpr, tableExprs sqlparser.TableExprs, where *sqlparser.Where) (ops.Operator, error) { + selectionStmt := &sqlparser.Select{ + SelectExprs: selectExprs, + From: tableExprs, + Where: where, + } + return createOpFromStmt(ctx, selectionStmt) +} + +func selectParentColumns(fk vindexes.ChildFKInfo, lastOffset int) ([]int, []sqlparser.SelectExpr) { + var cols []int + var exprs []sqlparser.SelectExpr + for _, column := range fk.ParentColumns { + cols = append(cols, lastOffset) + exprs = append(exprs, aeWrap(sqlparser.NewColName(column.String()))) + lastOffset++ + } + return cols, exprs +} diff --git a/go/vt/vtgate/planbuilder/operators/ast_to_delete_op.go b/go/vt/vtgate/planbuilder/operators/ast_to_delete_op.go new file mode 100644 index 00000000000..55db837e147 --- /dev/null +++ b/go/vt/vtgate/planbuilder/operators/ast_to_delete_op.go @@ -0,0 +1,209 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operators + +import ( + vschemapb "vitess.io/vitess/go/vt/proto/vschema" + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" + "vitess.io/vitess/go/vt/vtgate/vindexes" +) + +func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete) (ops.Operator, error) { + tableInfo, qt, err := createQueryTableForDML(ctx, deleteStmt.TableExprs[0], deleteStmt.Where) + if err != nil { + return nil, err + } + + vindexTable, routing, err := buildVindexTableForDML(ctx, tableInfo, qt, "delete") + if err != nil { + return nil, err + } + + delClone := sqlparser.CloneRefOfDelete(deleteStmt) + // Create the delete operator first. + delOp, err := createDeleteOperator(ctx, deleteStmt, qt, vindexTable, routing) + if err != nil { + return nil, err + } + + // Now we check for the foreign key mode and make changes if required. + ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) + if err != nil { + return nil, err + } + + // Unmanaged foreign-key-mode, we don't need to do anything. + if ksMode != vschemapb.Keyspace_FK_MANAGED { + return delOp, nil + } + + childFks := vindexTable.ChildFKsNeedsHandling(vindexes.DeleteAction) + // If there are no foreign key constraints, then we don't need to do anything. + if len(childFks) == 0 { + return delOp, nil + } + // If the delete statement has a limit, we don't support it yet. + if deleteStmt.Limit != nil { + return nil, vterrors.VT12001("foreign keys management at vitess with limit") + } + + return createFkCascadeOpForDelete(ctx, delOp, delClone, childFks) +} + +func createDeleteOperator(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete, qt *QueryTable, vindexTable *vindexes.Table, routing Routing) (ops.Operator, error) { + del := &Delete{ + QTable: qt, + VTable: vindexTable, + AST: deleteStmt, + } + route := &Route{ + Source: del, + Routing: routing, + } + + if !vindexTable.Keyspace.Sharded { + return route, nil + } + + primaryVindex, vindexAndPredicates, err := getVindexInformation(qt.ID, qt.Predicates, vindexTable) + if err != nil { + return nil, err + } + + tr, ok := routing.(*ShardedRouting) + if ok { + tr.VindexPreds = vindexAndPredicates + } + + var ovq string + if len(vindexTable.Owned) > 0 { + tblExpr := &sqlparser.AliasedTableExpr{Expr: sqlparser.TableName{Name: vindexTable.Name}, As: qt.Alias.As} + ovq = generateOwnedVindexQuery(tblExpr, deleteStmt, vindexTable, primaryVindex.Columns) + } + + del.OwnedVindexQuery = ovq + + for _, predicate := range qt.Predicates { + var err error + route.Routing, err = UpdateRoutingLogic(ctx, predicate, route.Routing) + if err != nil { + return nil, err + } + } + + if routing.OpCode() == engine.Scatter && deleteStmt.Limit != nil { + // TODO systay: we should probably check for other op code types - IN could also hit multiple shards (2022-04-07) + return nil, vterrors.VT12001("multi shard DELETE with LIMIT") + } + + subq, err := createSubqueryFromStatement(ctx, deleteStmt) + if err != nil { + return nil, err + } + if subq == nil { + return route, nil + } + subq.Outer = route + return subq, nil +} + +func createFkCascadeOpForDelete(ctx *plancontext.PlanningContext, parentOp ops.Operator, delStmt *sqlparser.Delete, childFks []vindexes.ChildFKInfo) (ops.Operator, error) { + var fkChildren []*FkChild + var selectExprs []sqlparser.SelectExpr + for _, fk := range childFks { + // Any RESTRICT type foreign keys that arrive here, + // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. + if isRestrict(fk.OnDelete) { + return nil, vterrors.VT12002() + } + + // We need to select all the parent columns for the foreign key constraint, to use in the update of the child table. + cols, exprs := selectParentColumns(fk, len(selectExprs)) + selectExprs = append(selectExprs, exprs...) + + fkChild, err := createFkChildForDelete(ctx, fk, cols) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, fkChild) + } + selectionOp, err := createSelectionOp(ctx, selectExprs, delStmt.TableExprs, delStmt.Where) + if err != nil { + return nil, err + } + + return &FkCascade{ + Selection: selectionOp, + Children: fkChildren, + Parent: parentOp, + }, nil +} + +func createFkChildForDelete(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, cols []int) (*FkChild, error) { + bvName := ctx.ReservedVars.ReserveVariable(foriegnKeyContraintValues) + + var childStmt sqlparser.Statement + switch fk.OnDelete { + case sqlparser.Cascade: + // We now construct the delete query for the child table. + // The query looks something like this - `DELETE FROM WHERE IN ()` + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) + childStmt = &sqlparser.Delete{ + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + } + case sqlparser.SetNull: + // We now construct the update query for the child table. + // The query looks something like this - `UPDATE SET = NULL [AND = NULL]... WHERE IN ()` + var valTuple sqlparser.ValTuple + var updExprs sqlparser.UpdateExprs + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + updExprs = append(updExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(column.String()), + Expr: &sqlparser.NullVal{}, + }) + } + compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) + childStmt = &sqlparser.Update{ + Exprs: updExprs, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, + } + case sqlparser.SetDefault: + return nil, vterrors.VT09016() + } + + childOp, err := createOpFromStmt(ctx, childStmt) + if err != nil { + return nil, err + } + + return &FkChild{ + BVName: bvName, + Cols: cols, + Op: childOp, + }, nil +} diff --git a/go/vt/vtgate/planbuilder/operators/ast_to_update_op.go b/go/vt/vtgate/planbuilder/operators/ast_to_update_op.go new file mode 100644 index 00000000000..8c358547064 --- /dev/null +++ b/go/vt/vtgate/planbuilder/operators/ast_to_update_op.go @@ -0,0 +1,327 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operators + +import ( + vschemapb "vitess.io/vitess/go/vt/proto/vschema" + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" + "vitess.io/vitess/go/vt/vtgate/vindexes" +) + +func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update) (ops.Operator, error) { + tableInfo, qt, err := createQueryTableForDML(ctx, updStmt.TableExprs[0], updStmt.Where) + if err != nil { + return nil, err + } + + vindexTable, routing, err := buildVindexTableForDML(ctx, tableInfo, qt, "update") + if err != nil { + return nil, err + } + + updClone := sqlparser.CloneRefOfUpdate(updStmt) + updOp, err := createUpdateOperator(ctx, updStmt, vindexTable, qt, routing) + if err != nil { + return nil, err + } + + ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) + if err != nil { + return nil, err + } + // Unmanaged foreign-key-mode, we don't need to do anything. + if ksMode != vschemapb.Keyspace_FK_MANAGED { + return updOp, nil + } + + parentFks, childFks := getFKRequirementsForUpdate(updStmt.Exprs, vindexTable) + if len(childFks) == 0 && len(parentFks) == 0 { + return updOp, nil + } + + if len(parentFks) > 0 { + return nil, vterrors.VT12003() + } + + // If there are no foreign key constraints, then we don't need to do anything. + if len(childFks) == 0 { + return updOp, nil + } + + // If the delete statement has a limit, we don't support it yet. + if updStmt.Limit != nil { + return nil, vterrors.VT12001("foreign keys management at vitess with limit") + } + + return createFKCascadeOp(ctx, updOp, updClone, childFks) +} + +func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, vindexTable *vindexes.Table, qt *QueryTable, routing Routing) (ops.Operator, error) { + assignments := make(map[string]sqlparser.Expr) + for _, set := range updStmt.Exprs { + assignments[set.Name.Name.String()] = set.Expr + } + + vp, cvv, ovq, err := getUpdateVindexInformation(updStmt, vindexTable, qt.ID, qt.Predicates) + if err != nil { + return nil, err + } + + tr, ok := routing.(*ShardedRouting) + if ok { + tr.VindexPreds = vp + } + + for _, predicate := range qt.Predicates { + routing, err = UpdateRoutingLogic(ctx, predicate, routing) + if err != nil { + return nil, err + } + } + + if routing.OpCode() == engine.Scatter && updStmt.Limit != nil { + // TODO systay: we should probably check for other op code types - IN could also hit multiple shards (2022-04-07) + return nil, vterrors.VT12001("multi shard UPDATE with LIMIT") + } + + r := &Route{ + Source: &Update{ + QTable: qt, + VTable: vindexTable, + Assignments: assignments, + ChangedVindexValues: cvv, + OwnedVindexQuery: ovq, + AST: updStmt, + }, + Routing: routing, + } + + subq, err := createSubqueryFromStatement(ctx, updStmt) + if err != nil { + return nil, err + } + if subq == nil { + return r, nil + } + subq.Outer = r + return subq, nil +} + +// getFKRequirementsForUpdate analyzes update expressions to determine which foreign key constraints needs management at the VTGate. +// It identifies parent and child foreign keys that require verification or cascade operations due to column updates. +func getFKRequirementsForUpdate(updateExprs sqlparser.UpdateExprs, vindexTable *vindexes.Table) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { + parentFks := vindexTable.ParentFKsNeedsHandling() + childFks := vindexTable.ChildFKsNeedsHandling(vindexes.UpdateAction) + if len(childFks) == 0 && len(parentFks) == 0 { + return nil, nil + } + + pFksRequired := make([]bool, len(parentFks)) + cFksRequired := make([]bool, len(childFks)) + // Go over all the update expressions + for _, updateExpr := range updateExprs { + // Any foreign key to a child table for a column that has been updated + // will require the cascade operations to happen, so we include all such foreign keys. + for idx, childFk := range childFks { + if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { + cFksRequired[idx] = true + } + } + // If we are setting a column to NULL, then we don't need to verify the existance of an + // equivalent row in the parent table, even if this column was part of a foreign key to a parent table. + if sqlparser.IsNull(updateExpr.Expr) { + continue + } + // We add all the possible parent foreign key constraints that need verification that an equivalent row + // exists, given that this column has changed. + for idx, parentFk := range parentFks { + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + pFksRequired[idx] = true + } + } + } + // For the parent foreign keys, if any of the columns part of the fk is set to NULL, + // then, we don't care for the existance of an equivalent row in the parent table. + for idx, parentFk := range parentFks { + for _, updateExpr := range updateExprs { + if !sqlparser.IsNull(updateExpr.Expr) { + continue + } + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + pFksRequired[idx] = false + } + } + } + // Get the filtered lists and return them. + var pFksNeedsHandling []vindexes.ParentFKInfo + var cFksNeedsHandling []vindexes.ChildFKInfo + for idx, parentFk := range parentFks { + if pFksRequired[idx] { + pFksNeedsHandling = append(pFksNeedsHandling, parentFk) + } + } + for idx, childFk := range childFks { + if cFksRequired[idx] { + cFksNeedsHandling = append(cFksNeedsHandling, childFk) + } + } + return pFksNeedsHandling, cFksNeedsHandling +} + +func createFKCascadeOp(ctx *plancontext.PlanningContext, parentOp ops.Operator, updStmt *sqlparser.Update, childFks []vindexes.ChildFKInfo) (ops.Operator, error) { + // We only support simple expressions in update queries with cascade. + for _, updateExpr := range updStmt.Exprs { + switch updateExpr.Expr.(type) { + case *sqlparser.Argument, *sqlparser.NullVal, sqlparser.BoolVal, *sqlparser.Literal: + default: + return nil, vterrors.VT12001("foreign keys management at vitess with non-literal values") + } + } + + var fkChildren []*FkChild + var selectExprs []sqlparser.SelectExpr + + for _, fk := range childFks { + // Any RESTRICT type foreign keys that arrive here, + // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. + if isRestrict(fk.OnUpdate) { + return nil, vterrors.VT12002() + } + + // We need to select all the parent columns for the foreign key constraint, to use in the update of the child table. + cols, exprs := selectParentColumns(fk, len(selectExprs)) + selectExprs = append(selectExprs, exprs...) + + fkChild, err := createFkChildForUpdate(ctx, fk, updStmt, cols) + if err != nil { + return nil, err + } + fkChildren = append(fkChildren, fkChild) + } + + selectionOp, err := createSelectionOp(ctx, selectExprs, updStmt.TableExprs, updStmt.Where) + if err != nil { + return nil, err + } + + return &FkCascade{ + Selection: selectionOp, + Children: fkChildren, + Parent: parentOp, + }, nil +} + +// createFkChildForUpdate creates the update query operator for the child table based on the foreign key constraints. +func createFkChildForUpdate(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, updStmt *sqlparser.Update, cols []int) (*FkChild, error) { + // Reserve a bind variable name + bvName := ctx.ReservedVars.ReserveVariable(foriegnKeyContraintValues) + + // Create child update operator + // Create a ValTuple of child column names + var valTuple sqlparser.ValTuple + for _, column := range fk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColName(column.String())) + } + + // Create a comparison expression for WHERE clause + compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) + + // Populate the update expressions and the where clause for the child update query based on the foreign key constraint type. + var childWhereExpr sqlparser.Expr = compExpr + var childUpdateExprs sqlparser.UpdateExprs + + switch fk.OnUpdate { + case sqlparser.Cascade: + // For CASCADE type constraint, the query looks like this - + // `UPDATE SET WHERE IN ()` + + // The update expressions are the same as the update expressions in the parent update query + // with the column names replaced with the child column names. + for _, updateExpr := range updStmt.Exprs { + colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx == -1 { + continue + } + + // The where condition is the same as the comparison expression above + // with the column names replaced with the child column names. + childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(fk.ChildColumns[colIdx].String()), + Expr: updateExpr.Expr, + }) + } + case sqlparser.SetNull: + // For SET NULL type constraint, the query looks like this - + // `UPDATE SET + // WHERE IN () + // [AND NOT IN ()]` + + // For the SET NULL type constraint, we need to set all the child columns to NULL. + for _, column := range fk.ChildColumns { + childUpdateExprs = append(childUpdateExprs, &sqlparser.UpdateExpr{ + Name: sqlparser.NewColName(column.String()), + Expr: &sqlparser.NullVal{}, + }) + } + + // SET NULL cascade should be avoided for the case where the parent columns remains unchanged on the update. + // We need to add a condition to the where clause to handle this case. + // The additional condition looks like [AND NOT IN ()]. + // If any of the parent columns is being set to NULL, then we don't need this condition. + var updateValues sqlparser.ValTuple + colSetToNull := false + for _, updateExpr := range updStmt.Exprs { + colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx >= 0 { + if sqlparser.IsNull(updateExpr.Expr) { + colSetToNull = true + break + } + updateValues = append(updateValues, updateExpr.Expr) + } + } + if !colSetToNull { + childWhereExpr = &sqlparser.AndExpr{ + Left: compExpr, + Right: sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, updateValues, nil), + } + } + case sqlparser.SetDefault: + return nil, vterrors.VT09016() + } + + childStmt := &sqlparser.Update{ + Exprs: childUpdateExprs, + TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, + Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: childWhereExpr}, + } + + childOp, err := createOpFromStmt(ctx, childStmt) + if err != nil { + return nil, err + } + + return &FkChild{ + BVName: bvName, + Cols: cols, + Op: childOp, + }, nil +} diff --git a/go/vt/vtgate/planbuilder/operators/fk_cascade.go b/go/vt/vtgate/planbuilder/operators/fk_cascade.go index e3a2ef3d38d..f4528694c39 100644 --- a/go/vt/vtgate/planbuilder/operators/fk_cascade.go +++ b/go/vt/vtgate/planbuilder/operators/fk_cascade.go @@ -22,6 +22,7 @@ import ( "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" ) +// FkChild is used to represent a foreign key child table operation type FkChild struct { BVName string Cols []int // indexes diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go index 15f3ba245da..032a3e623e6 100644 --- a/go/vt/vtgate/planbuilder/select.go +++ b/go/vt/vtgate/planbuilder/select.go @@ -202,14 +202,14 @@ func newBuildSelectPlan( reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, version querypb.ExecuteOptions_PlannerVersion, -) (logicalPlan, []string, error) { +) (plan logicalPlan, tablesUsed []string, err error) { ctx, err := plancontext.CreatePlanningContext(selStmt, reservedVars, vschema, version) if err != nil { return nil, nil, err } if ks, _ := ctx.SemTable.SingleUnshardedKeyspace(); ks != nil { - plan, tablesUsed, err := selectUnshardedShortcut(ctx, selStmt, ks) + plan, tablesUsed, err = selectUnshardedShortcut(ctx, selStmt, ks) if err != nil { return nil, nil, err } @@ -227,7 +227,7 @@ func newBuildSelectPlan( return nil, nil, err } - plan, err := transformToLogicalPlan(ctx, op) + plan, err = transformToLogicalPlan(ctx, op) if err != nil { return nil, nil, err } diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index 4eafc597f20..a729bc96c0a 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -127,8 +127,7 @@ func columnModified(exprs sqlparser.UpdateExprs, getFks func(expr *sqlparser.Upd return true } } - _, isNull := updateExpr.Expr.(*sqlparser.NullVal) - if isNull { + if sqlparser.IsNull(updateExpr.Expr) { continue } for _, parentFk := range parentFKs {