Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order foreign keys DML operations to get correct rows affected #15779

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions go/test/endtoend/vtgate/foreignkey/fk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,14 @@ func TestFkQueries(t *testing.T) {
"delete fk_t11 from fk_t11 join fk_t12 using (id) where fk_t11.id = 4",
},
},
{
name: "Foreign key join rows affected",
queries: []string{
"insert /*+ SET_VAR(foreign_key_checks=0) */ into fk_t11 (id, col) values (4, '1'), (5, '3'), (7, '22'), (8, '5'), (9, NULL), (10, '3')",
"insert /*+ SET_VAR(foreign_key_checks=0) */ into fk_t12 (id, col) values (4, '1'), (5, '3'), (7, '22'), (8, '5'), (9, NULL), (10, '3')",
"delete fk_t11, fk_t12 from fk_t11 join fk_t12 using (id) where fk_t11.id = 5",
},
},
}

for _, tt := range testcases {
Expand Down
33 changes: 33 additions & 0 deletions go/vt/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,36 @@ func (gr *Graph[C]) hasCyclesDfs(color map[C]int, vertex C) (bool, []C) {
color[vertex] = black
return false, nil
}

// TopologicalSorting returns a topological sorting of the vertices. This means that the ordering of vertices such that all the edges will
// go from a lower number vertex to a higher numbered one. For a acyclic graph, a valid topological sorting solution always exists.
// More information can be found at https://cp-algorithms.com/graph/topological-sort.html
func (gr *Graph[C]) TopologicalSorting() []C {
// If the graph is empty, then we don't need to check anything.
if gr.Empty() {
return nil
}

visited := map[C]bool{}
var topoSort []C
for _, vertex := range gr.orderedVertices {
// If any vertex is still white, we initiate a new DFS.
if !visited[vertex] {
topoSort = gr.topoSortDfs(visited, topoSort, vertex)
}
}

slices.Reverse(topoSort)
return topoSort
}

// topoSortDfs is the DFS implementation used for finding a topological sorting.
func (gr *Graph[C]) topoSortDfs(visited map[C]bool, topoSort []C, vertex C) []C {
visited[vertex] = true
for _, end := range gr.edges[vertex] {
if !visited[end] {
topoSort = gr.topoSortDfs(visited, topoSort, end)
}
}
return append(topoSort, vertex)
}
30 changes: 30 additions & 0 deletions go/vt/graph/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,33 @@ F - A`,
})
}
}

func TestTopologicalSorting(t *testing.T) {
testcases := []struct {
name string
edges [][2]string
topologicalSort []string
}{
{
name: "non-cyclic graph",
edges: [][2]string{
{"A", "B"},
{"B", "C"},
{"A", "D"},
{"B", "E"},
{"D", "E"},
},
topologicalSort: []string{"A", "D", "B", "E", "C"},
},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
graph := NewGraph[string]()
for _, edge := range tt.edges {
graph.AddEdge(edge[0], edge[1])
}
topoSort := graph.TopologicalSorting()
require.EqualValues(t, tt.topologicalSort, topoSort)
})
}
}
7 changes: 7 additions & 0 deletions go/vt/vtgate/planbuilder/operators/dml_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ type dmlOp struct {
func sortDmlOps(dmlOps []dmlOp) []dmlOp {
sort.Slice(dmlOps, func(i, j int) bool {
a, b := dmlOps[i], dmlOps[j]
// We want the dml orders to happen in a specific order so that multi table dml queries
// return the correct rows updated. For example in a multi table delete on a pair of tables
// related by foreign keys, we want the delete to happen on the child table first and then the parent
// so that the cascade operations don't affect the rows affected.
if a.vTbl.FkOrder != b.vTbl.FkOrder {
return a.vTbl.FkOrder > b.vTbl.FkOrder
}
// Get the first Vindex of a and b, if available
aVdx, bVdx := getFirstVindex(a.vTbl), getFirstVindex(b.vTbl)

Expand Down
33 changes: 32 additions & 1 deletion go/vt/vtgate/planbuilder/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"vitess.io/vitess/go/test/utils"
"vitess.io/vitess/go/test/vschemawrapper"
"vitess.io/vitess/go/vt/key"
"vitess.io/vitess/go/vt/log"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/sidecardb"
"vitess.io/vitess/go/vt/sqlparser"
Expand Down Expand Up @@ -231,7 +232,37 @@ func setFks(t *testing.T, vschema *vindexes.VSchema) {
addPKs(t, vschema, "unsharded_fk_allow", []string{"u_tbl1", "u_tbl2", "u_tbl3", "u_tbl4", "u_tbl5", "u_tbl6", "u_tbl7", "u_tbl8", "u_tbl9", "u_tbl10", "u_tbl11",
"u_multicol_tbl1", "u_multicol_tbl2", "u_multicol_tbl3"})
}

setFkOrderForFkTables(vschema)
}

func setFkOrderForFkTables(vschema *vindexes.VSchema) {
// We aren't using the topological sorting algorithm here in the test
// because it leads to ambiguity in test output since the topological sort can lead to
// multiple different ordering, all of them equally valid.
fkOrder := map[string]int{
"u_tbl2": 11,
"u_tbl7": 9,
"u_tbl": 0,
"u_tbl10": 7,
"u_tbl3": 12,
"u_tbl6": 5,
"u_tbl9": 4,
"u_tbl5": 0,
"u_tbl8": 6,
"u_tbl1": 3,
"u_multicol_tbl2": 2,
"u_multicol_tbl3": 0,
"u_tbl4": 10,
"u_tbl11": 8,
"u_multicol_tbl1": 1,
}

for tblName, ord := range fkOrder {
vschema.Keyspaces["unsharded_fk_allow"].Tables[tblName].FkOrder = ord
}
for _, table := range vschema.Keyspaces["unsharded_fk_allow"].Tables {
log.Errorf("%s: %d", table.Name.String(), table.FkOrder)
}
}

func addPKs(t *testing.T, vschema *vindexes.VSchema, ks string, tbls []string) {
Expand Down
89 changes: 89 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,95 @@
]
}
},
{
"comment": "Multi table delete related by foreign keys, ordering of the deletes is important for rows affected",
"query": "delete u_tbl7, u_tbl4 from u_tbl7 join u_tbl4 on u_tbl4.col4 = u_tbl7.col7 where u_tbl4.col4 = 5",
"plan": {
"QueryType": "DELETE",
"Original": "delete u_tbl7, u_tbl4 from u_tbl7 join u_tbl4 on u_tbl4.col4 = u_tbl7.col7 where u_tbl4.col4 = 5",
"Instructions": {
"OperatorType": "DMLWithInput",
"TargetTabletType": "PRIMARY",
"Offset": [
"0:[0]",
"1:[1]"
],
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Unsharded",
"Keyspace": {
"Name": "unsharded_fk_allow",
"Sharded": false
},
"FieldQuery": "select u_tbl4.id, u_tbl7.id from u_tbl7, u_tbl4 where 1 != 1",
"Query": "select u_tbl4.id, u_tbl7.id from u_tbl7, u_tbl4 where u_tbl4.col4 = 5 and u_tbl4.col4 = u_tbl7.col7 for update",
"Table": "u_tbl4, u_tbl7"
},
{
"OperatorType": "Delete",
"Variant": "Unsharded",
"Keyspace": {
"Name": "unsharded_fk_allow",
"Sharded": false
},
"TargetTabletType": "PRIMARY",
"Query": "delete from u_tbl4 where u_tbl4.id in ::dml_vals",
"Table": "u_tbl4"
},
{
"OperatorType": "FkCascade",
"Inputs": [
{
"InputName": "Selection",
"OperatorType": "Route",
"Variant": "Unsharded",
"Keyspace": {
"Name": "unsharded_fk_allow",
"Sharded": false
},
"FieldQuery": "select u_tbl7.col7 from u_tbl7 where 1 != 1",
"Query": "select u_tbl7.col7 from u_tbl7 where u_tbl7.id in ::dml_vals for update",
"Table": "u_tbl7"
},
{
"InputName": "CascadeChild-1",
"OperatorType": "Delete",
"Variant": "Unsharded",
"Keyspace": {
"Name": "unsharded_fk_allow",
"Sharded": false
},
"TargetTabletType": "PRIMARY",
"BvName": "fkc_vals",
"Cols": [
0
],
"Query": "delete from u_tbl4 where (col4) in ::fkc_vals",
"Table": "u_tbl4"
},
{
"InputName": "Parent",
"OperatorType": "Delete",
"Variant": "Unsharded",
"Keyspace": {
"Name": "unsharded_fk_allow",
"Sharded": false
},
"TargetTabletType": "PRIMARY",
"Query": "delete from u_tbl7 where u_tbl7.id in ::dml_vals",
"Table": "u_tbl7"
}
]
}
]
},
"TablesUsed": [
"unsharded_fk_allow.u_tbl4",
"unsharded_fk_allow.u_tbl7"
]
}
},
{
"comment": "update in a table with non-literal value - set null",
"query": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1",
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/vindexes/vschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ type Table struct {
// MySQL error message: ERROR 3756 (HY000): The primary key cannot be a functional index
PrimaryKey sqlparser.Columns `json:"primary_key,omitempty"`
UniqueKeys []sqlparser.Exprs `json:"unique_keys,omitempty"`

// FkOrder is used to order the tables in a keyspace such that all CASCADE foreign keys have child tables coming before parent tables.
FkOrder int `json:"-"`
}

// GetTableName gets the sqlparser.TableName for the vindex Table.
Expand Down
27 changes: 22 additions & 5 deletions go/vt/vtgate/vschema_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ func (vm *VSchemaManager) buildAndEnhanceVSchema(v *vschemapb.SrvVSchema) *vinde
vm.updateFromSchema(vschema)
// We mark the keyspaces that have foreign key management in Vitess and have cyclic foreign keys
// to have an error. This makes all queries against them to fail.
markErrorIfCyclesInFk(vschema)
// We also order the tables so that that all CASCADE foreign keys have child tables coming before parent tables.
markErrorIfCyclesInFkAndOrderTables(vschema)
}
return vschema
}
Expand Down Expand Up @@ -273,7 +274,7 @@ func (vm *VSchemaManager) updateUDFsInfo(ks *vindexes.KeyspaceSchema, ksName str
ks.AggregateUDFs = vm.schema.UDFs(ksName)
}

func markErrorIfCyclesInFk(vschema *vindexes.VSchema) {
func markErrorIfCyclesInFkAndOrderTables(vschema *vindexes.VSchema) {
for ksName, ks := range vschema.Keyspaces {
// Only check cyclic foreign keys for keyspaces that have
// foreign keys managed in Vitess.
Expand All @@ -287,6 +288,7 @@ func markErrorIfCyclesInFk(vschema *vindexes.VSchema) {
3. ON DELETE CASCADE: This is a special case wherein a deletion on the parent table will affect all the columns in the child table irrespective of the columns involved in the foreign key! So, we'll add an edge from all the columns in the parent side of the foreign key to all the columns of the child table.
*/
g := graph.NewGraph[string]()
vertexToTable := make(map[string]*vindexes.Table)
for _, table := range ks.Tables {
for _, cfk := range table.ChildForeignKeys {
// Check for case 1.
Expand All @@ -298,18 +300,24 @@ func markErrorIfCyclesInFk(vschema *vindexes.VSchema) {
var parentVertices []string
var childVertices []string
for _, column := range cfk.ParentColumns {
parentVertices = append(parentVertices, sqlparser.String(sqlparser.NewColNameWithQualifier(column.String(), table.GetTableName())))
vertex := sqlparser.String(sqlparser.NewColNameWithQualifier(column.String(), table.GetTableName()))
vertexToTable[vertex] = table
parentVertices = append(parentVertices, vertex)
}

// Check for case 3.
if cfk.OnDelete.IsCascade() {
for _, column := range childTable.Columns {
childVertices = append(childVertices, sqlparser.String(sqlparser.NewColNameWithQualifier(column.Name.String(), childTable.GetTableName())))
vertex := sqlparser.String(sqlparser.NewColNameWithQualifier(column.Name.String(), childTable.GetTableName()))
vertexToTable[vertex] = childTable
childVertices = append(childVertices, vertex)
}
} else {
// Case 2.
for _, column := range cfk.ChildColumns {
childVertices = append(childVertices, sqlparser.String(sqlparser.NewColNameWithQualifier(column.String(), childTable.GetTableName())))
vertex := sqlparser.String(sqlparser.NewColNameWithQualifier(column.String(), childTable.GetTableName()))
vertexToTable[vertex] = childTable
childVertices = append(childVertices, vertex)
}
}
addCrossEdges(g, parentVertices, childVertices)
Expand All @@ -318,6 +326,15 @@ func markErrorIfCyclesInFk(vschema *vindexes.VSchema) {
hasCycle, cycle := g.HasCycles()
if hasCycle {
ks.Error = vterrors.VT09019(ksName, cycle)
continue
}
idx := 1
for _, vertex := range g.TopologicalSorting() {
tbl := vertexToTable[vertex]
if tbl.FkOrder == 0 {
tbl.FkOrder = idx
idx++
}
}
}
}
Expand Down
Loading
Loading