Skip to content

Commit ee0bb8f

Browse files
craig[bot]fqazi
craig[bot]
and
fqazi
committed
151691: sql/schemachanger: hide new secondary indexes during PK swap r=fqazi a=fqazi Previously, during a primary key swap, the recreated secondary indexes were visible to introspection queries. This could cause users to encounter errors if they queried these indexes mid-operation. This patch resolves the issue by making the new indexes temporarily non-public and renaming them to prevent accidental access during the swap. Fixes: cockroachdb#148405 Release note (bug fix): Fixed a bug that allowed replacement indexes, created during an ALTER PRIMARY KEY operation, to be accessed with index hints before they were in a ready state. This could lead to runtime errors. Co-authored-by: Faizan Qazi <[email protected]>
2 parents 29896e5 + 6c342d7 commit ee0bb8f

File tree

56 files changed

+578
-539
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+578
-539
lines changed

‎pkg/ccl/schemachangerccl/testdata/decomp/partitioning‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,7 @@ ElementState:
517517
constraintId: 0
518518
embeddedExpr: null
519519
geoConfig: null
520+
hideForPrimaryKeyRecreated: false
520521
indexId: 2
521522
invisibility: 0
522523
isConcurrently: false
@@ -956,6 +957,7 @@ ElementState:
956957
constraintId: 1
957958
embeddedExpr: null
958959
geoConfig: null
960+
hideForPrimaryKeyRecreated: false
959961
indexId: 2
960962
invisibility: 0
961963
isConcurrently: false

‎pkg/ccl/schemachangerccl/testdata/end_to_end/alter_table_alter_primary_key_rbr/alter_table_alter_primary_key_rbr.definition‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CREATE TABLE multiregion_db.public.table_regional_by_row (
44
k INT NOT NULL,
55
V STRING
66
) LOCALITY REGIONAL BY ROW;
7-
CREATE INDEX on multiregion_db.table_regional_by_row(v);
7+
CREATE INDEX idx_v on multiregion_db.table_regional_by_row(v);
88
----
99

1010
stage-exec phase=PostCommitPhase stage=:
@@ -29,6 +29,22 @@ USE multiregion_db;
2929
SELECT crdb_internal.validate_multi_region_zone_configs()
3030
----
3131

32+
stage-query phase=PostCommitPhase stage=:
33+
SELECT index_name FROM crdb_internal.table_indexes WHERE descriptor_name='table_regional_by_row' ORDER BY index_name;
34+
----
35+
idx_v
36+
table_regional_by_row_pkey
37+
38+
39+
stage-query phase=PostCommitNonRevertiblePhase stage=:
40+
SELECT index_name FROM crdb_internal.table_indexes WHERE descriptor_name='table_regional_by_row' ORDER BY index_name;
41+
----
42+
idx_v
43+
table_regional_by_row_pkey
44+
table_regional_by_row_rowid_key
45+
46+
47+
3248
test
3349
alter table multiregion_db.table_regional_by_row add column m int8 default unique_rowid(), alter primary key using columns(k) USING HASH;
3450
----

‎pkg/ccl/schemachangerccl/testdata/end_to_end/alter_table_alter_primary_key_rbr/alter_table_alter_primary_key_rbr.explain‎

Lines changed: 30 additions & 30 deletions
Large diffs are not rendered by default.

‎pkg/ccl/schemachangerccl/testdata/end_to_end/alter_table_alter_primary_key_rbr/alter_table_alter_primary_key_rbr.explain_shape‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CREATE TABLE multiregion_db.public.table_regional_by_row (
44
k INT NOT NULL,
55
V STRING
66
) LOCALITY REGIONAL BY ROW;
7-
CREATE INDEX on multiregion_db.table_regional_by_row(v);
7+
CREATE INDEX idx_v on multiregion_db.table_regional_by_row(v);
88

99
/* test */
1010
EXPLAIN (DDL, SHAPE) alter table multiregion_db.table_regional_by_row add column m int8 default unique_rowid(), alter primary key using columns(k) USING HASH;
@@ -20,16 +20,16 @@ Schema change plan for ALTER TABLE ‹multiregion_db›.‹public›.‹table_re
2020
├── validate UNIQUE constraint backed by index table_regional_by_row_pkey~ in relation table_regional_by_row
2121
├── execute 2 system table mutations transactions
2222
├── backfill using primary index table_regional_by_row_pkey~ in relation table_regional_by_row
23-
│ ├── into table_regional_by_row_v_idx+ (crdb_region, v: k, crdb_internal_k_shard_16+)
23+
│ ├── into idx_v+ (crdb_region, v: k, crdb_internal_k_shard_16+)
2424
│ └── into table_regional_by_row_pkey+ (crdb_internal_k_shard_16+, k, crdb_region; rowid, v, m+)
2525
├── execute 2 system table mutations transactions
2626
├── merge temporary indexes into backfilled indexes in relation table_regional_by_row
27-
│ ├── from table_regional_by_row@[5] into table_regional_by_row_v_idx+
27+
│ ├── from table_regional_by_row@[5] into idx_v+
2828
│ └── from table_regional_by_row@[11] into table_regional_by_row_pkey+
2929
├── execute 1 system table mutations transaction
3030
├── validate UNIQUE constraint backed by index table_regional_by_row_pkey+ in relation table_regional_by_row
3131
├── validate NOT NULL constraint on column crdb_internal_k_shard_16+ in index table_regional_by_row_pkey+ in relation table_regional_by_row
32-
├── validate UNIQUE constraint backed by index table_regional_by_row_v_idx+ in relation table_regional_by_row
32+
├── validate UNIQUE constraint backed by index idx_v+ in relation table_regional_by_row
3333
├── execute 2 system table mutations transactions
3434
├── backfill using primary index table_regional_by_row_pkey+ in relation table_regional_by_row
3535
│ └── into table_regional_by_row_rowid_key+ (rowid, crdb_region: crdb_internal_k_shard_16+, k)

‎pkg/ccl/schemachangerccl/testdata/end_to_end/alter_table_alter_primary_key_rbr/alter_table_alter_primary_key_rbr.side_effects‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CREATE TABLE multiregion_db.public.table_regional_by_row (
44
k INT NOT NULL,
55
V STRING
66
) LOCALITY REGIONAL BY ROW;
7-
CREATE INDEX on multiregion_db.table_regional_by_row(v);
7+
CREATE INDEX idx_v on multiregion_db.table_regional_by_row(v);
88
----
99
...
1010
+database {0 0 multiregion_db} -> 104
@@ -290,7 +290,7 @@ upsert descriptor #108
290290
+ "0": primary
291291
+ id: 108
292292
+ indexes:
293-
+ "4": table_regional_by_row_v_idx
293+
+ "4": idx_v
294294
+ "6": table_regional_by_row_rowid_key
295295
+ "10": table_regional_by_row_pkey
296296
+ name: table_regional_by_row
@@ -973,7 +973,7 @@ validate CHECK constraint crdb_internal_k_shard_16_auto_not_null in table #108
973973
validate forward indexes [4] in table #108
974974
commit transaction #17
975975
begin transaction #18
976-
## PostCommitPhase stage 16 of 24 with 26 MutationType ops
976+
## PostCommitPhase stage 16 of 24 with 27 MutationType ops
977977
upsert descriptor #108
978978
table:
979979
- checks:
@@ -998,19 +998,15 @@ upsert descriptor #108
998998
- id: 2
999999
+ id: 4
10001000
interleave: {}
1001-
+ invisibility: 1
10021001
keyColumnDirections:
1003-
- ASC
10041002
...
10051003
- v
10061004
keySuffixColumnIds:
10071005
- - 4
10081006
+ - 1
10091007
+ - 6
1010-
name: table_regional_by_row_v_idx
1011-
+ notVisible: true
1008+
name: idx_v
10121009
partitioning:
1013-
list:
10141010
...
10151011
numImplicitColumns: 1
10161012
sharded: {}
@@ -1263,7 +1259,7 @@ upsert descriptor #108
12631259
+ - v
12641260
+ keySuffixColumnIds:
12651261
+ - 4
1266-
+ name: table_regional_by_row_v_idx
1262+
+ name: idx_v
12671263
+ partitioning:
12681264
+ list:
12691265
+ - name: us-east1
@@ -1731,7 +1727,7 @@ upsert descriptor #108
17311727
...
17321728
keySuffixColumnIds:
17331729
- 4
1734-
- name: table_regional_by_row_v_idx
1730+
- name: idx_v
17351731
+ name: crdb_internal_index_2_name_placeholder
17361732
partitioning:
17371733
list:
@@ -1748,10 +1744,10 @@ upsert descriptor #108
17481744
- version: "24"
17491745
+ version: "25"
17501746
persist all catalog changes to storage
1751-
update progress of schema change job #1: "Pending: Updating schema metadata (13 operations) — PostCommitNonRevertible phase (stage 2 of 6)."
1747+
update progress of schema change job #1: "Pending: Updating schema metadata (12 operations) — PostCommitNonRevertible phase (stage 2 of 6)."
17521748
commit transaction #27
17531749
begin transaction #28
1754-
## PostCommitNonRevertiblePhase stage 2 of 6 with 15 MutationType ops
1750+
## PostCommitNonRevertiblePhase stage 2 of 6 with 14 MutationType ops
17551751
upsert descriptor #108
17561752
table:
17571753
- checks: []
@@ -1787,18 +1783,6 @@ upsert descriptor #108
17871783
+ virtual: true
17881784
createAsOfTime:
17891785
wallTime: "1640995200000000000"
1790-
...
1791-
id: 4
1792-
interleave: {}
1793-
- invisibility: 1
1794-
keyColumnDirections:
1795-
- ASC
1796-
...
1797-
- 6
1798-
name: table_regional_by_row_v_idx
1799-
- notVisible: true
1800-
partitioning:
1801-
list:
18021786
...
18031787
modificationTime: {}
18041788
mutations:
@@ -2178,7 +2162,7 @@ upsert descriptor #108
21782162
- "0": primary
21792163
- id: 108
21802164
- indexes:
2181-
- "4": table_regional_by_row_v_idx
2165+
- "4": idx_v
21822166
- "6": table_regional_by_row_rowid_key
21832167
- "10": table_regional_by_row_pkey
21842168
- name: table_regional_by_row

‎pkg/ccl/schemachangerccl/testdata/end_to_end/alter_table_alter_primary_key_rbr/alter_table_alter_primary_key_rbr__rollback_10_of_24.explain‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CREATE TABLE multiregion_db.public.table_regional_by_row (
44
k INT NOT NULL,
55
V STRING
66
) LOCALITY REGIONAL BY ROW;
7-
CREATE INDEX on multiregion_db.table_regional_by_row(v);
7+
CREATE INDEX idx_v on multiregion_db.table_regional_by_row(v);
88

99
/* test */
1010
alter table multiregion_db.table_regional_by_row add column m int8 default unique_rowid(), alter primary key using columns(k) USING HASH;
@@ -49,12 +49,12 @@ Schema change plan for rolling back ALTER TABLE multiregion_db.public.table_regi
4949
│ │ ├── WRITE_ONLY → ABSENT ColumnNotNull:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 10 (table_regional_by_row_pkey-)}
5050
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 10 (table_regional_by_row_pkey-)}
5151
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 11}
52-
│ │ ├── BACKFILL_ONLY → ABSENT SecondaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 4 (table_regional_by_row_v_idx-), ConstraintID: 3, TemporaryIndexID: 5, SourceIndexID: 8 (table_regional_by_row_pkey-), RecreateSourceIndexID: 2}
53-
│ │ ├── PUBLIC → ABSENT IndexPartitioning:{DescID: 108 (table_regional_by_row), IndexID: 4 (table_regional_by_row_v_idx-)}
54-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 3 (crdb_region), IndexID: 4 (table_regional_by_row_v_idx-)}
55-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 2 (v), IndexID: 4 (table_regional_by_row_v_idx-)}
56-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 4 (table_regional_by_row_v_idx-)}
57-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 4 (table_regional_by_row_v_idx-)}
52+
│ │ ├── BACKFILL_ONLY → ABSENT SecondaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 4 (idx_v-), ConstraintID: 3, TemporaryIndexID: 5, SourceIndexID: 8 (table_regional_by_row_pkey-), RecreateSourceIndexID: 2}
53+
│ │ ├── PUBLIC → ABSENT IndexPartitioning:{DescID: 108 (table_regional_by_row), IndexID: 4 (idx_v-)}
54+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 3 (crdb_region), IndexID: 4 (idx_v-)}
55+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 2 (v), IndexID: 4 (idx_v-)}
56+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 4 (idx_v-)}
57+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 4 (idx_v-)}
5858
│ │ ├── WRITE_ONLY → DELETE_ONLY TemporaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 5, ConstraintID: 4, SourceIndexID: 8 (table_regional_by_row_pkey-)}
5959
│ │ ├── TRANSIENT_ABSENT → ABSENT IndexPartitioning:{DescID: 108 (table_regional_by_row), IndexID: 5}
6060
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 3 (crdb_region), IndexID: 5}
@@ -129,7 +129,7 @@ Schema change plan for rolling back ALTER TABLE multiregion_db.public.table_regi
129129
│ │ ├── DELETE_ONLY → ABSENT Column:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-)}
130130
│ │ ├── PUBLIC → ABSENT ColumnType:{DescID: 108 (table_regional_by_row), ColumnFamilyID: 0 (primary), ColumnID: 6 (crdb_internal_k_shard_16-), TypeName: "INT8"}
131131
│ │ ├── PUBLIC → ABSENT ColumnComputeExpression:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), Usage: REGULAR}
132-
│ │ ├── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 4 (table_regional_by_row_v_idx-)}
132+
│ │ ├── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 4 (idx_v-)}
133133
│ │ ├── DELETE_ONLY → ABSENT TemporaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 5, ConstraintID: 4, SourceIndexID: 8 (table_regional_by_row_pkey-)}
134134
│ │ └── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 5}
135135
│ └── 15 Mutation operations

‎pkg/ccl/schemachangerccl/testdata/end_to_end/alter_table_alter_primary_key_rbr/alter_table_alter_primary_key_rbr__rollback_11_of_24.explain‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CREATE TABLE multiregion_db.public.table_regional_by_row (
44
k INT NOT NULL,
55
V STRING
66
) LOCALITY REGIONAL BY ROW;
7-
CREATE INDEX on multiregion_db.table_regional_by_row(v);
7+
CREATE INDEX idx_v on multiregion_db.table_regional_by_row(v);
88

99
/* test */
1010
alter table multiregion_db.table_regional_by_row add column m int8 default unique_rowid(), alter primary key using columns(k) USING HASH;
@@ -49,12 +49,12 @@ Schema change plan for rolling back ALTER TABLE multiregion_db.public.table_regi
4949
│ │ ├── WRITE_ONLY → ABSENT ColumnNotNull:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 10 (table_regional_by_row_pkey-)}
5050
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 10 (table_regional_by_row_pkey-)}
5151
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 11}
52-
│ │ ├── BACKFILL_ONLY → ABSENT SecondaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 4 (table_regional_by_row_v_idx-), ConstraintID: 3, TemporaryIndexID: 5, SourceIndexID: 8 (table_regional_by_row_pkey-), RecreateSourceIndexID: 2}
53-
│ │ ├── PUBLIC → ABSENT IndexPartitioning:{DescID: 108 (table_regional_by_row), IndexID: 4 (table_regional_by_row_v_idx-)}
54-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 3 (crdb_region), IndexID: 4 (table_regional_by_row_v_idx-)}
55-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 2 (v), IndexID: 4 (table_regional_by_row_v_idx-)}
56-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 4 (table_regional_by_row_v_idx-)}
57-
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 4 (table_regional_by_row_v_idx-)}
52+
│ │ ├── BACKFILL_ONLY → ABSENT SecondaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 4 (idx_v-), ConstraintID: 3, TemporaryIndexID: 5, SourceIndexID: 8 (table_regional_by_row_pkey-), RecreateSourceIndexID: 2}
53+
│ │ ├── PUBLIC → ABSENT IndexPartitioning:{DescID: 108 (table_regional_by_row), IndexID: 4 (idx_v-)}
54+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 3 (crdb_region), IndexID: 4 (idx_v-)}
55+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 2 (v), IndexID: 4 (idx_v-)}
56+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 1 (k), IndexID: 4 (idx_v-)}
57+
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), IndexID: 4 (idx_v-)}
5858
│ │ ├── WRITE_ONLY → DELETE_ONLY TemporaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 5, ConstraintID: 4, SourceIndexID: 8 (table_regional_by_row_pkey-)}
5959
│ │ ├── TRANSIENT_ABSENT → ABSENT IndexPartitioning:{DescID: 108 (table_regional_by_row), IndexID: 5}
6060
│ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108 (table_regional_by_row), ColumnID: 3 (crdb_region), IndexID: 5}
@@ -129,7 +129,7 @@ Schema change plan for rolling back ALTER TABLE multiregion_db.public.table_regi
129129
│ │ ├── DELETE_ONLY → ABSENT Column:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-)}
130130
│ │ ├── PUBLIC → ABSENT ColumnType:{DescID: 108 (table_regional_by_row), ColumnFamilyID: 0 (primary), ColumnID: 6 (crdb_internal_k_shard_16-), TypeName: "INT8"}
131131
│ │ ├── PUBLIC → ABSENT ColumnComputeExpression:{DescID: 108 (table_regional_by_row), ColumnID: 6 (crdb_internal_k_shard_16-), Usage: REGULAR}
132-
│ │ ├── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 4 (table_regional_by_row_v_idx-)}
132+
│ │ ├── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 4 (idx_v-)}
133133
│ │ ├── DELETE_ONLY → ABSENT TemporaryIndex:{DescID: 108 (table_regional_by_row), IndexID: 5, ConstraintID: 4, SourceIndexID: 8 (table_regional_by_row_pkey-)}
134134
│ │ └── PUBLIC → ABSENT IndexData:{DescID: 108 (table_regional_by_row), IndexID: 5}
135135
│ └── 15 Mutation operations

0 commit comments

Comments
 (0)