Skip to content

Commit 8dfdc02

Browse files
committed
sql: use new increment when restarting sequence
Previously, if a sequence were restarted and its increment updated in the same transaction (command), the old increment would be used to set the value in KV. This change corrects the `ALTER` DDL to use the new increment value. Informs: #142914 Informs: #21564 Epic: CRDB-31283 Release note (sql change): Restarting a sequence with an updated increment has the expected initial value.
1 parent ef3f07e commit 8dfdc02

File tree

3 files changed

+17
-14
lines changed

3 files changed

+17
-14
lines changed

pkg/sql/alter_sequence.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,13 @@ func alterSequenceImpl(
127127
}
128128

129129
setSequenceVal := func(val int64) error {
130+
// Setting the sequence value should always cause the operation to run
131+
// in the current transaction. This is achieved by treating the sequence
132+
// as if it were just created.
133+
if err := params.p.createdSequences.addCreatedSequence(seqDesc.ID); err != nil {
134+
return err
135+
}
136+
130137
err := params.p.txn.Put(params.ctx, seqValueKey, val)
131138
if err != nil {
132139
return err
@@ -148,7 +155,7 @@ func alterSequenceImpl(
148155
//
149156
// The code below handles the second case.
150157

151-
if opts.Increment < 0 && (oldIncrement != seqDesc.SequenceOpts.Increment || oldMinValue != seqDesc.SequenceOpts.MinValue) {
158+
if opts.Increment < 0 && (oldIncrement != opts.Increment || opts.MinValue < oldMinValue) {
152159
// Only get the sequence value from KV if it's needed.
153160
sequenceVal, err := getSequenceValue()
154161
if err != nil {
@@ -157,7 +164,7 @@ func alterSequenceImpl(
157164

158165
// If the sequence were never advanced, its current value is offset by the increment.
159166
if sequenceVal > oldStart {
160-
err := setSequenceVal(oldStart - seqDesc.SequenceOpts.Increment)
167+
err := setSequenceVal(oldStart - opts.Increment)
161168
if err != nil {
162169
return err
163170
}
@@ -174,14 +181,14 @@ func alterSequenceImpl(
174181
return err
175182
}
176183
}
177-
} else if opts.Increment > 0 && (oldIncrement != seqDesc.SequenceOpts.Increment || seqDesc.SequenceOpts.MaxValue > oldMaxValue) {
184+
} else if opts.Increment > 0 && (oldIncrement != opts.Increment || opts.MaxValue > oldMaxValue) {
178185
sequenceVal, err := getSequenceValue()
179186
if err != nil {
180187
return err
181188
}
182189

183190
if sequenceVal < oldStart {
184-
err := setSequenceVal(oldStart - seqDesc.SequenceOpts.Increment)
191+
err := setSequenceVal(oldStart - opts.Increment)
185192
if err != nil {
186193
return err
187194
}
@@ -209,13 +216,8 @@ func alterSequenceImpl(
209216
}
210217
}
211218
if restartVal != nil {
212-
// Using RESTART on a sequence should always cause the operation to run
213-
// in the current transaction. This is achieved by treating the sequence
214-
// as if it were just created.
215-
if err := params.p.createdSequences.addCreatedSequence(seqDesc.ID); err != nil {
216-
return err
217-
}
218-
if err := params.p.SetSequenceValueByID(params.ctx, uint32(seqDesc.ID), *restartVal, false); err != nil {
219+
err := setSequenceVal(*restartVal - opts.Increment)
220+
if err != nil {
219221
return err
220222
}
221223
}

pkg/sql/alter_table.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
gojson "encoding/json"
1212
"fmt"
1313
"sort"
14+
"strings"
1415
"time"
1516

1617
"github.com/cockroachdb/cockroach/pkg/build"
@@ -1267,7 +1268,7 @@ func applyColumnMutation(
12671268
return err
12681269
}
12691270

1270-
// Alter referenced sequence for identity with sepcified option.
1271+
// Alter referenced sequence for identity with specified option.
12711272
// Does not override existing values if not specified.
12721273
if err := alterSequenceImpl(params, seqDesc, t.SeqOptions, t); err != nil {
12731274
return err
@@ -1288,7 +1289,7 @@ func applyColumnMutation(
12881289
if opts.Virtual {
12891290
optsNode = append(optsNode, tree.SequenceOption{Name: tree.SeqOptVirtual})
12901291
}
1291-
s := tree.Serialize(&optsNode)
1292+
s := strings.TrimSpace(tree.Serialize(&optsNode))
12921293
col.ColumnDesc().GeneratedAsIdentitySequenceOption = &s
12931294

12941295
case *tree.AlterTableDropIdentity:

pkg/sql/logictest/testdata/logic_test/alter_table

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4436,8 +4436,8 @@ SELECT b from t_alter_identity ORDER BY b;
44364436
1
44374437
5
44384438
9
4439-
18
44404439
20
4440+
22
44414441

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

0 commit comments

Comments
 (0)