Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql/schemachanger: create partial index with inserts can fail #110216

Merged
merged 1 commit into from Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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,
Expr: expr,
}
newTempSpec.temporary.TemporaryIndexID = 0
newTempSpec.temporary.IndexID = tempID
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/schemachanger/scpb/elements.proto
Expand Up @@ -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 expr = 3 [(gogoproto.nullable) = true];
}

message SecondaryIndexPartial {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/schemachanger/scpb/uml/table.puml
Expand Up @@ -370,6 +370,7 @@ object TemporaryIndex

TemporaryIndex : Index
TemporaryIndex : IsUsingSecondaryEncoding
TemporaryIndex : Expr

object UniqueWithoutIndexConstraint

Expand Down
Expand Up @@ -28,6 +28,17 @@ func init() {
IsSecondaryIndex: this.IsUsingSecondaryEncoding,
}
}),
emit(func(this *scpb.TemporaryIndex) *scop.SetAddedIndexPartialPredicate {
if this.Expr == nil {
return nil
}
return &scop.SetAddedIndexPartialPredicate{
TableID: this.TableID,
IndexID: this.IndexID,
Expr: this.Expr.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) {
Expand Down
Expand Up @@ -3,40 +3,46 @@ CREATE TYPE e AS ENUM('a', 'b', 'c');
CREATE TABLE t (k INT PRIMARY KEY, v e NOT NULL);
----


# Intentionally, insert one value for the partial index ('a')
# (see below). Update one value so it will get added into
# the partial index. The $stageKeyValue + 1 will not be in the
# partial index. The delete remove the first insert which will
# get added back again after. All these changes are propogated in
# the temporary index.
stage-exec phase=PostCommitPhase stage=:
INSERT INTO t VALUES($stageKey, 'a');
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a note as to why % 2 is being used in favor of > 0?

----

# Each insert will be injected thrice per stage, so we should always,
# see a count of 2.

# The value 'a' is added twice per stage, see the rational above.
stage-query phase=PostCommitPhase stage=:
SELECT count(*)=$successfulStageCount*3 FROM t where v='a';
SELECT count(*)=$successfulStageCount*2 FROM t where v='a';
----
true


# Similar to the sequence above in the PostCommitPhase.
stage-exec phase=PostCommitNonRevertiblePhase stage=:
INSERT INTO t VALUES($stageKey, 'a');
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.

# The value 'a' is added twice per stage, see the rational above.
stage-query phase=PostCommitNonRevertiblePhase stage=:
SELECT count(*)=$successfulStageCount*3 FROM t where v='a';
SELECT count(*)=$successfulStageCount*2 FROM t where v='a';
----
true

# Create a partial index on column V where the value is A
test
CREATE INDEX idx1 ON t (v) WHERE (v = 'a');
----
Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -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}
Expand Down
Expand Up @@ -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:
Expand Down Expand Up @@ -84,6 +84,7 @@ upsert descriptor #106
+ - 1
+ name: crdb_internal_index_3_name_placeholder
+ partitioning: {}
+ predicate: v = b'@':::@100104
+ sharded: {}
+ storeColumnNames: []
+ useDeletePreservingEncoding: true
Expand All @@ -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
Expand Down Expand Up @@ -225,6 +226,7 @@ upsert descriptor #106
+ - 1
+ name: crdb_internal_index_3_name_placeholder
+ partitioning: {}
+ predicate: v = b'@':::@100104
+ sharded: {}
+ storeColumnNames: []
+ useDeletePreservingEncoding: true
Expand Down Expand Up @@ -534,6 +536,7 @@ upsert descriptor #106
- - 1
- name: crdb_internal_index_3_name_placeholder
- partitioning: {}
- predicate: v = b'@':::@100104
- sharded: {}
- storeColumnNames: []
- useDeletePreservingEncoding: true
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -74,6 +74,7 @@ upsert descriptor #104
+ - 1
+ name: crdb_internal_index_3_name_placeholder
+ partitioning: {}
+ predicate: i > 0:::INT8
+ sharded: {}
+ storeColumnNames: []
+ unique: true
Expand Down Expand Up @@ -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
-
Expand Down Expand Up @@ -277,6 +278,7 @@ upsert descriptor #104
+ - 1
+ name: crdb_internal_index_3_name_placeholder
+ partitioning: {}
+ predicate: i > 0:::INT8
+ sharded: {}
+ storeColumnNames: []
+ unique: true
Expand Down Expand Up @@ -573,6 +575,7 @@ upsert descriptor #104
- - 1
- name: crdb_internal_index_3_name_placeholder
- partitioning: {}
- predicate: i > 0:::INT8
- sharded: {}
- storeColumnNames: []
- unique: true
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down
Expand Up @@ -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"}
Expand Down