Skip to content

Commit e794b2c

Browse files
spilchenspilchen
committed
sql/schemachanger: support complex ALTER COLUMN … SET TYPE in DSC
Previously, complex column type changes that required a backfill reverted to the legacy schema changer. This update now enables support for such changes in the declarative schema changer (DSC). As with the legacy process, a complex type change involves dropping the column being modified and adding a new one with the updated type. A temporary compute expression is used to map the old column to the new one for backfilling. The process includes performing a double backfill for the primary key (PK): first, both the old and new columns are included in the PK, and then a second backfill is done after the old column is removed. This staged approach is required due to a rule that prevents dropping the column’s compute expression before the dependent column is removed, keeping the expression until after the first backfill. Since the old and new columns coexist temporarily, the old column is renamed to avoid ambiguity in the PK. A transient name is assigned to the dropped column (e.g., <col>_shadow), which is automatically cleaned up later. This ensures clear differentiation during the transition. A new field was also added to ColumnName to restore the original name if the operation is undone, avoiding the use of a placeholder. However, there are a few limitations with the current implementation: - No support for columns that already have a compute expression - No support for columns with ON UPDATE or DEFAULT expressions - No support for conversions to enum types - While column order is preserved in the catalog for SELECT * queries, the order within the column family is not maintained, with the new column being added at the end of the internal ordering. Some tests, designed for the legacy schema changer, couldn’t easily be adapted for the DSC. These limitations are planned to be addressed in future changes. Epic: CRDB-40232 Closes #127014 Release notes: none
1 parent d50ca11 commit e794b2c

File tree

48 files changed

+4091
-87
lines changed

Some content is hidden

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

48 files changed

+4091
-87
lines changed

‎pkg/ccl/changefeedccl/cdcevent/event_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,9 @@ func TestEventColumnOrderingWithSchemaChanges(t *testing.T) {
598598
sqlDB := sqlutils.MakeSQLRunner(db)
599599
// Use alter column type to force column reordering.
600600
sqlDB.Exec(t, `SET enable_experimental_alter_column_type_general = true`)
601+
// TODO(spilchen): force the legacy schema changer. When run with the DSC,
602+
// the ordering changes in the column family. This needs to be revisited in 133040.
603+
sqlDB.Exec(t, `SET use_declarative_schema_changer = 'off'`)
601604

602605
type decodeExpectation struct {
603606
expectUnwatchedErr bool

‎pkg/ccl/schemachangerccl/backup_base_generated_test.go‎

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/ccl/schemachangerccl/testdata/decomp/multiregion‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,31 +190,37 @@ ElementState:
190190
tableId: 110
191191
Status: PUBLIC
192192
- ColumnName:
193+
absentName: ""
193194
columnId: 1
194195
name: b
195196
tableId: 110
196197
Status: PUBLIC
197198
- ColumnName:
199+
absentName: ""
198200
columnId: 2
199201
name: rowid
200202
tableId: 110
201203
Status: PUBLIC
202204
- ColumnName:
205+
absentName: ""
203206
columnId: 4.294967292e+09
204207
name: crdb_internal_origin_timestamp
205208
tableId: 110
206209
Status: PUBLIC
207210
- ColumnName:
211+
absentName: ""
208212
columnId: 4.294967293e+09
209213
name: crdb_internal_origin_id
210214
tableId: 110
211215
Status: PUBLIC
212216
- ColumnName:
217+
absentName: ""
213218
columnId: 4.294967294e+09
214219
name: tableoid
215220
tableId: 110
216221
Status: PUBLIC
217222
- ColumnName:
223+
absentName: ""
218224
columnId: 4.294967295e+09
219225
name: crdb_internal_mvcc_timestamp
220226
tableId: 110
@@ -579,31 +585,37 @@ ElementState:
579585
tableId: 109
580586
Status: PUBLIC
581587
- ColumnName:
588+
absentName: ""
582589
columnId: 1
583590
name: a
584591
tableId: 109
585592
Status: PUBLIC
586593
- ColumnName:
594+
absentName: ""
587595
columnId: 2
588596
name: rowid
589597
tableId: 109
590598
Status: PUBLIC
591599
- ColumnName:
600+
absentName: ""
592601
columnId: 4.294967292e+09
593602
name: crdb_internal_origin_timestamp
594603
tableId: 109
595604
Status: PUBLIC
596605
- ColumnName:
606+
absentName: ""
597607
columnId: 4.294967293e+09
598608
name: crdb_internal_origin_id
599609
tableId: 109
600610
Status: PUBLIC
601611
- ColumnName:
612+
absentName: ""
602613
columnId: 4.294967294e+09
603614
name: tableoid
604615
tableId: 109
605616
Status: PUBLIC
606617
- ColumnName:
618+
absentName: ""
607619
columnId: 4.294967295e+09
608620
name: crdb_internal_mvcc_timestamp
609621
tableId: 109
@@ -997,36 +1009,43 @@ ElementState:
9971009
tableId: 108
9981010
Status: PUBLIC
9991011
- ColumnName:
1012+
absentName: ""
10001013
columnId: 1
10011014
name: k
10021015
tableId: 108
10031016
Status: PUBLIC
10041017
- ColumnName:
1018+
absentName: ""
10051019
columnId: 2
10061020
name: v
10071021
tableId: 108
10081022
Status: PUBLIC
10091023
- ColumnName:
1024+
absentName: ""
10101025
columnId: 3
10111026
name: crdb_region
10121027
tableId: 108
10131028
Status: PUBLIC
10141029
- ColumnName:
1030+
absentName: ""
10151031
columnId: 4.294967292e+09
10161032
name: crdb_internal_origin_timestamp
10171033
tableId: 108
10181034
Status: PUBLIC
10191035
- ColumnName:
1036+
absentName: ""
10201037
columnId: 4.294967293e+09
10211038
name: crdb_internal_origin_id
10221039
tableId: 108
10231040
Status: PUBLIC
10241041
- ColumnName:
1042+
absentName: ""
10251043
columnId: 4.294967294e+09
10261044
name: tableoid
10271045
tableId: 108
10281046
Status: PUBLIC
10291047
- ColumnName:
1048+
absentName: ""
10301049
columnId: 4.294967295e+09
10311050
name: crdb_internal_mvcc_timestamp
10321051
tableId: 108

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,36 +111,43 @@ ElementState:
111111
tableId: 104
112112
Status: PUBLIC
113113
- ColumnName:
114+
absentName: ""
114115
columnId: 1
115116
name: pk
116117
tableId: 104
117118
Status: PUBLIC
118119
- ColumnName:
120+
absentName: ""
119121
columnId: 2
120122
name: a
121123
tableId: 104
122124
Status: PUBLIC
123125
- ColumnName:
126+
absentName: ""
124127
columnId: 3
125128
name: j
126129
tableId: 104
127130
Status: PUBLIC
128131
- ColumnName:
132+
absentName: ""
129133
columnId: 4.294967292e+09
130134
name: crdb_internal_origin_timestamp
131135
tableId: 104
132136
Status: PUBLIC
133137
- ColumnName:
138+
absentName: ""
134139
columnId: 4.294967293e+09
135140
name: crdb_internal_origin_id
136141
tableId: 104
137142
Status: PUBLIC
138143
- ColumnName:
144+
absentName: ""
139145
columnId: 4.294967294e+09
140146
name: tableoid
141147
tableId: 104
142148
Status: PUBLIC
143149
- ColumnName:
150+
absentName: ""
144151
columnId: 4.294967295e+09
145152
name: crdb_internal_mvcc_timestamp
146153
tableId: 104
@@ -617,31 +624,37 @@ ElementState:
617624
tableId: 105
618625
Status: PUBLIC
619626
- ColumnName:
627+
absentName: ""
620628
columnId: 1
621629
name: a
622630
tableId: 105
623631
Status: PUBLIC
624632
- ColumnName:
633+
absentName: ""
625634
columnId: 2
626635
name: b
627636
tableId: 105
628637
Status: PUBLIC
629638
- ColumnName:
639+
absentName: ""
630640
columnId: 4.294967292e+09
631641
name: crdb_internal_origin_timestamp
632642
tableId: 105
633643
Status: PUBLIC
634644
- ColumnName:
645+
absentName: ""
635646
columnId: 4.294967293e+09
636647
name: crdb_internal_origin_id
637648
tableId: 105
638649
Status: PUBLIC
639650
- ColumnName:
651+
absentName: ""
640652
columnId: 4.294967294e+09
641653
name: tableoid
642654
tableId: 105
643655
Status: PUBLIC
644656
- ColumnName:
657+
absentName: ""
645658
columnId: 4.294967295e+09
646659
name: crdb_internal_mvcc_timestamp
647660
tableId: 105

‎pkg/sql/alter_column_type.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ import (
3232
var AlterColTypeInTxnNotSupportedErr = unimplemented.NewWithIssuef(
3333
49351, "ALTER COLUMN TYPE is not supported inside a transaction")
3434

35-
var alterColTypeInCombinationNotSupportedErr = unimplemented.NewWithIssuef(
36-
49351, "ALTER COLUMN TYPE cannot be used in combination "+
37-
"with other ALTER TABLE commands")
38-
3935
// AlterColumnType takes an AlterTableAlterColumnType, determines
4036
// which conversion to use and applies the type conversion.
4137
func AlterColumnType(
@@ -216,7 +212,7 @@ func alterColumnTypeGeneral(
216212
}
217213

218214
if len(cmds) > 1 {
219-
return alterColTypeInCombinationNotSupportedErr
215+
return sqlerrors.NewAlterColTypeInCombinationNotSupportedError()
220216
}
221217

222218
// Disallow ALTER COLUMN TYPE general if the table is already undergoing

‎pkg/sql/alter_column_type_test.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ INSERT INTO test2 VALUES ('hello');`)
7575
go func() {
7676
sqlDB.ExecMultiple(t,
7777
`SET enable_experimental_alter_column_type_general = true;`,
78+
// TODO(spilchen): This test is designed for the legacy schema changer.
79+
// Update it for the declarative schema changer (DSC).
80+
`SET use_declarative_schema_changer = 'off';`,
7881
`ALTER TABLE test ALTER COLUMN x TYPE STRING;`)
7982
wg.Done()
8083
}()
@@ -151,6 +154,9 @@ INSERT INTO test2 VALUES (true);
151154
go func() {
152155
sqlDB.ExecMultiple(t,
153156
`SET enable_experimental_alter_column_type_general = true;`,
157+
// TODO(spilchen): This test is designed for the legacy schema changer.
158+
// Update it for the declarative schema changer (DSC).
159+
`SET use_declarative_schema_changer = 'off';`,
154160
`ALTER TABLE test ALTER COLUMN x TYPE BOOL USING (x > 0);`)
155161
wg.Done()
156162
}()
@@ -203,7 +209,10 @@ func TestVisibilityDuringAlterColumnType(t *testing.T) {
203209
sqlDB := sqlutils.MakeSQLRunner(db)
204210
defer s.Stopper().Stop(ctx)
205211

206-
sqlDB.Exec(t, `SET enable_experimental_alter_column_type_general = true;`)
212+
sqlDB.ExecMultiple(t, `SET enable_experimental_alter_column_type_general = true;`,
213+
// TODO(spilchen): This test is designed for the legacy schema changer.
214+
// Update it for the declarative schema changer (DSC).
215+
`SET use_declarative_schema_changer = 'off'`)
207216

208217
sqlDB.Exec(t, `
209218
CREATE DATABASE t;
@@ -259,7 +268,7 @@ func TestAlterColumnTypeFailureRollback(t *testing.T) {
259268
sqlDB.Exec(t, `CREATE TABLE t.test (x STRING);`)
260269
sqlDB.Exec(t, `INSERT INTO t.test VALUES ('1'), ('2'), ('HELLO');`)
261270

262-
expected := "pq: could not parse \"HELLO\" as type int: strconv.ParseInt: parsing \"HELLO\": invalid syntax"
271+
expected := "pq: failed to construct index entries during backfill: could not parse \"HELLO\" as type int: strconv.ParseInt: parsing \"HELLO\": invalid syntax"
263272
sqlDB.ExpectErr(t, expected, `ALTER TABLE t.test ALTER COLUMN x TYPE INT USING x::INT8;`)
264273

265274
// Ensure that the add column and column swap mutations are cleaned up.

0 commit comments

Comments
 (0)