scop: replace scpb.Expression by catpb.Expression in constraint ops

Only the expression string is needed, and embedding the scpb.Expression makes it impossible to serialize the op in the scplan tests and in the EXPLAIN (DDL) diagrams. Release note: None
cockroachdb · Jan 23, 2023 · 463528e · 463528e
1 parent 8115204
commit 463528e
Show file tree

Hide file tree

Showing 10 changed files with 33 additions and 16 deletions.
diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go b/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go
@@ -58,7 +58,7 @@ func (m *visitor) MakeAbsentCheckConstraintWriteOnly(
 	// is syntactically valid in the builder, so we just need to
 	// enqueue it to the descriptor's mutation slice.
 	ck := &descpb.TableDescriptor_CheckConstraint{
-		Expr:                  string(op.Expr),
+		Expr:                  string(op.CheckExpr),
 		Name:                  tabledesc.ConstraintNamePlaceholder(op.ConstraintID),
 		Validity:              descpb.ConstraintValidity_Validating,
 		ColumnIDs:             op.ColumnIDs,
@@ -353,9 +353,7 @@ func (m *visitor) MakeAbsentUniqueWithoutIndexConstraintWriteOnly(
 		Name:         tabledesc.ConstraintNamePlaceholder(op.ConstraintID),
 		Validity:     descpb.ConstraintValidity_Validating,
 		ConstraintID: op.ConstraintID,
-	}
-	if op.Predicate != nil {
-		uwi.Predicate = string(op.Predicate.Expr)
+		Predicate:    string(op.PartialExpr),
 	}
 	if err = enqueueAddUniqueWithoutIndexConstraintMutation(tbl, uwi); err != nil {
 		return err

diff --git a/pkg/sql/schemachanger/scop/mutation.go b/pkg/sql/schemachanger/scop/mutation.go
@@ -298,10 +298,10 @@ type RemoveCheckConstraint struct {
 // to the table in the WRITE_ONLY state.
 type MakeAbsentCheckConstraintWriteOnly struct {
 	mutationOp
-	TableID      descpb.ID
-	ConstraintID descpb.ConstraintID
-	ColumnIDs    []descpb.ColumnID
-	scpb.Expression
+	TableID               descpb.ID
+	ConstraintID          descpb.ConstraintID
+	ColumnIDs             []descpb.ColumnID
+	CheckExpr             catpb.Expression
 	FromHashShardedColumn bool
 }
 
@@ -375,7 +375,7 @@ type MakeAbsentUniqueWithoutIndexConstraintWriteOnly struct {
 	TableID      descpb.ID
 	ConstraintID descpb.ConstraintID
 	ColumnIDs    []descpb.ColumnID
-	Predicate    *scpb.Expression
+	PartialExpr  catpb.Expression
 }
 
 // MakeValidatedUniqueWithoutIndexConstraintPublic moves a new, validated unique_without_index

diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel
@@ -67,6 +67,7 @@ go_library(
     deps = [
         "//pkg/clusterversion",
         "//pkg/sql/catalog",
+        "//pkg/sql/catalog/catpb",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/schemachanger/rel",

diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go
@@ -25,7 +25,7 @@ func init() {
 						TableID:               this.TableID,
 						ConstraintID:          this.ConstraintID,
 						ColumnIDs:             this.ColumnIDs,
-						Expression:            this.Expression,
+						CheckExpr:             this.Expr,
 						FromHashShardedColumn: this.FromHashShardedColumn,
 					}
 				}),

diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go
@@ -11,6 +11,7 @@
 package opgen
 
 import (
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
 )
@@ -21,11 +22,15 @@ func init() {
 			scpb.Status_ABSENT,
 			to(scpb.Status_WRITE_ONLY,
 				emit(func(this *scpb.UniqueWithoutIndexConstraint) *scop.MakeAbsentUniqueWithoutIndexConstraintWriteOnly {
+					var partialExpr catpb.Expression
+					if this.Predicate != nil {
+						partialExpr = this.Predicate.Expr
+					}
 					return &scop.MakeAbsentUniqueWithoutIndexConstraintWriteOnly{
 						TableID:      this.TableID,
 						ConstraintID: this.ConstraintID,
 						ColumnIDs:    this.ColumnIDs,
-						Predicate:    this.Predicate,
+						PartialExpr:  partialExpr,
 					}
 				}),
 				emit(func(this *scpb.UniqueWithoutIndexConstraint) *scop.UpdateTableBackReferencesInTypes {

diff --git a/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check b/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check
@@ -10,7 +10,11 @@ StatementPhase stage 1 of 1 with 1 MutationType op
     [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, PUBLIC], ABSENT] -> WRITE_ONLY
   ops:
     *scop.MakeAbsentCheckConstraintWriteOnly
-      {}
+      CheckExpr: i > 0:::INT8
+      ColumnIDs:
+      - 1
+      ConstraintID: 2
+      TableID: 104
 PreCommitPhase stage 1 of 1 with 2 MutationType ops
   transitions:
   ops:

diff --git a/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_vanilla b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_vanilla
@@ -11,7 +11,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC
  │         ├── 1 element transitioning toward PUBLIC
  │         │    └── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}
  │         └── 1 Mutation operation
- │              └── MakeAbsentCheckConstraintWriteOnly
+ │              └── MakeAbsentCheckConstraintWriteOnly {"CheckExpr":"i \u003e 0:::INT8","ConstraintID":2,"TableID":104}
  ├── PreCommitPhase
  │    └── Stage 1 of 1 in PreCommitPhase
  │         └── 2 Mutation operations

diff --git a/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_with_seq_and_udt b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_with_seq_and_udt
@@ -12,7 +12,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC
  │         ├── 1 element transitioning toward PUBLIC
  │         │    └── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 107, ReferencedTypeIDs: [105 106], IndexID: 0, ConstraintID: 2, ReferencedSequenceIDs: [104]}
  │         └── 3 Mutation operations
- │              ├── MakeAbsentCheckConstraintWriteOnly
+ │              ├── MakeAbsentCheckConstraintWriteOnly {"CheckExpr":"(i \u003e nextval(104...","ConstraintID":2,"TableID":107}
  │              ├── UpdateTableBackReferencesInTypes {"BackReferencedTableID":107}
  │              └── UpdateBackReferencesInSequences {"BackReferencedTableID":107}
  ├── PreCommitPhase

diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_vanilla b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_vanilla
@@ -22,7 +22,11 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > 0)
 │       └── • 1 Mutation operation
 │           │
 │           └── • MakeAbsentCheckConstraintWriteOnly
-│                 {}
+│                 CheckExpr: i > 0:::INT8
+│                 ColumnIDs:
+│                 - 1
+│                 ConstraintID: 2
+│                 TableID: 104
 │
 ├── • PreCommitPhase
 │   │

diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_with_seq_and_udt b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_with_seq_and_udt
@@ -23,7 +23,12 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > nextval('s') OR j::typ = 'a'
 │       └── • 3 Mutation operations
 │           │
 │           ├── • MakeAbsentCheckConstraintWriteOnly
-│           │     {}
+│           │     CheckExpr: (i > nextval(104:::REGCLASS)) OR (j::@100105 = b'@':::@100105)
+│           │     ColumnIDs:
+│           │     - 1
+│           │     - 2
+│           │     ConstraintID: 2
+│           │     TableID: 107
 │           │
 │           ├── • UpdateTableBackReferencesInTypes
 │           │     BackReferencedTableID: 107