sql/schemachanger: create partial index with inserts can fail

Previously, the temporary index created during CREATE INDEX with
partial indexes did not correctly copy over the predicate. If an
insert and a CREATE INDEX (for partial indexes) were run
concurrently, the CREATE INDEX could fail since the temporary
index would contain extra rows. Eventually, validation would fail.
This patch copies the embedded expression for partial indexes to address
this.

Fixes: #110198

Release note (bug fix): CREATE INDEX for partial indexes could fail with
"ERROR: duplicate key value violates unique constraint" if concurrent inserts
happened simultaneously.

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go

@@ -684,18 +684,21 @@ func makeTempIndexSpec(src indexSpec) indexSpec {
 	}
 	newTempSpec := src.clone()
 	var srcIdx scpb.Index
+	var expr *scpb.Expression
 	isSecondary := false
 	if src.primary != nil {
 		srcIdx = newTempSpec.primary.Index
 	}
 	if src.secondary != nil {
 		srcIdx = newTempSpec.secondary.Index
+		expr = newTempSpec.secondary.EmbeddedExpr
 		isSecondary = true
 	}
 	tempID := srcIdx.TemporaryIndexID
 	newTempSpec.temporary = &scpb.TemporaryIndex{
 		Index:                    srcIdx,
 		IsUsingSecondaryEncoding: isSecondary,
+		EmbeddedExpr:             expr,
 	}
 	newTempSpec.temporary.TemporaryIndexID = 0
 	newTempSpec.temporary.IndexID = tempID

pkg/sql/schemachanger/scpb/elements.proto

@@ -279,6 +279,7 @@ message SecondaryIndex {
 message TemporaryIndex {
   Index embedded_index = 1 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
   bool is_using_secondary_encoding = 2;
+  Expression embedded_expr = 3 [(gogoproto.nullable) = true];
 }
 
 message SecondaryIndexPartial {

pkg/sql/schemachanger/scpb/uml/table.puml

@@ -370,6 +370,7 @@ object TemporaryIndex
 
 TemporaryIndex :  Index
 TemporaryIndex :  IsUsingSecondaryEncoding
+TemporaryIndex :  EmbeddedExpr
 
 object UniqueWithoutIndexConstraint
 

pkg/sql/schemachanger/scplan/internal/opgen/opgen_temporary_index.go

@@ -28,6 +28,17 @@ func init() {
 						IsSecondaryIndex: this.IsUsingSecondaryEncoding,
 					}
 				}),
+				emit(func(this *scpb.TemporaryIndex) *scop.SetAddedIndexPartialPredicate {
+					if this.EmbeddedExpr == nil {
+						return nil
+					}
+					return &scop.SetAddedIndexPartialPredicate{
+						TableID: this.TableID,
+						IndexID: this.IndexID,
+						Expr:    this.EmbeddedExpr.Expr,
+					}
+				}),
+
 				emit(func(this *scpb.TemporaryIndex, md *opGenContext) *scop.MaybeAddSplitForIndex {
 					// Avoid adding splits for tables without any data (i.e. newly created ones).
 					if checkIfDescriptorIsWithoutData(this.TableID, md) {

pkg/sql/schemachanger/testdata/end_to_end/create_index/create_index.definition

@@ -10,13 +10,14 @@ INSERT INTO t VALUES($stageKey + 1, 'b');
 INSERT INTO t VALUES($stageKey + 2, 'c');
 DELETE FROM t WHERE v = 'a' and k=$stageKey;
 INSERT INTO t VALUES($stageKey, 'a');
-UPDATE t SET v='a' WHERE k > 0;
+UPDATE t SET v='a' WHERE k % 2 = 0 
 ----
 
+
 # Each insert will be injected thrice per stage, so we should always,
 # see a count of 2.
 stage-query phase=PostCommitPhase stage=:
-SELECT count(*)=$successfulStageCount*3 FROM t where v='a';
+SELECT count(*)=$successfulStageCount*2 FROM t where v='a';
 ----
 true
 
@@ -27,13 +28,13 @@ INSERT INTO t VALUES($stageKey + 1, 'b');
 INSERT INTO t VALUES($stageKey + 2, 'c');
 DELETE FROM t WHERE v = 'a' and k=$stageKey;
 INSERT INTO t VALUES($stageKey, 'a');
-UPDATE t SET v='a' WHERE k > 0;
+UPDATE t SET v='a' WHERE k % 2 = 0;
 ----
 
 # Each insert will be injected twice per stage, so we should always,
 # see a count of 2.
 stage-query phase=PostCommitNonRevertiblePhase stage=:
-SELECT count(*)=$successfulStageCount*3 FROM t where v='a';
+SELECT count(*)=$successfulStageCount*2 FROM t where v='a';
 ----
 true
 

pkg/sql/schemachanger/testdata/end_to_end/create_index/create_index.explain

@@ -18,14 +18,15 @@ Schema change plan for CREATE INDEX ‹idx1› ON ‹defaultdb›.‹public›.
  │         │    ├── ABSENT → DELETE_ONLY   TemporaryIndex:{DescID: 106 (t), IndexID: 3, ConstraintID: 1, SourceIndexID: 1 (t_pkey)}
  │         │    ├── ABSENT → PUBLIC        IndexColumn:{DescID: 106 (t), ColumnID: 2 (v), IndexID: 3}
  │         │    └── ABSENT → PUBLIC        IndexColumn:{DescID: 106 (t), ColumnID: 1 (k), IndexID: 3}
- │         └── 9 Mutation operations
+ │         └── 10 Mutation operations
  │              ├── MakeAbsentIndexBackfilling {"IsSecondaryIndex":true}
  │              ├── SetAddedIndexPartialPredicate {"Expr":"v = b'@':::@1001...","IndexID":2,"TableID":106}
  │              ├── UpdateTableBackReferencesInTypes {"BackReferencedTableID":106}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":2,"TableID":106}
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":2,"Kind":1,"TableID":106}
  │              ├── SetIndexName {"IndexID":2,"Name":"idx1","TableID":106}
  │              ├── MakeAbsentTempIndexDeleteOnly {"IsSecondaryIndex":true}
+ │              ├── SetAddedIndexPartialPredicate {"Expr":"v = b'@':::@1001...","IndexID":3,"TableID":106}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":3,"TableID":106}
  │              └── AddColumnToIndex {"ColumnID":1,"IndexID":3,"Kind":1,"TableID":106}
  ├── PreCommitPhase
@@ -53,7 +54,7 @@ Schema change plan for CREATE INDEX ‹idx1› ON ‹defaultdb›.‹public›.
  │         │    ├── ABSENT → DELETE_ONLY   TemporaryIndex:{DescID: 106 (t), IndexID: 3, ConstraintID: 1, SourceIndexID: 1 (t_pkey)}
  │         │    ├── ABSENT → PUBLIC        IndexColumn:{DescID: 106 (t), ColumnID: 2 (v), IndexID: 3}
  │         │    └── ABSENT → PUBLIC        IndexColumn:{DescID: 106 (t), ColumnID: 1 (k), IndexID: 3}
- │         └── 15 Mutation operations
+ │         └── 16 Mutation operations
  │              ├── MakeAbsentIndexBackfilling {"IsSecondaryIndex":true}
  │              ├── MaybeAddSplitForIndex {"IndexID":2,"TableID":106}
  │              ├── SetAddedIndexPartialPredicate {"Expr":"v = b'@':::@1001...","IndexID":2,"TableID":106}
@@ -62,6 +63,7 @@ Schema change plan for CREATE INDEX ‹idx1› ON ‹defaultdb›.‹public›.
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":2,"Kind":1,"TableID":106}
  │              ├── SetIndexName {"IndexID":2,"Name":"idx1","TableID":106}
  │              ├── MakeAbsentTempIndexDeleteOnly {"IsSecondaryIndex":true}
+ │              ├── SetAddedIndexPartialPredicate {"Expr":"v = b'@':::@1001...","IndexID":3,"TableID":106}
  │              ├── MaybeAddSplitForIndex {"IndexID":3,"TableID":106}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":3,"TableID":106}
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":3,"Kind":1,"TableID":106}

pkg/sql/schemachanger/testdata/end_to_end/create_index/create_index.side_effects

@@ -24,7 +24,7 @@ write *eventpb.CreateIndex to event log:
     tag: CREATE INDEX
     user: root
   tableName: defaultdb.public.t
-## StatementPhase stage 1 of 1 with 9 MutationType ops
+## StatementPhase stage 1 of 1 with 10 MutationType ops
 upsert descriptor #104
   ...
      referencingDescriptorIds:
@@ -84,6 +84,7 @@ upsert descriptor #106
   +      - 1
   +      name: crdb_internal_index_3_name_placeholder
   +      partitioning: {}
+  +      predicate: v = b'@':::@100104
   +      sharded: {}
   +      storeColumnNames: []
   +      useDeletePreservingEncoding: true
@@ -108,7 +109,7 @@ upsert descriptor #106
 ## PreCommitPhase stage 1 of 2 with 1 MutationType op
 undo all catalog changes within txn #1
 persist all catalog changes to storage
-## PreCommitPhase stage 2 of 2 with 15 MutationType ops
+## PreCommitPhase stage 2 of 2 with 16 MutationType ops
 upsert descriptor #104
    type:
      arrayTypeId: 105
@@ -225,6 +226,7 @@ upsert descriptor #106
   +      - 1
   +      name: crdb_internal_index_3_name_placeholder
   +      partitioning: {}
+  +      predicate: v = b'@':::@100104
   +      sharded: {}
   +      storeColumnNames: []
   +      useDeletePreservingEncoding: true
@@ -534,6 +536,7 @@ upsert descriptor #106
   -      - 1
   -      name: crdb_internal_index_3_name_placeholder
   -      partitioning: {}
+  -      predicate: v = b'@':::@100104
   -      sharded: {}
   -      storeColumnNames: []
   -      useDeletePreservingEncoding: true

pkg/sql/schemachanger/testdata/end_to_end/create_index_create_schema_separate_statements/create_index_create_schema_separate_statements.side_effects

@@ -24,7 +24,7 @@ write *eventpb.CreateIndex to event log:
     tag: CREATE INDEX
     user: root
   tableName: defaultdb.public.t
-## StatementPhase stage 1 of 1 with 8 MutationType ops
+## StatementPhase stage 1 of 1 with 9 MutationType ops
 upsert descriptor #104
   ...
      id: 104
@@ -74,6 +74,7 @@ upsert descriptor #104
   +      - 1
   +      name: crdb_internal_index_3_name_placeholder
   +      partitioning: {}
+  +      predicate: i > 0:::INT8
   +      sharded: {}
   +      storeColumnNames: []
   +      unique: true
@@ -138,7 +139,7 @@ upsert descriptor #100
 ## PreCommitPhase stage 1 of 2 with 1 MutationType op
 undo all catalog changes within txn #1
 persist all catalog changes to storage
-## PreCommitPhase stage 2 of 2 with 21 MutationType ops
+## PreCommitPhase stage 2 of 2 with 22 MutationType ops
 add schema namespace entry {100 0 sc} -> 105
 upsert descriptor #105
   -
@@ -277,6 +278,7 @@ upsert descriptor #104
   +      - 1
   +      name: crdb_internal_index_3_name_placeholder
   +      partitioning: {}
+  +      predicate: i > 0:::INT8
   +      sharded: {}
   +      storeColumnNames: []
   +      unique: true
@@ -573,6 +575,7 @@ upsert descriptor #104
   -      - 1
   -      name: crdb_internal_index_3_name_placeholder
   -      partitioning: {}
+  -      predicate: i > 0:::INT8
   -      sharded: {}
   -      storeColumnNames: []
   -      unique: true

pkg/sql/schemachanger/testdata/end_to_end/create_index_create_schema_separate_statements/create_index_create_schema_separate_statements__statement_1_of_2.explain

@@ -18,13 +18,14 @@ Schema change plan for CREATE UNIQUE INDEX ‹idx› ON ‹defaultdb›.‹publi
  │         │    ├── ABSENT → DELETE_ONLY   TemporaryIndex:{DescID: 104 (t), IndexID: 3, ConstraintID: 3, SourceIndexID: 1 (t_pkey)}
  │         │    ├── ABSENT → PUBLIC        IndexColumn:{DescID: 104 (t), ColumnID: 2 (j), IndexID: 3}
  │         │    └── ABSENT → PUBLIC        IndexColumn:{DescID: 104 (t), ColumnID: 1 (i), IndexID: 3}
- │         └── 8 Mutation operations
+ │         └── 9 Mutation operations
  │              ├── MakeAbsentIndexBackfilling {"IsSecondaryIndex":true}
  │              ├── SetAddedIndexPartialPredicate {"Expr":"i \u003e 0:::INT8","IndexID":2,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":2,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":2,"Kind":1,"TableID":104}
  │              ├── SetIndexName {"IndexID":2,"Name":"idx","TableID":104}
  │              ├── MakeAbsentTempIndexDeleteOnly {"IsSecondaryIndex":true}
+ │              ├── SetAddedIndexPartialPredicate {"Expr":"i \u003e 0:::INT8","IndexID":3,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":3,"TableID":104}
  │              └── AddColumnToIndex {"ColumnID":1,"IndexID":3,"Kind":1,"TableID":104}
  ├── PreCommitPhase
@@ -52,14 +53,15 @@ Schema change plan for CREATE UNIQUE INDEX ‹idx› ON ‹defaultdb›.‹publi
  │         │    ├── ABSENT → DELETE_ONLY   TemporaryIndex:{DescID: 104 (t), IndexID: 3, ConstraintID: 3, SourceIndexID: 1 (t_pkey)}
  │         │    ├── ABSENT → PUBLIC        IndexColumn:{DescID: 104 (t), ColumnID: 2 (j), IndexID: 3}
  │         │    └── ABSENT → PUBLIC        IndexColumn:{DescID: 104 (t), ColumnID: 1 (i), IndexID: 3}
- │         └── 12 Mutation operations
+ │         └── 13 Mutation operations
  │              ├── MakeAbsentIndexBackfilling {"IsSecondaryIndex":true}
  │              ├── MaybeAddSplitForIndex {"IndexID":2,"TableID":104}
  │              ├── SetAddedIndexPartialPredicate {"Expr":"i \u003e 0:::INT8","IndexID":2,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":2,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":2,"Kind":1,"TableID":104}
  │              ├── SetIndexName {"IndexID":2,"Name":"idx","TableID":104}
  │              ├── MakeAbsentTempIndexDeleteOnly {"IsSecondaryIndex":true}
+ │              ├── SetAddedIndexPartialPredicate {"Expr":"i \u003e 0:::INT8","IndexID":3,"TableID":104}
  │              ├── MaybeAddSplitForIndex {"IndexID":3,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":3,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":3,"Kind":1,"TableID":104}

pkg/sql/schemachanger/testdata/end_to_end/create_index_create_schema_separate_statements/create_index_create_schema_separate_statements__statement_2_of_2.explain

@@ -62,11 +62,12 @@ Schema change plan for CREATE SCHEMA ‹defaultdb›.‹sc›; following CREATE
  │         │    ├── ABSENT → PUBLIC           IndexColumn:{DescID: 104 (t), ColumnID: 2 (j), IndexID: 3 (crdb_internal_index_3_name_placeholder)}
  │         │    ├── ABSENT → PUBLIC           IndexColumn:{DescID: 104 (t), ColumnID: 1 (i), IndexID: 3 (crdb_internal_index_3_name_placeholder)}
  │         │    └── ABSENT → DELETE_ONLY      TemporaryIndex:{DescID: 104 (t), IndexID: 3 (crdb_internal_index_3_name_placeholder), ConstraintID: 3, SourceIndexID: 1 (t_pkey)}
- │         └── 21 Mutation operations
+ │         └── 22 Mutation operations
  │              ├── MakeAbsentIndexBackfilling {"IsSecondaryIndex":true}
  │              ├── MaybeAddSplitForIndex {"IndexID":2,"TableID":104}
  │              ├── SetAddedIndexPartialPredicate {"Expr":"i \u003e 0:::INT8","IndexID":2,"TableID":104}
  │              ├── MakeAbsentTempIndexDeleteOnly {"IsSecondaryIndex":true}
+ │              ├── SetAddedIndexPartialPredicate {"Expr":"i \u003e 0:::INT8","IndexID":3,"TableID":104}
  │              ├── MaybeAddSplitForIndex {"IndexID":3,"TableID":104}
  │              ├── CreateSchemaDescriptor {"SchemaID":105}
  │              ├── SetNameInDescriptor {"DescriptorID":105,"Name":"sc"}