Merge #82043 #82256

82043: storage: fix incorrect condition in `mvccPutInternal` intent check r=jbowens a=erikgrinaker Introduced in a3e608d. Release note: None 82256: sql/schemachanger/scplan/internal/rules: rework dep rules r=ajwerner a=ajwerner The dep rules helpers were brittle. Instead, make it easier to construct rel queries directly. Release note: None Co-authored-by: Erik Grinaker <[email protected]> Co-authored-by: Andrew Werner <[email protected]>
cockroachdb · Jun 6, 2022 · 1860eaa · 1860eaa
3 parents ea499a5 + f18adb3 + e5ee890
commit 1860eaa
Show file tree

Hide file tree

Showing 13 changed files with 1,659 additions and 1,521 deletions.
diff --git a/pkg/sql/logictest/testdata/logic_test/new_schema_changer b/pkg/sql/logictest/testdata/logic_test/new_schema_changer
@@ -36,7 +36,7 @@ EXPLAIN (DDL, VERBOSE) ALTER TABLE foo ADD COLUMN j INT
 │       │   │   │ ABSENT → PUBLIC
 │       │   │   │
 │       │   │   └── • SameStagePrecedence dependency from PUBLIC ColumnName:{DescID: 106, Name: j, ColumnID: 2}
-│       │   │         rule: "column named right before column becomes public"
+│       │   │         rule: "column named right before column type becomes public"
 │       │   │
 │       │   └── • PrimaryIndex:{DescID: 106, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3}
 │       │       │ ABSENT → BACKFILL_ONLY
@@ -429,6 +429,7 @@ EXPLAIN (DDL, VERBOSE) ALTER TABLE foo ADD COLUMN j INT
 
 
 
+
 statement ok
 ALTER TABLE foo ADD COLUMN j INT
 

diff --git a/pkg/sql/schemachanger/rel/database.go b/pkg/sql/schemachanger/rel/database.go
@@ -261,6 +261,12 @@ func (t *Database) chooseIndex(
 		if best >= 0 && overlap.len() <= bestOverlap.len() {
 			continue
 		}
+		// Only allow queries to proceed with no index overlap if this is the
+		// zero-attribute index, which implies the database creator accepts bad
+		// query plans.
+		if overlap == 0 && len(dims[i].attrs) > 0 {
+			continue
+		}
 		if !dims[i].exists.isContainedIn(m.union(hasAttrs)) {
 			continue
 		}

diff --git a/pkg/sql/schemachanger/rel/internal/cyclegraphtest/tests.go b/pkg/sql/schemachanger/rel/internal/cyclegraphtest/tests.go
@@ -66,6 +66,7 @@ var (
 						{container1, message1},
 						{container2, message2},
 					},
+					UnsatisfiableIndexes: []int{1},
 				},
 				{
 					Name: "oneOf member",

diff --git a/pkg/sql/schemachanger/rel/internal/entitynodetest/tests.go b/pkg/sql/schemachanger/rel/internal/entitynodetest/tests.go
@@ -100,6 +100,7 @@ var (
 				{
 					Name: "a fields",
 					Query: rel.Clauses{
+						v("a").Type((*entity)(nil)),
 						v("a").AttrEq(i16, int16(1)),
 						v("a").AttrEqVar(i8, "ai8"),
 						v("a").AttrEqVar(pi8, "api8"),
@@ -109,7 +110,7 @@ var (
 					Results: [][]interface{}{
 						{a, int8(1), int8(1)},
 					},
-					UnsatisfiableIndexes: []int{2},
+					UnsatisfiableIndexes: []int{1, 2, 3, 5, 6},
 				},
 				{
 					Name: "a-c-b join",
@@ -125,7 +126,7 @@ var (
 					Results: [][]interface{}{
 						{a, b, c},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{2, 3, 4, 5, 6},
 				},
 				{
 					Name: "nil values don't show up",
@@ -137,7 +138,7 @@ var (
 					Results: [][]interface{}{
 						{a},
 					},
-					UnsatisfiableIndexes: []int{2},
+					UnsatisfiableIndexes: []int{2, 4, 5, 6},
 				},
 				{
 					Name: "nil values don't show up, scalar pointers same as pointers",
@@ -149,7 +150,7 @@ var (
 					Results: [][]interface{}{
 						{a},
 					},
-					UnsatisfiableIndexes: []int{2},
+					UnsatisfiableIndexes: []int{2, 4, 5, 6},
 				},
 				{
 					Name: "list all the values",
@@ -163,7 +164,7 @@ var (
 						{b, int8(2)},
 						{c, int8(2)},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "list all the values with type constraint",
@@ -178,6 +179,7 @@ var (
 						{b, int8(2)},
 						{c, int8(2)},
 					},
+					UnsatisfiableIndexes: []int{1, 2, 3, 5, 6},
 				},
 				{
 					Name: "nodes with elements where i8=2",
@@ -192,7 +194,7 @@ var (
 						{nb, b},
 						{nc, c},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{2, 3, 4, 5, 6},
 				},
 				{
 					Name: "list all the i8 values",
@@ -209,6 +211,7 @@ var (
 						{int8(2)},
 						{int8(2)},
 					},
+					UnsatisfiableIndexes: []int{1, 2, 3, 5, 6},
 				},
 				{
 					Name: "use a filter",
@@ -223,7 +226,7 @@ var (
 					Results: [][]interface{}{
 						{a},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "types of all the entities",
@@ -240,7 +243,7 @@ var (
 						{nb, reflect.TypeOf((*node)(nil))},
 						{nc, reflect.TypeOf((*node)(nil))},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "nodes by type",
@@ -255,7 +258,7 @@ var (
 					Results: [][]interface{}{
 						{na, nb, nc, a},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "list nodes",
@@ -269,6 +272,7 @@ var (
 						{nb},
 						{nc},
 					},
+					UnsatisfiableIndexes: []int{1, 2, 3, 5, 6},
 				},
 				{
 					Name: "basic any",
@@ -285,6 +289,7 @@ var (
 						{nb},
 						{nc},
 					},
+					UnsatisfiableIndexes: []int{1, 2, 3, 5, 6},
 				},
 				{
 					Name: "self eq value",
@@ -296,7 +301,7 @@ var (
 					Results: [][]interface{}{
 						{c},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3},
 				},
 				{
 					Name: "contradiction due to missing attribute",
@@ -307,7 +312,7 @@ var (
 					Entities:             []v{"entity"},
 					ResVars:              []v{"entity", "pi8"},
 					Results:              [][]interface{}{},
-					UnsatisfiableIndexes: []int{2},
+					UnsatisfiableIndexes: []int{1, 2, 3},
 				},
 				{
 					Name: "self eq self",
@@ -319,7 +324,7 @@ var (
 					Results: [][]interface{}{
 						{a}, {b}, {c}, {na}, {nb}, {nc},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "variable type mismatch",
@@ -345,7 +350,7 @@ var (
 						{na, a, na, a},
 						{na, a, nc, c},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "entity bound via variable with ne filter",
@@ -365,7 +370,7 @@ var (
 					Results: [][]interface{}{
 						{na, a, nc, c},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "any value type mismatch",
@@ -385,7 +390,7 @@ var (
 					Entities:             []v{"e"},
 					ResVars:              []v{"e", "i8"},
 					Results:              [][]interface{}{},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 				{
 					Name: "pointer scalar values any",
@@ -397,7 +402,7 @@ var (
 					Results: [][]interface{}{
 						{a}, {b}, {c},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{2, 3, 4, 5, 6},
 				},
 				{
 					Name: "pointer scalar values",
@@ -409,7 +414,7 @@ var (
 					Results: [][]interface{}{
 						{a},
 					},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{2, 3, 4, 5, 6},
 				},
 				{
 					Name: "nil pointer scalar values any",
@@ -433,7 +438,7 @@ var (
 					Entities:             []v{"e"},
 					ResVars:              []v{"e"},
 					Results:              [][]interface{}{},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{2, 3, 4, 5, 6},
 				},
 				{
 					Name: "any clause no match on variable eq",
@@ -444,7 +449,7 @@ var (
 					Entities:             []v{"e"},
 					ResVars:              []v{"e", "i8"},
 					Results:              [][]interface{}{},
-					UnsatisfiableIndexes: []int{2, 3},
+					UnsatisfiableIndexes: []int{1, 2, 3, 4, 5, 6},
 				},
 			},
 		},

diff --git a/pkg/sql/schemachanger/rel/reltest/database.go b/pkg/sql/schemachanger/rel/reltest/database.go
@@ -64,7 +64,7 @@ type QueryTest struct {
 
 func (tc DatabaseTest) run(t *testing.T, s Suite) {
 	for i, databaseIndexes := range tc.databaseIndexes() {
-		t.Run(fmt.Sprintf("%v", databaseIndexes), func(t *testing.T) {
+		t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
 			db, err := rel.NewDatabase(s.Schema, databaseIndexes...)
 			require.NoError(t, err)
 			for _, k := range tc.Data {

diff --git a/pkg/sql/schemachanger/rel/testdata/cyclegraph b/pkg/sql/schemachanger/rel/testdata/cyclegraph
@@ -24,6 +24,7 @@ queries:
             results:
                 - [container1, message1]
                 - [container2, message2]
+            unsatisfiableIndexes: [1]
         oneOf member:
             query:
                 - $c[s] = struct1(message1)