Skip to content

Commit

Permalink
Merge #86071
Browse files Browse the repository at this point in the history 
86071: scbuildstmt: ALTER PK drops rowid column when possible r=postamar a=postamar

This commit improves the ALTER TABLE ... ALTER PRIMARY KEY ...
implementation in the declarative schema changer by also having it drop
the hidden rowid column when possible.

This required introducing a transient public index which includes rowid
as a storing column. This transient index is then swapped out with
the definitive primary index which does not include rowid at all. This
commit adds a rule to ensure that the second swapping takes place only
after the original primary index, in which rowid was the PK, has been
completely removed. Otherwise, we run into some nasty issues when faced
with concurrent writes.

Fixes #80149.

Release note (sql change): When performed by the declarative schema
changer (as is the case by default) the ALTER PRIMARY KEY statement
now also drops the rowid column when no references are held to it
anywhere. The rowid column is a hidden column which is implicitly added
and serves as primary key on any table which is created without
explicitly specifying a primary key.


Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
  • Loading branch information
@ajwerner
3 people committed Aug 13, 2022
2 parents 384a281 + c15d1ad commit aa77fba
Show file tree
Hide file tree
Showing 312 changed files with 18,948 additions and 10,276 deletions.
8 changes: 1 addition & 7 deletions pkg/ccl/schemachangerccl/testdata/explain/create_index.rollback_1_of_7
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,7 @@ Schema change plan for rolling back CREATE INDEX ‹id1› ON ‹defaultdb›.pu
│ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ └── PUBLIC → ABSENT IndexPartitioning:{DescID: 104, IndexID: 3}
└── 13 Mutation operations
├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":3,"TableID":104}
├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":3,"Ordinal":1,"TableID":104}
├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":3,"Kind":2,"TableID":104}
├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"TableID":104}
├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"Ordinal":1,"TableID":104}
├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":2,"Kind":2,"TableID":104}
└── 7 Mutation operations
├── LogEvent {"TargetStatus":1}
├── CreateGcJobForIndex {"IndexID":2,"TableID":104}
├── MakeIndexAbsent {"IndexID":2,"TableID":104}
Expand Down
8 changes: 1 addition & 7 deletions pkg/ccl/schemachangerccl/testdata/explain/create_index.rollback_2_of_7
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,8 @@ Schema change plan for rolling back CREATE INDEX ‹id1› ON ‹defaultdb›.pu
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ └── PUBLIC → ABSENT IndexPartitioning:{DescID: 104, IndexID: 3}
│ └── 12 Mutation operations
│ └── 6 Mutation operations
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":3,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":3,"Kind":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":2,"Kind":2,"TableID":104}
│ ├── LogEvent {"TargetStatus":1}
│ ├── CreateGcJobForIndex {"IndexID":2,"TableID":104}
│ ├── MakeIndexAbsent {"IndexID":2,"TableID":104}
Expand Down
8 changes: 1 addition & 7 deletions pkg/ccl/schemachangerccl/testdata/explain/create_index.rollback_3_of_7
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,8 @@ Schema change plan for rolling back CREATE INDEX ‹id1› ON ‹defaultdb›.pu
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ └── PUBLIC → ABSENT IndexPartitioning:{DescID: 104, IndexID: 3}
│ └── 12 Mutation operations
│ └── 6 Mutation operations
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":3,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":3,"Kind":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":2,"Kind":2,"TableID":104}
│ ├── LogEvent {"TargetStatus":1}
│ ├── CreateGcJobForIndex {"IndexID":2,"TableID":104}
│ ├── MakeIndexAbsent {"IndexID":2,"TableID":104}
Expand Down
8 changes: 1 addition & 7 deletions pkg/ccl/schemachangerccl/testdata/explain/create_index.rollback_4_of_7
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,8 @@ Schema change plan for rolling back CREATE INDEX ‹id1› ON ‹defaultdb›.pu
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ └── PUBLIC → ABSENT IndexPartitioning:{DescID: 104, IndexID: 3}
│ └── 12 Mutation operations
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":2,"Kind":2,"TableID":104}
│ └── 6 Mutation operations
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":3,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":3,"Kind":2,"TableID":104}
│ ├── LogEvent {"TargetStatus":1}
│ ├── CreateGcJobForIndex {"IndexID":2,"TableID":104}
│ ├── MakeIndexAbsent {"IndexID":2,"TableID":104}
Expand Down
8 changes: 1 addition & 7 deletions pkg/ccl/schemachangerccl/testdata/explain/create_index.rollback_5_of_7
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,9 @@ Schema change plan for rolling back CREATE INDEX ‹id1› ON ‹defaultdb›.pu
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ └── PUBLIC → ABSENT IndexPartitioning:{DescID: 104, IndexID: 3}
│ └── 10 Mutation operations
│ └── 4 Mutation operations
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":3,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":3,"Kind":2,"TableID":104}
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":2,"Kind":2,"TableID":104}
│ ├── SetJobStateOnDescriptor {"DescriptorID":104}
│ └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"PostCommitNonRev..."}
└── Stage 2 of 2 in PostCommitNonRevertiblePhase
Expand Down
8 changes: 1 addition & 7 deletions pkg/ccl/schemachangerccl/testdata/explain/create_index.rollback_6_of_7
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,9 @@ Schema change plan for rolling back CREATE INDEX ‹id1› ON ‹defaultdb›.pu
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ └── PUBLIC → ABSENT IndexPartitioning:{DescID: 104, IndexID: 3}
│ └── 10 Mutation operations
│ └── 4 Mutation operations
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":3,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":3,"Kind":2,"TableID":104}
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":2,"Kind":2,"TableID":104}
│ ├── SetJobStateOnDescriptor {"DescriptorID":104}
│ └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"PostCommitNonRev..."}
└── Stage 2 of 2 in PostCommitNonRevertiblePhase
Expand Down
8 changes: 1 addition & 7 deletions pkg/ccl/schemachangerccl/testdata/explain/create_index.rollback_7_of_7
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,9 @@ Schema change plan for rolling back CREATE INDEX ‹id1› ON ‹defaultdb›.pu
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ └── PUBLIC → ABSENT IndexPartitioning:{DescID: 104, IndexID: 3}
│ └── 10 Mutation operations
│ └── 4 Mutation operations
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":2,"TableID":104}
│ ├── MakeDroppedIndexDeleteOnly {"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":3,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":3,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":3,"Kind":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"Ordinal":1,"TableID":104}
│ ├── RemoveColumnFromIndex {"ColumnID":3,"IndexID":2,"Kind":2,"TableID":104}
│ ├── SetJobStateOnDescriptor {"DescriptorID":104}
│ └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"PostCommitNonRev..."}
└── Stage 2 of 2 in PostCommitNonRevertiblePhase
Expand Down
52 changes: 28 additions & 24 deletions pkg/ccl/schemachangerccl/testdata/explain_verbose/create_index
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,52 +19,52 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
│ │ │ │ ABSENT → PUBLIC
│ │ │ │
│ │ │ └── • Precedence dependency from BACKFILL_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
│ │ │ rule: "index-column added to index after index exists"
│ │ │ rule: "index existence precedes index dependents"
│ │ │
│ │ ├── • IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 2}
│ │ │ │ ABSENT → PUBLIC
│ │ │ │
│ │ │ └── • Precedence dependency from BACKFILL_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
│ │ │ rule: "index-column added to index after index exists"
│ │ │ rule: "index existence precedes index dependents"
│ │ │
│ │ ├── • IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 2}
│ │ │ │ ABSENT → PUBLIC
│ │ │ │
│ │ │ └── • Precedence dependency from BACKFILL_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
│ │ │ rule: "index-column added to index after index exists"
│ │ │ rule: "index existence precedes index dependents"
│ │ │
│ │ ├── • SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
│ │ │ ABSENT → BACKFILL_ONLY
│ │ │
│ │ ├── • IndexPartitioning:{DescID: 104, IndexID: 2}
│ │ │ ABSENT → PUBLIC
│ │ │ │ ABSENT → PUBLIC
│ │ │ │
│ │ │ └── • Precedence dependency from BACKFILL_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
│ │ │ rule: "index existence precedes index dependents"
│ │ │
│ │ ├── • IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 3}
│ │ │ │ ABSENT → PUBLIC
│ │ │ │
│ │ │ └── • Precedence dependency from DELETE_ONLY TemporaryIndex:{DescID: 104, IndexID: 3, ConstraintID: 0, SourceIndexID: 1}
│ │ │ rule: "temp index exists before columns, partitioning, and partial"
│ │ │ rule: "index-column added to index after temp index exists"
│ │ │ rule: "temp index existence precedes index dependents"
│ │ │
│ │ ├── • IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ │ │ ABSENT → PUBLIC
│ │ │ │
│ │ │ └── • Precedence dependency from DELETE_ONLY TemporaryIndex:{DescID: 104, IndexID: 3, ConstraintID: 0, SourceIndexID: 1}
│ │ │ rule: "temp index exists before columns, partitioning, and partial"
│ │ │ rule: "index-column added to index after temp index exists"
│ │ │ rule: "temp index existence precedes index dependents"
│ │ │
│ │ ├── • IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ │ │ ABSENT → PUBLIC
│ │ │ │
│ │ │ └── • Precedence dependency from DELETE_ONLY TemporaryIndex:{DescID: 104, IndexID: 3, ConstraintID: 0, SourceIndexID: 1}
│ │ │ rule: "temp index exists before columns, partitioning, and partial"
│ │ │ rule: "index-column added to index after temp index exists"
│ │ │ rule: "temp index existence precedes index dependents"
│ │ │
│ │ └── • IndexPartitioning:{DescID: 104, IndexID: 3}
│ │ │ ABSENT → PUBLIC
│ │ │
│ │ └── • Precedence dependency from DELETE_ONLY TemporaryIndex:{DescID: 104, IndexID: 3, ConstraintID: 0, SourceIndexID: 1}
│ │ rule: "temp index exists before columns, partitioning, and partial"
│ │ rule: "temp index existence precedes index dependents"
│ │
│ ├── • 1 element transitioning toward TRANSIENT_ABSENT
│ │ │
Expand Down Expand Up @@ -187,16 +187,7 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
│ │ ├── • 1 element transitioning toward TRANSIENT_ABSENT
│ │ │ │
│ │ │ └── • TemporaryIndex:{DescID: 104, IndexID: 3, ConstraintID: 0, SourceIndexID: 1}
│ │ │ │ DELETE_ONLY → WRITE_ONLY
│ │ │ │
│ │ │ ├── • Precedence dependency from PUBLIC IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 3}
│ │ │ │ rule: "index-column added to index before temp index receives writes"
│ │ │ │
│ │ │ ├── • Precedence dependency from PUBLIC IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 3}
│ │ │ │ rule: "index-column added to index before temp index receives writes"
│ │ │ │
│ │ │ └── • Precedence dependency from PUBLIC IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 3}
│ │ │ rule: "index-column added to index before temp index receives writes"
│ │ │ DELETE_ONLY → WRITE_ONLY
│ │ │
│ │ └── • 3 Mutation operations
│ │ │
Expand Down Expand Up @@ -333,14 +324,27 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
│ │ ├── • SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
│ │ │ │ VALIDATED → PUBLIC
│ │ │ │
│ │ │ └── • SameStagePrecedence dependency from PUBLIC IndexName:{DescID: 104, Name: id1, IndexID: 2}
│ │ │ rule: "index named right before index becomes public"
│ │ │ ├── • Precedence dependency from PUBLIC IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 2}
│ │ │ │ rule: "index dependents exist before index becomes public"
│ │ │ │
│ │ │ ├── • Precedence dependency from PUBLIC IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 2}
│ │ │ │ rule: "index dependents exist before index becomes public"
│ │ │ │
│ │ │ ├── • Precedence dependency from PUBLIC IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 2}
│ │ │ │ rule: "index dependents exist before index becomes public"
│ │ │ │
│ │ │ ├── • SameStagePrecedence dependency from PUBLIC IndexName:{DescID: 104, Name: id1, IndexID: 2}
│ │ │ │ rule: "index dependents exist before index becomes public"
│ │ │ │ rule: "index named right before index becomes public"
│ │ │ │
│ │ │ └── • Precedence dependency from PUBLIC IndexPartitioning:{DescID: 104, IndexID: 2}
│ │ │ rule: "index dependents exist before index becomes public"
│ │ │
│ │ └── • IndexName:{DescID: 104, Name: id1, IndexID: 2}
│ │ │ ABSENT → PUBLIC
│ │ │
│ │ └── • Precedence dependency from BACKFILL_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
│ │ rule: "index existence precedes index name and comment"
│ │ rule: "index existence precedes index dependents"
│ │
│ ├── • 1 element transitioning toward TRANSIENT_ABSENT
│ │ │
Expand Down
Loading

0 comments on commit aa77fba

Please sign in to comment.