Skip to content

Commit a7020d6

Browse files
bghalbghal
committed
sql: alter column identity with the declarative schema changer
Adds support for the `ALTER COLUMN ... [RESTART | SET [CACHE | CYCLE | ... ]` identity operations in the declarative schema changer. Fixes: #142914 Epic: CRDB-31283 Release note (sql change): The `ALTER COLUMN ...` sequence identity commands are run by the declarative schema changer. Release note (bug fix): Simultaneously restarting the identity sequence and changing the increment sets the correct initial value.
1 parent e1d390d commit a7020d6

File tree

84 files changed

+704
-376
lines changed

Some content is hidden

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

84 files changed

+704
-376
lines changed

‎pkg/sql/alter_sequence.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func alterSequenceImpl(
148148
//
149149
// The code below handles the second case.
150150

151-
if opts.Increment < 0 && (oldIncrement != seqDesc.SequenceOpts.Increment || oldMinValue != seqDesc.SequenceOpts.MinValue) {
151+
if opts.Increment < 0 && (oldIncrement != seqDesc.SequenceOpts.Increment || seqDesc.SequenceOpts.MinValue < oldMinValue) {
152152
// Only get the sequence value from KV if it's needed.
153153
sequenceVal, err := getSequenceValue()
154154
if err != nil {

‎pkg/sql/catalog/schemaexpr/sequence_options.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,3 +274,21 @@ func AssignSequenceOptions(
274274
}
275275
return nil
276276
}
277+
278+
// DefaultSequenceOptions is a helper that returns the default sequence options.
279+
// It panics on error.
280+
func DefaultSequenceOptions() descpb.TableDescriptor_SequenceOpts {
281+
defaultOpts := descpb.TableDescriptor_SequenceOpts{
282+
Increment: 1,
283+
}
284+
if err := AssignSequenceOptions(&defaultOpts,
285+
nil, /* optsNode */
286+
64,
287+
true, /* setDefaults */
288+
nil, /* existingTypes */
289+
); err != nil {
290+
panic(err)
291+
}
292+
293+
return defaultOpts
294+
}

‎pkg/sql/logictest/testdata/logic_test/alter_table‎

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4368,16 +4368,20 @@ SET create_table_with_schema_locked=false
43684368
statement ok
43694369
CREATE TABLE t_alter_identity (id SERIAL PRIMARY KEY, a int GENERATED ALWAYS AS IDENTITY, b int GENERATED ALWAYS AS IDENTITY, z int);
43704370

4371+
skipif config local-mixed-25.4 local-mixed-25.3 local-mixed-25.2
43714372
statement error pq: INCREMENT must not be zero
43724373
ALTER TABLE t_alter_identity ALTER COLUMN b SET INCREMENT 0;
43734374

4375+
skipif config local-mixed-25.4 local-mixed-25.3 local-mixed-25.2
43744376
query error pq: START value \(11\) cannot be greater than MAXVALUE \(10\)
43754377
ALTER TABLE t_alter_identity ALTER COLUMN b SET MAXVALUE 10 SET START WITH 11;
43764378

4379+
skipif config local-mixed-25.4 local-mixed-25.3 local-mixed-25.2
43774380
query error pq: START value \(5\) cannot be less than MINVALUE \(10\)
43784381
ALTER TABLE t_alter_identity ALTER COLUMN b SET MINVALUE 10 SET START WITH 5;
43794382

4380-
query error pq: column "z" of relation "t_alter_identity" is not an identity column
4383+
skipif config local-mixed-25.4 local-mixed-25.3 local-mixed-25.2
4384+
query error pq: cannot alter identity of a non-sequence column "z"
43814385
ALTER TABLE t_alter_identity ALTER COLUMN z SET START WITH 5;
43824386

43834387
# Verify sequence options are implemented on the identity
@@ -4401,22 +4405,91 @@ statement ok
44014405
INSERT INTO t_alter_identity DEFAULT VALUES;
44024406

44034407
statement ok
4404-
ALTER TABLE t_alter_identity ALTER COLUMN b SET MAXVALUE 40 RESTART WITH 20 SET CACHE 5 SET INCREMENT BY 2
4408+
ALTER TABLE t_alter_identity ALTER COLUMN b SET MAXVALUE 40 RESTART WITH 20 SET CACHE 5 SET INCREMENT BY 2;
44054409

44064410
statement ok
44074411
INSERT INTO t_alter_identity DEFAULT VALUES;
44084412

44094413
statement ok
44104414
INSERT INTO t_alter_identity DEFAULT VALUES;
44114415

4416+
skipif config local-mixed-25.4 local-mixed-25.3 local-mixed-25.2
44124417
query I rowsort
44134418
SELECT b from t_alter_identity ORDER BY b;
44144419
----
44154420
1
44164421
5
44174422
9
4418-
18
44194423
20
4424+
22
4425+
4426+
# TODO(21564): Enable this test when `ALTER COLUMN` on options updates the value of a sequence
4427+
# subtest issue-52552
4428+
#
4429+
# statement ok
4430+
# CREATE TABLE test_52552_asc (c1 int GENERATED ALWAYS AS IDENTITY);
4431+
# ALTER TABLE test_52552_asc ALTER COLUMN c1 SET INCREMENT 3 SET MINVALUE 1 SET MAXVALUE 12;
4432+
# ALTER TABLE test_52552_asc ALTER COLUMN c1 SET INCREMENT 8 SET MINVALUE 1 SET MAXVALUE 12;
4433+
# INSERT INTO test_52552_asc DEFAULT VALUES;
4434+
#
4435+
# query I
4436+
# SELECT c1 FROM test_52552_asc
4437+
# ----
4438+
# 8
4439+
#
4440+
# statement error pq: reached maximum value of sequence "test_52552_asc_c1_seq" \(12\)
4441+
# INSERT INTO test_52552_asc DEFAULT VALUES;
4442+
#
4443+
# statement error pq: reached maximum value of sequence "test_52552_asc_c1_seq" \(12\)
4444+
# INSERT INTO test_52552_asc DEFAULT VALUES;
4445+
#
4446+
# statement ok
4447+
# ALTER TABLE test_52552_asc ALTER COLUMN c1 SET NO MAXVALUE;
4448+
# INSERT INTO test_52552_asc DEFAULT VALUES;
4449+
#
4450+
# query I nosort
4451+
# SELECT c1 FROM test_52552_asc
4452+
# ----
4453+
# 8
4454+
# 32
4455+
#
4456+
# statement ok
4457+
# CREATE TABLE test_52552_desc (c1 int GENERATED ALWAYS AS IDENTITY);
4458+
# ALTER TABLE test_52552_desc ALTER COLUMN c1 SET INCREMENT -5 SET MINVALUE 1 SET MAXVALUE 12;
4459+
# ALTER TABLE test_52552_desc ALTER COLUMN c1 SET INCREMENT -8 SET MINVALUE 1 SET MAXVALUE 12;
4460+
# INSERT INTO test_52552_desc DEFAULT VALUES;
4461+
#
4462+
# query I
4463+
# SELECT c1 FROM test_52552_desc
4464+
# ----
4465+
# 8
4466+
#
4467+
# statement error pq: reached minimum value of sequence "test_52552_desc_seq" \(1\)
4468+
# INSERT INTO test_52552_desc DEFAULT VALUES;
4469+
#
4470+
# statement error pq: reached minimum value of sequence "test_52552_desc_seq" \(1\)
4471+
# INSERT INTO test_52552_desc DEFAULT VALUES;
4472+
#
4473+
# statement ok
4474+
# ALTER TABLE test_52552_desc ALTER COLUMN c1 SET NO MINVALUE;
4475+
# INSERT INTO test_52552_desc DEFAULT VALUES;
4476+
#
4477+
# query I nosort
4478+
# SELECT c1 FROM test_52552_desc
4479+
# ----
4480+
# -4
4481+
#
4482+
# statement ok
4483+
# CREATE TABLE test_52552_start INCREMENT BY 3 MINVALUE 1 MAXVALUE 100 START 50;
4484+
# ALTER SEQUENCE test_52552_start INCREMENT BY 8;
4485+
# INSERT INTO test_52552_desc DEFAULT VALUES;
4486+
#
4487+
# query I
4488+
# SELECT nextval('test_52552_start');
4489+
# ----
4490+
# 50
4491+
#
4492+
# subtest end
44204493

44214494
statement ok
44224495
CREATE TABLE t_identity_drop (a int GENERATED ALWAYS AS IDENTITY (START WITH 10), b int GENERATED BY DEFAULT AS IDENTITY);

‎pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ go_library(
1010
"alter_table_alter_column_add_identity.go",
1111
"alter_table_alter_column_drop_not_null.go",
1212
"alter_table_alter_column_drop_stored.go",
13+
"alter_table_alter_column_identity.go",
1314
"alter_table_alter_column_set_default.go",
1415
"alter_table_alter_column_set_identity.go",
1516
"alter_table_alter_column_set_not_null.go",

‎pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{
4747
reflect.TypeOf((*tree.AlterTableRenameConstraint)(nil)): {fn: alterTableRenameConstraint, on: true, checks: isV261Active},
4848
reflect.TypeOf((*tree.AlterTableSetIdentity)(nil)): {fn: alterTableSetIdentity, on: true, checks: isV261Active},
4949
reflect.TypeOf((*tree.AlterTableAddIdentity)(nil)): {fn: alterTableAddIdentity, on: true, checks: isV261Active},
50+
reflect.TypeOf((*tree.AlterTableIdentity)(nil)): {fn: alterTableAlterColumnIdentity, on: true, checks: isV261Active},
5051
}
5152

5253
func init() {

‎pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func alterTableAddColumn(
293293
seqOptions = *ptr
294294
}
295295
// Versions from 26.1 GeneratedAsIdentity will have a separate element for
296-
// GeneratedAsIdentity. Older versios store it in the column element.
296+
// GeneratedAsIdentity. Older versions store it in the column element.
297297
if spec.colType.ElementCreationMetadata.In_26_1OrLater {
298298
spec.generatedAsID = &scpb.ColumnGeneratedAsIdentity{
299299
TableID: tbl.TableID,

‎pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_column_identity.go‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package scbuildstmt
7+
8+
import (
9+
"fmt"
10+
"reflect"
11+
"strconv"
12+
"strings"
13+
14+
"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr"
15+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
16+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
17+
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
18+
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
19+
)
20+
21+
func alterTableAlterColumnIdentity(
22+
b BuildCtx, tn *tree.TableName, tbl *scpb.Table, stmt tree.Statement, t *tree.AlterTableIdentity,
23+
) {
24+
alterColumnPreChecks(b, tn, tbl, t.Column)
25+
26+
columnID := getColumnIDFromColumnName(b, tbl.TableID, t.Column, true /* required */)
27+
// Block alters on system columns.
28+
column := mustRetrieveColumnElem(b, tbl.TableID, columnID)
29+
panicIfSystemColumn(column, t.Column)
30+
31+
sequenceOwner := b.QueryByID(tbl.TableID).FilterSequenceOwner().Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.SequenceOwner) bool {
32+
return e.TableID == tbl.TableID && e.ColumnID == columnID
33+
}).MustGetZeroOrOneElement()
34+
if sequenceOwner == nil {
35+
panic(pgerror.Newf(
36+
pgcode.FeatureNotSupported,
37+
"cannot alter identity of a non-sequence column %q", tree.ErrString(&t.Column)))
38+
}
39+
40+
// Get the current sequence options from elements and defaults.
41+
currentOpts := schemaexpr.DefaultSequenceOptions()
42+
b.QueryByID(sequenceOwner.SequenceID).FilterSequenceOption().ForEach(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.SequenceOption) {
43+
var err error
44+
switch name := e.Key; name {
45+
case tree.SeqOptAs:
46+
currentOpts.AsIntegerType = e.Value
47+
case tree.SeqOptCacheNode:
48+
currentOpts.NodeCacheSize, err = strconv.ParseInt(e.Value, 10, 64)
49+
if err != nil {
50+
panic(pgerror.Wrapf(err, pgcode.Internal, "invalid sequence option value for %q", name))
51+
}
52+
case tree.SeqOptCacheSession:
53+
currentOpts.SessionCacheSize, err = strconv.ParseInt(e.Value, 10, 64)
54+
if err != nil {
55+
panic(pgerror.Wrapf(err, pgcode.Internal, "invalid sequence option value for %q", name))
56+
}
57+
case tree.SeqOptIncrement:
58+
currentOpts.Increment, err = strconv.ParseInt(e.Value, 10, 64)
59+
if err != nil {
60+
panic(pgerror.Wrapf(err, pgcode.Internal, "invalid sequence option value for %q", name))
61+
}
62+
case tree.SeqOptMinValue:
63+
currentOpts.MinValue, err = strconv.ParseInt(e.Value, 10, 64)
64+
if err != nil {
65+
panic(pgerror.Wrapf(err, pgcode.Internal, "invalid sequence option value for %q", name))
66+
}
67+
case tree.SeqOptMaxValue:
68+
currentOpts.MaxValue, err = strconv.ParseInt(e.Value, 10, 64)
69+
if err != nil {
70+
panic(pgerror.Wrapf(err, pgcode.Internal, "invalid sequence option value for %q", name))
71+
}
72+
case tree.SeqOptStart:
73+
currentOpts.Start, err = strconv.ParseInt(e.Value, 10, 64)
74+
if err != nil {
75+
panic(pgerror.Wrapf(err, pgcode.Internal, "invalid sequence option value for %q", name))
76+
}
77+
case tree.SeqOptVirtual:
78+
currentOpts.Virtual, err = strconv.ParseBool(e.Value)
79+
if err != nil {
80+
panic(pgerror.Wrapf(err, pgcode.Internal, "invalid sequence option value for %q", name))
81+
}
82+
default:
83+
panic(pgerror.Newf(pgcode.Internal, "unexpected sequence option %q", name))
84+
}
85+
})
86+
87+
// And the final state for the sequence options.
88+
updatedOpts := currentOpts
89+
if err := schemaexpr.AssignSequenceOptions(&updatedOpts,
90+
t.SeqOptions,
91+
64,
92+
false, /* setDefaults */
93+
nil, /* existingTypes */
94+
); err != nil {
95+
panic(pgerror.Wrap(
96+
err,
97+
pgcode.FeatureNotSupported,
98+
"", /* message */
99+
))
100+
}
101+
102+
defaultOpts := schemaexpr.DefaultSequenceOptions()
103+
104+
updateElement := func(key string, defaultValue, value interface{}) {
105+
newSeqOption := scpb.SequenceOption{
106+
SequenceID: sequenceOwner.SequenceID,
107+
Key: key,
108+
Value: fmt.Sprintf("%v", value),
109+
}
110+
111+
oldSeqOption := b.QueryByID(sequenceOwner.SequenceID).FilterSequenceOption().Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.SequenceOption) bool {
112+
return e.Key == key
113+
}).MustGetZeroOrOneElement()
114+
if oldSeqOption != nil {
115+
// Skip a noop update.
116+
if oldSeqOption.Value == newSeqOption.Value {
117+
return
118+
}
119+
b.Drop(oldSeqOption)
120+
}
121+
122+
// Skip setting to default values.
123+
if reflect.DeepEqual(defaultValue, value) {
124+
return
125+
}
126+
127+
b.Add(&newSeqOption)
128+
}
129+
130+
var updateSequenceValue bool
131+
var restartWith int64
132+
var useRestartWith bool
133+
for _, opt := range t.SeqOptions {
134+
switch name := opt.Name; name {
135+
case tree.SeqOptAs:
136+
updateElement(name, defaultOpts.AsIntegerType, updatedOpts.AsIntegerType)
137+
case tree.SeqOptCacheNode:
138+
updateElement(name, defaultOpts.NodeCacheSize, updatedOpts.NodeCacheSize)
139+
case tree.SeqOptCacheSession:
140+
updateElement(name, defaultOpts.SessionCacheSize, updatedOpts.SessionCacheSize)
141+
case tree.SeqOptIncrement:
142+
updateElement(name, defaultOpts.Increment, updatedOpts.Increment)
143+
updateSequenceValue = true
144+
case tree.SeqOptMinValue:
145+
updateElement(name, defaultOpts.MinValue, updatedOpts.MinValue)
146+
updateSequenceValue = true
147+
case tree.SeqOptMaxValue:
148+
updateElement(name, defaultOpts.MaxValue, updatedOpts.MaxValue)
149+
updateSequenceValue = true
150+
case tree.SeqOptStart:
151+
updateElement(name, defaultOpts.Start, updatedOpts.Start)
152+
updateSequenceValue = true
153+
case tree.SeqOptVirtual:
154+
updateElement(name, defaultOpts.Virtual, updatedOpts.Virtual)
155+
case tree.SeqOptRestart:
156+
useRestartWith = true
157+
if opt.IntVal != nil {
158+
restartWith = *opt.IntVal
159+
} else {
160+
restartWith = updatedOpts.Start
161+
}
162+
default:
163+
panic(fmt.Sprintf("unexpected sequence option: %q", name))
164+
}
165+
}
166+
167+
if useRestartWith {
168+
restartSeqOption := scpb.SequenceOption{
169+
SequenceID: sequenceOwner.SequenceID,
170+
Key: tree.SeqOptRestart,
171+
Value: fmt.Sprintf("%v", restartWith-updatedOpts.Increment),
172+
}
173+
b.AddTransient(&restartSeqOption)
174+
} else if updateSequenceValue {
175+
// TODO(21564): Update the sequence value when changes in options expose implementation.
176+
}
177+
178+
var seqOptions tree.SequenceOptions
179+
if updatedOpts.SessionCacheSize != defaultOpts.SessionCacheSize {
180+
seqOptions = append(seqOptions, tree.SequenceOption{Name: tree.SeqOptCacheSession, IntVal: &updatedOpts.SessionCacheSize})
181+
}
182+
if updatedOpts.NodeCacheSize != defaultOpts.NodeCacheSize {
183+
seqOptions = append(seqOptions, tree.SequenceOption{Name: tree.SeqOptCacheNode, IntVal: &updatedOpts.NodeCacheSize})
184+
}
185+
if updatedOpts.MinValue != defaultOpts.MinValue {
186+
seqOptions = append(seqOptions, tree.SequenceOption{Name: tree.SeqOptMinValue, IntVal: &updatedOpts.MinValue})
187+
}
188+
if updatedOpts.MaxValue != defaultOpts.MaxValue {
189+
seqOptions = append(seqOptions, tree.SequenceOption{Name: tree.SeqOptMaxValue, IntVal: &updatedOpts.MaxValue})
190+
}
191+
if updatedOpts.Increment != defaultOpts.Increment {
192+
seqOptions = append(seqOptions, tree.SequenceOption{Name: tree.SeqOptIncrement, IntVal: &updatedOpts.Increment})
193+
}
194+
if updatedOpts.Start != defaultOpts.Start {
195+
seqOptions = append(seqOptions, tree.SequenceOption{Name: tree.SeqOptStart, IntVal: &updatedOpts.Start})
196+
}
197+
if updatedOpts.Virtual != defaultOpts.Virtual {
198+
seqOptions = append(seqOptions, tree.SequenceOption{Name: tree.SeqOptVirtual})
199+
}
200+
seqOptionsString := strings.TrimSpace(tree.Serialize(&seqOptions))
201+
202+
columnType := b.QueryByID(tbl.TableID).FilterColumnType().Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.ColumnType) bool {
203+
return e.ColumnID == sequenceOwner.ColumnID
204+
}).MustGetOneElement()
205+
206+
if columnType.ElementCreationMetadata.In_26_1OrLater {
207+
oldColumnGeneratedAsIdentity := b.QueryByID(tbl.TableID).FilterColumnGeneratedAsIdentity().Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.ColumnGeneratedAsIdentity) bool {
208+
return e.ColumnID == sequenceOwner.ColumnID
209+
}).MustGetOneElement()
210+
211+
if oldColumnGeneratedAsIdentity.SequenceOption != seqOptionsString {
212+
b.Drop(oldColumnGeneratedAsIdentity)
213+
newColumnGeneratedAsIdentity := *oldColumnGeneratedAsIdentity
214+
newColumnGeneratedAsIdentity.SequenceOption = seqOptionsString
215+
b.Add(&newColumnGeneratedAsIdentity)
216+
}
217+
} else {
218+
if column.GeneratedAsIdentitySequenceOption != seqOptionsString {
219+
b.Drop(column)
220+
newColumn := *column
221+
newColumn.GeneratedAsIdentitySequenceOption = seqOptionsString
222+
b.Add(&newColumn)
223+
}
224+
}
225+
}

0 commit comments

Comments
 (0)