sql/schemachanger: relax rules for setting constraint names

Previously, the rules for adding and removing constraint names were strict, which could lead to mixed version compatibility issues. Specifically, plans on the 23.1 branch could have names set in later stages violating a rule that said a constraint should be public/non-public with the name. To address this, this patch will relax these rules to have Precedence instead of SameStagePrecedence Fixes: cockroachdb#100908 Release note: None
THardy98 · Apr 11, 2023 · af1d9cf · af1d9cf
1 parent b36c119
commit af1d9cf
Show file tree

Hide file tree

Showing 46 changed files with 426 additions and 302 deletions.
diff --git a/pkg/cli/testdata/declarative-rules/deprules b/pkg/cli/testdata/declarative-rules/deprules
@@ -295,7 +295,7 @@ deprules
     - joinTargetNode($next, $next-Target, $next-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -308,7 +308,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -321,7 +321,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -335,7 +335,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -3402,7 +3402,7 @@ deprules
     - joinTargetNode($dependent, $dependent-Target, $dependent-Node)
 - name: simple constraint visible before name
   from: simple-constraint-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-name-Node
   query:
     - $simple-constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.ColumnNotNull']

diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_constraint.go b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_constraint.go
@@ -52,7 +52,7 @@ func init() {
 	// we won't have the correct message inside errors.
 	registerDepRule(
 		"simple constraint visible before name",
-		scgraph.SameStagePrecedence,
+		scgraph.Precedence,
 		"simple-constraint", "constraint-name",
 		func(from, to NodeVars) rel.Clauses {
 			return rel.Clauses{

diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_constraint.go b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_constraint.go
@@ -72,7 +72,7 @@ func init() {
 	// longer visible.
 	registerDepRuleForDrop(
 		"Constraint should be hidden before name",
-		scgraph.SameStagePrecedence,
+		scgraph.Precedence,
 		"constraint-name", "constraint",
 		scpb.Status_ABSENT, scpb.Status_ABSENT,
 		func(from, to NodeVars) rel.Clauses {

diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules
@@ -292,7 +292,7 @@ deprules
     - joinTargetNode($next, $next-Target, $next-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -305,7 +305,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -318,7 +318,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -332,7 +332,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -3399,7 +3399,7 @@ deprules
     - joinTargetNode($dependent, $dependent-Target, $dependent-Node)
 - name: simple constraint visible before name
   from: simple-constraint-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-name-Node
   query:
     - $simple-constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.ColumnNotNull']
@@ -3774,7 +3774,7 @@ deprules
     - joinTargetNode($next, $next-Target, $next-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -3787,7 +3787,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -3800,7 +3800,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -3814,7 +3814,7 @@ deprules
     - joinTargetNode($constraint, $constraint-Target, $constraint-Node)
 - name: Constraint should be hidden before name
   from: constraint-name-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-Node
   query:
     - $constraint-name[Type] = '*scpb.ConstraintWithoutIndexName'
@@ -6881,7 +6881,7 @@ deprules
     - joinTargetNode($dependent, $dependent-Target, $dependent-Node)
 - name: simple constraint visible before name
   from: simple-constraint-Node
-  kind: SameStagePrecedence
+  kind: Precedence
   to: constraint-name-Node
   query:
     - $simple-constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.ColumnNotNull']

diff --git a/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check b/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check
@@ -98,5 +98,5 @@ ALTER TABLE t ADD CHECK (i > 0)
   rule: CheckConstraint transitions to PUBLIC uphold 2-version invariant: WRITE_ONLY->VALIDATED
 - from: [CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, WRITE_ONLY]
   to:   [ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2}, PUBLIC]
-  kind: SameStagePrecedence
+  kind: Precedence
   rule: simple constraint visible before name
diff --git a/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check_udf b/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check_udf
@@ -118,5 +118,5 @@ ALTER TABLE t ADD CONSTRAINT check_b CHECK (f(b) > 1);
   rule: CheckConstraint transitions to PUBLIC uphold 2-version invariant: WRITE_ONLY->VALIDATED
 - from: [CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, WRITE_ONLY]
   to:   [ConstraintWithoutIndexName:{DescID: 104, Name: check_b, ConstraintID: 2}, PUBLIC]
-  kind: SameStagePrecedence
+  kind: Precedence
   rule: simple constraint visible before name
diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_index b/pkg/sql/schemachanger/scplan/testdata/drop_index
@@ -366,13 +366,14 @@ DROP INDEX idx2 CASCADE
 ops
 DROP INDEX idx3 CASCADE
 ----
-StatementPhase stage 1 of 1 with 5 MutationType ops
+StatementPhase stage 1 of 1 with 6 MutationType ops
   transitions:
     [[Column:{DescID: 104, ColumnID: 5}, ABSENT], PUBLIC] -> WRITE_ONLY
     [[ColumnName:{DescID: 104, Name: crdb_internal_i_shard_16, ColumnID: 5}, ABSENT], PUBLIC] -> ABSENT
     [[ColumnNotNull:{DescID: 104, ColumnID: 5, IndexID: 0}, ABSENT], PUBLIC] -> VALIDATED
     [[SecondaryIndex:{DescID: 104, IndexID: 6, ConstraintID: 0}, ABSENT], PUBLIC] -> VALIDATED
     [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, ABSENT], PUBLIC] -> VALIDATED
+    [[ConstraintWithoutIndexName:{DescID: 104, Name: check_crdb_internal_i_shard_16, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT
   ops:
     *scop.MakePublicColumnNotNullValidated
       ColumnID: 5
@@ -383,6 +384,10 @@ StatementPhase stage 1 of 1 with 5 MutationType ops
     *scop.MakePublicCheckConstraintValidated
       ConstraintID: 2
       TableID: 104
+    *scop.SetConstraintName
+      ConstraintID: 2
+      Name: crdb_internal_constraint_2_name_placeholder
+      TableID: 104
     *scop.MakePublicColumnWriteOnly
       ColumnID: 5
       TableID: 104
@@ -397,16 +402,18 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op
     [[ColumnNotNull:{DescID: 104, ColumnID: 5, IndexID: 0}, ABSENT], VALIDATED] -> PUBLIC
     [[SecondaryIndex:{DescID: 104, IndexID: 6, ConstraintID: 0}, ABSENT], VALIDATED] -> PUBLIC
     [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, ABSENT], VALIDATED] -> PUBLIC
+    [[ConstraintWithoutIndexName:{DescID: 104, Name: check_crdb_internal_i_shard_16, ConstraintID: 2}, ABSENT], ABSENT] -> PUBLIC
   ops:
     *scop.UndoAllInTxnImmediateMutationOpSideEffects
       {}
-PreCommitPhase stage 2 of 2 with 7 MutationType ops
+PreCommitPhase stage 2 of 2 with 8 MutationType ops
   transitions:
     [[Column:{DescID: 104, ColumnID: 5}, ABSENT], PUBLIC] -> WRITE_ONLY
     [[ColumnName:{DescID: 104, Name: crdb_internal_i_shard_16, ColumnID: 5}, ABSENT], PUBLIC] -> ABSENT
     [[ColumnNotNull:{DescID: 104, ColumnID: 5, IndexID: 0}, ABSENT], PUBLIC] -> VALIDATED
     [[SecondaryIndex:{DescID: 104, IndexID: 6, ConstraintID: 0}, ABSENT], PUBLIC] -> VALIDATED
     [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, ABSENT], PUBLIC] -> VALIDATED
+    [[ConstraintWithoutIndexName:{DescID: 104, Name: check_crdb_internal_i_shard_16, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT
   ops:
     *scop.MakePublicColumnNotNullValidated
       ColumnID: 5
@@ -417,6 +424,10 @@ PreCommitPhase stage 2 of 2 with 7 MutationType ops
     *scop.MakePublicCheckConstraintValidated
       ConstraintID: 2
       TableID: 104
+    *scop.SetConstraintName
+      ConstraintID: 2
+      Name: crdb_internal_constraint_2_name_placeholder
+      TableID: 104
     *scop.MakePublicColumnWriteOnly
       ColumnID: 5
       TableID: 104
@@ -434,12 +445,12 @@ PreCommitPhase stage 2 of 2 with 7 MutationType ops
       - 104
       JobID: 1
       NonCancelable: true
-      RunningStatus: PostCommitNonRevertiblePhase stage 1 of 2 with 9 MutationType ops pending
+      RunningStatus: PostCommitNonRevertiblePhase stage 1 of 2 with 8 MutationType ops pending
       Statements:
       - statement: DROP INDEX idx3 CASCADE
         redactedstatement: DROP INDEX ‹defaultdb›.public.‹t1›@‹idx3› CASCADE
         statementtag: DROP INDEX
-PostCommitNonRevertiblePhase stage 1 of 2 with 11 MutationType ops
+PostCommitNonRevertiblePhase stage 1 of 2 with 10 MutationType ops
   transitions:
     [[Column:{DescID: 104, ColumnID: 5}, ABSENT], WRITE_ONLY] -> DELETE_ONLY
     [[ColumnNotNull:{DescID: 104, ColumnID: 5, IndexID: 0}, ABSENT], VALIDATED] -> ABSENT
@@ -449,14 +460,12 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 11 MutationType ops
     [[SecondaryIndex:{DescID: 104, IndexID: 6, ConstraintID: 0}, ABSENT], VALIDATED] -> DELETE_ONLY
     [[IndexName:{DescID: 104, Name: idx3, IndexID: 6}, ABSENT], PUBLIC] -> ABSENT
     [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, ABSENT], VALIDATED] -> ABSENT
-    [[ConstraintWithoutIndexName:{DescID: 104, Name: check_crdb_internal_i_shard_16, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT
   ops:
     *scop.RemoveColumnNotNull
       ColumnID: 5
       TableID: 104
-    *scop.SetConstraintName
+    *scop.RemoveCheckConstraint
       ConstraintID: 2
-      Name: crdb_internal_constraint_2_name_placeholder
       TableID: 104
     *scop.MakeWriteOnlyColumnDeleteOnly
       ColumnID: 5
@@ -468,9 +477,6 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 11 MutationType ops
       IndexID: 6
       Name: crdb_internal_index_6_name_placeholder
       TableID: 104
-    *scop.RemoveCheckConstraint
-      ConstraintID: 2
-      TableID: 104
     *scop.RemoveColumnFromIndex
       ColumnID: 5
       IndexID: 6
@@ -586,7 +592,7 @@ DROP INDEX idx3 CASCADE
   rules: [dependents removed before column; column type removed right before column when not dropping relation]
 - from: [ConstraintWithoutIndexName:{DescID: 104, Name: check_crdb_internal_i_shard_16, ConstraintID: 2}, ABSENT]
   to:   [CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, ABSENT]
-  kind: SameStagePrecedence
+  kind: Precedence
   rule: Constraint should be hidden before name
 - from: [IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 6}, ABSENT]
   to:   [SecondaryIndex:{DescID: 104, IndexID: 6, ConstraintID: 0}, ABSENT]