vindexes.BuildVSchema: prevent one bad sequence from breaking all oth…

…er sequences PR vitessio#4371 made vtgate more permissive about allowing bad data in a vschema without failing requests that only involve properly-configured tables. This PR extends that so a single table entry in the vschema that references a bad sequence won't disable the sequences for all the other tables with valid sequence configs. Signed-off-by: David Weitzman <[email protected]>
tinyspeck · Mar 22, 2019 · 4937f39 · 4937f39
1 parent 5d425ed
commit 4937f39
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 3 deletions.
diff --git a/go/vt/vtgate/vindexes/vschema.go b/go/vt/vtgate/vindexes/vschema.go
@@ -301,7 +301,7 @@ func resolveAutoIncrement(source *vschemapb.SrvVSchema, vschema *VSchema) {
 			seq, err := vschema.findQualified(table.AutoIncrement.Sequence)
 			if err != nil {
 				ksvschema.Error = fmt.Errorf("cannot resolve sequence %s: %v", table.AutoIncrement.Sequence, err)
-				break
+				continue
 			}
 			t.AutoIncrement.Sequence = seq
 		}

diff --git a/go/vt/vtgate/vindexes/vschema_test.go b/go/vt/vtgate/vindexes/vschema_test.go
@@ -1441,6 +1441,13 @@ func TestSequence(t *testing.T) {
 func TestBadSequence(t *testing.T) {
 	bad := vschemapb.SrvVSchema{
 		Keyspaces: map[string]*vschemapb.Keyspace{
+			"unsharded": {
+				Tables: map[string]*vschemapb.Table{
+					"valid_seq": {
+						Type: "sequence",
+					},
+				},
+			},
 			"sharded": {
 				Sharded: true,
 				Vindexes: map[string]*vschemapb.Vindex{
@@ -1458,7 +1465,19 @@ func TestBadSequence(t *testing.T) {
 						},
 						AutoIncrement: &vschemapb.AutoIncrement{
 							Column:   "c1",
-							Sequence: "seq",
+							Sequence: "invalid_seq",
+						},
+					},
+					"t2": {
+						ColumnVindexes: []*vschemapb.ColumnVindex{
+							{
+								Column: "c1",
+								Name:   "stfu1",
+							},
+						},
+						AutoIncrement: &vschemapb.AutoIncrement{
+							Column:   "c1",
+							Sequence: "valid_seq",
 						},
 					},
 				},
@@ -1467,10 +1486,25 @@ func TestBadSequence(t *testing.T) {
 	}
 	got, _ := BuildVSchema(&bad)
 	err := got.Keyspaces["sharded"].Error
-	want := "cannot resolve sequence seq: table seq not found"
+	want := "cannot resolve sequence invalid_seq: table invalid_seq not found"
 	if err == nil || err.Error() != want {
 		t.Errorf("BuildVSchema: %v, want %v", err, want)
 	}
+
+	t1Seq := got.Keyspaces["sharded"].Tables["t1"].AutoIncrement.Sequence
+	if t1Seq != nil {
+		t.Errorf("BuildVSchema: unexpected sequence for table t1: %v", t1Seq)
+	}
+
+	// Verify that a failure to set up a sequence for t1 doesn't prevent setting up
+	// a sequence for t2.
+	t2Seq := got.Keyspaces["sharded"].Tables["t2"].AutoIncrement.Sequence
+	if t2Seq.Name.String() != "valid_seq" {
+		t.Errorf("BuildVSchema: unexpected t2 sequence name. Got: %v. Want: %v",
+			t2Seq.AutoIncrement.Sequence.Name,
+			"valid_seq",
+		)
+	}
 }
 
 func TestBadSequenceName(t *testing.T) {