Skip to content

Commit

Permalink
Add cycle detection for foreign keys (#14339)
Browse files Browse the repository at this point in the history
Signed-off-by: Manan Gupta <[email protected]>
  • Loading branch information
GuptaManan100 authored Nov 1, 2023
1 parent 563c921 commit e802cff
Show file tree
Hide file tree
Showing 17 changed files with 544 additions and 12 deletions.
27 changes: 27 additions & 0 deletions go/test/endtoend/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,33 @@ func WaitForAuthoritative(t *testing.T, ks, tbl string, readVSchema func() (*int
}
}

// WaitForKsError waits for the ks error field to be populated and returns it.
func WaitForKsError(t *testing.T, vtgateProcess cluster.VtgateProcess, ks string) string {
timeout := time.After(60 * time.Second)
for {
select {
case <-timeout:
t.Fatalf("schema tracking did not find error in '%s'", ks)
return ""
default:
time.Sleep(1 * time.Second)
res, err := vtgateProcess.ReadVSchema()
require.NoError(t, err, res)
kss := convertToMap(*res)["keyspaces"]
ksMap := convertToMap(convertToMap(kss)[ks])
ksErr, fieldPresent := ksMap["error"]
if !fieldPresent {
break
}
errString, isErr := ksErr.(string)
if !isErr {
break
}
return errString
}
}
}

// WaitForColumn waits for a table's column to be present
func WaitForColumn(t *testing.T, vtgateProcess cluster.VtgateProcess, ks, tbl, col string) error {
timeout := time.After(60 * time.Second)
Expand Down
23 changes: 23 additions & 0 deletions go/test/endtoend/vtgate/foreignkey/fk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -774,3 +774,26 @@ func TestFkScenarios(t *testing.T) {
})
}
}

func TestCyclicFks(t *testing.T) {
mcmp, closer := start(t)
defer closer()
_ = utils.Exec(t, mcmp.VtConn, "use `uks`")

// Create a cyclic foreign key constraint.
utils.Exec(t, mcmp.VtConn, "alter table fk_t10 add constraint test_cyclic_fks foreign key (col) references fk_t12 (col) on delete cascade on update cascade")
defer func() {
utils.Exec(t, mcmp.VtConn, "alter table fk_t10 drop foreign key test_cyclic_fks")
}()

// Wait for schema-tracking to be complete.
ksErr := utils.WaitForKsError(t, clusterInstance.VtgateProcess, unshardedKs)
// Make sure Vschema has the error for cyclic foreign keys.
assert.Contains(t, ksErr, "VT09019: uks has cyclic foreign keys")

// Ensure that the Vitess database is originally empty
ensureDatabaseState(t, mcmp.VtConn, true)

_, err := utils.ExecAllowError(t, mcmp.VtConn, "insert into fk_t10(id, col) values (1, 1)")
require.ErrorContains(t, err, "VT09019: uks has cyclic foreign keys")
}
4 changes: 4 additions & 0 deletions go/test/vschemawrapper/vschema_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ func (vw *VSchemaWrapper) ForeignKeyMode(keyspace string) (vschemapb.Keyspace_Fo
return defaultFkMode, nil
}

func (vw *VSchemaWrapper) KeyspaceError(keyspace string) error {
return nil
}

func (vw *VSchemaWrapper) AllKeyspace() ([]*vindexes.Keyspace, error) {
if vw.Keyspace == nil {
return nil, vterrors.VT13001("keyspace not available")
Expand Down
119 changes: 119 additions & 0 deletions go/vt/graph/graph.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
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 graph

import (
"fmt"
"slices"
"strings"
)

// Graph is a generic graph implementation.
type Graph[C comparable] struct {
edges map[C][]C
}

// NewGraph creates a new graph for the given comparable type.
func NewGraph[C comparable]() *Graph[C] {
return &Graph[C]{
edges: map[C][]C{},
}
}

// AddVertex adds a vertex to the given Graph.
func (gr *Graph[C]) AddVertex(vertex C) {
_, alreadyExists := gr.edges[vertex]
if alreadyExists {
return
}
gr.edges[vertex] = []C{}
}

// AddEdge adds an edge to the given Graph.
// It also makes sure that the vertices are added to the graph if not already present.
func (gr *Graph[C]) AddEdge(start C, end C) {
gr.AddVertex(start)
gr.AddVertex(end)
gr.edges[start] = append(gr.edges[start], end)
}

// PrintGraph is used to print the graph. This is only used for testing.
func (gr *Graph[C]) PrintGraph() string {
adjacencyLists := []string{}
for vertex, edges := range gr.edges {
adjacencyList := fmt.Sprintf("%v -", vertex)
for _, end := range edges {
adjacencyList += fmt.Sprintf(" %v", end)
}
adjacencyLists = append(adjacencyLists, adjacencyList)
}
slices.Sort(adjacencyLists)
return strings.Join(adjacencyLists, "\n")
}

// Empty checks whether the graph is empty or not.
func (gr *Graph[C]) Empty() bool {
return len(gr.edges) == 0
}

// HasCycles checks whether the given graph has a cycle or not.
// We are using a well-known DFS based colouring algorithm to check for cycles.
// Look at https://cp-algorithms.com/graph/finding-cycle.html for more details on the algorithm.
func (gr *Graph[C]) HasCycles() bool {
// If the graph is empty, then we don't need to check anything.
if gr.Empty() {
return false
}
// Initialize the coloring map.
// 0 represents white.
// 1 represents grey.
// 2 represents black.
color := map[C]int{}
for vertex := range gr.edges {
// If any vertex is still white, we initiate a new DFS.
if color[vertex] == 0 {
if gr.hasCyclesDfs(color, vertex) {
return true
}
}
}
return false
}

// hasCyclesDfs is a utility function for checking for cycles in a graph.
// It runs a dfs from the given vertex marking each vertex as grey. During the dfs,
// if we encounter a grey vertex, we know we have a cycle. We mark the visited vertices black
// on finishing the dfs.
func (gr *Graph[C]) hasCyclesDfs(color map[C]int, vertex C) bool {
// Mark the vertex grey.
color[vertex] = 1
// Go over all the edges.
for _, end := range gr.edges[vertex] {
// If we encounter a white vertex, we continue the dfs.
if color[end] == 0 {
if gr.hasCyclesDfs(color, end) {
return true
}
} else if color[end] == 1 {
// We encountered a grey vertex, we have a cycle.
return true
}
}
// Mark the vertex black before finishing
color[vertex] = 2
return false
}
153 changes: 153 additions & 0 deletions go/vt/graph/graph_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/*
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 graph

import (
"testing"

"github.com/stretchr/testify/require"
)

// TestIntegerGraph tests that a graph with Integers can be created and all graph functions work as intended.
func TestIntegerGraph(t *testing.T) {
testcases := []struct {
name string
edges [][2]int
wantedGraph string
wantEmpty bool
wantHasCycles bool
}{
{
name: "empty graph",
edges: nil,
wantedGraph: "",
wantEmpty: true,
wantHasCycles: false,
}, {
name: "non-cyclic graph",
edges: [][2]int{
{1, 2},
{2, 3},
{1, 4},
{2, 5},
{4, 5},
},
wantedGraph: `1 - 2 4
2 - 3 5
3 -
4 - 5
5 -`,
wantEmpty: false,
wantHasCycles: false,
}, {
name: "cyclic graph",
edges: [][2]int{
{1, 2},
{2, 3},
{1, 4},
{2, 5},
{4, 5},
{5, 6},
{6, 1},
},
wantedGraph: `1 - 2 4
2 - 3 5
3 -
4 - 5
5 - 6
6 - 1`,
wantEmpty: false,
wantHasCycles: true,
},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
graph := NewGraph[int]()
for _, edge := range tt.edges {
graph.AddEdge(edge[0], edge[1])
}
require.Equal(t, tt.wantedGraph, graph.PrintGraph())
require.Equal(t, tt.wantEmpty, graph.Empty())
require.Equal(t, tt.wantHasCycles, graph.HasCycles())
})
}
}

// TestStringGraph tests that a graph with strings can be created and all graph functions work as intended.
func TestStringGraph(t *testing.T) {
testcases := []struct {
name string
edges [][2]string
wantedGraph string
wantEmpty bool
wantHasCycles bool
}{
{
name: "empty graph",
edges: nil,
wantedGraph: "",
wantEmpty: true,
wantHasCycles: false,
}, {
name: "non-cyclic graph",
edges: [][2]string{
{"A", "B"},
{"B", "C"},
{"A", "D"},
{"B", "E"},
{"D", "E"},
},
wantedGraph: `A - B D
B - C E
C -
D - E
E -`,
wantEmpty: false,
wantHasCycles: false,
}, {
name: "cyclic graph",
edges: [][2]string{
{"A", "B"},
{"B", "C"},
{"A", "D"},
{"B", "E"},
{"D", "E"},
{"E", "F"},
{"F", "A"},
},
wantedGraph: `A - B D
B - C E
C -
D - E
E - F
F - A`,
wantEmpty: false,
wantHasCycles: true,
},
}
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])
}
require.Equal(t, tt.wantedGraph, graph.PrintGraph())
require.Equal(t, tt.wantEmpty, graph.Empty())
require.Equal(t, tt.wantHasCycles, graph.HasCycles())
})
}
}
4 changes: 4 additions & 0 deletions go/vt/schemadiff/semantics.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ func (si *declarativeSchemaInformation) ForeignKeyMode(keyspace string) (vschema
return vschemapb.Keyspace_unmanaged, nil
}

func (si *declarativeSchemaInformation) KeyspaceError(keyspace string) error {
return nil
}

// addTable adds a fake table with an empty column list
func (si *declarativeSchemaInformation) addTable(tableName string) {
tbl := &vindexes.Table{
Expand Down
3 changes: 2 additions & 1 deletion go/vt/vterrors/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ var (
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")
VT09017 = errorWithoutState("VT09017", vtrpcpb.Code_FAILED_PRECONDITION, "%s", "Invalid syntax for the statement type.")
VT09018 = errorWithoutState("VT09017", vtrpcpb.Code_FAILED_PRECONDITION, "%s", "Invalid syntax for the vindex function statement.")
VT09018 = errorWithoutState("VT09018", vtrpcpb.Code_FAILED_PRECONDITION, "%s", "Invalid syntax for the vindex function statement.")
VT09019 = errorWithoutState("VT09019", vtrpcpb.Code_FAILED_PRECONDITION, "%s has cyclic foreign keys", "Vitess doesn't support cyclic foreign keys.")

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/.")

Expand Down
3 changes: 3 additions & 0 deletions go/vt/vtgate/planbuilder/plancontext/vschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ type VSchema interface {
// ForeignKeyMode returns the foreign_key flag value
ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyMode, error)

// KeyspaceError returns any error in the keyspace vschema.
KeyspaceError(keyspace string) error

// GetVSchema returns the latest cached vindexes.VSchema
GetVSchema() *vindexes.VSchema

Expand Down
Loading

0 comments on commit e802cff

Please sign in to comment.