Merge #154489

154489: sql: fix the starting values of `ALTER`-ed sequences r=bghal a=bghal

When altering the minvalue, maxvalue, or increment, a sequence that had
either never been advanced with `nextval` or had been exhausted would be
set with an incorrect current value. This change better aligns the
`ALTER SEQUENCE` behavior to postgres.

Fixes: #52552
Informs: #154413, #21564

Release note (sql change): The value of a sequence after an `ALTER
SEQUENCE` is now consistent with a sequence created with those values.


Co-authored-by: Brendan Gerrity <[email protected]>

Author: craig[bot]

pkg/sql/alter_sequence.go

@@ -77,7 +77,7 @@ func (n *alterSequenceNode) startExec(params runParams) error {
 }
 
 // alterSequenceImpl applies given tree.SequenceOptions to the specified sequence descriptor.
-// Exisiting sequence options are not overridden with default values.
+// Existing sequence options are not overridden with default values.
 func alterSequenceImpl(
 	params runParams,
 	seqDesc *tabledesc.Mutable,
@@ -86,6 +86,8 @@ func alterSequenceImpl(
 ) error {
 	oldMinValue := seqDesc.SequenceOpts.MinValue
 	oldMaxValue := seqDesc.SequenceOpts.MaxValue
+	oldIncrement := seqDesc.SequenceOpts.Increment
+	oldStart := seqDesc.SequenceOpts.Start
 
 	existingType := types.Int
 	if seqDesc.GetSequenceOpts().AsIntegerType != "" {
@@ -120,45 +122,80 @@ func alterSequenceImpl(
 		if err != nil {
 			return 0, err
 		}
+
 		return kv.ValueInt(), nil
 	}
 
-	// Due to the semantics of sequence caching (see sql.planner.incrementSequenceUsingCache()),
-	// it is possible for a sequence to have a value that exceeds its MinValue or MaxValue. Users
-	// do no see values extending the sequence's bounds, and instead see "bounds exceeded" errors.
-	// To make a usable again after exceeding its bounds, there are two options:
+	setSequenceVal := func(val int64) error {
+		err := params.p.txn.Put(params.ctx, seqValueKey, val)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}
+
+	// Due to the semantics of sequence caching (see sql.planner.incrementSequenceUsingCache()), it
+	// is possible for a sequence to have a value that exceeds its MinValue or MaxValue. Users do
+	// not see values beyond the sequence's bounds, and instead see "bounds exceeded" errors. To
+	// make a sequence usable again after exceeding its bounds, there are two options:
+	//
 	// 1. The user changes the sequence's value by calling setval(...)
-	// 2. The user performs a schema change to alter the sequences MinValue or MaxValue. In this case, the
-	// value of the sequence must be restored to the original MinValue or MaxValue transactionally.
+	//
+	// 2. The user performs a schema change to alter the sequence's MinValue, MaxValue, or
+	// Increment. In this case, the value of the sequence must be (transactionally) restored to a
+	// value within MinValue and MaxValue.
+	//
 	// The code below handles the second case.
 
-	// The sequence is decreasing and the minvalue is being decreased.
-	if opts.Increment < 0 && seqDesc.SequenceOpts.MinValue < oldMinValue {
+	if opts.Increment < 0 && (oldIncrement != seqDesc.SequenceOpts.Increment || oldMinValue != seqDesc.SequenceOpts.MinValue) {
+		// Only get the sequence value from KV if it's needed.
 		sequenceVal, err := getSequenceValue()
 		if err != nil {
 			return err
 		}
 
-		// If the sequence exceeded the old MinValue, it must be changed to start at the old MinValue.
+		// If the sequence were never advanced, its current value is offset by the increment.
+		if sequenceVal > oldStart {
+			err := setSequenceVal(oldStart - seqDesc.SequenceOpts.Increment)
+			if err != nil {
+				return err
+			}
+		}
+
+		// If the sequence were exhausted, it would be beyond its previous bounds.
 		if sequenceVal < oldMinValue {
-			err := params.p.txn.Put(params.ctx, seqValueKey, oldMinValue)
+			// Every call to nextval increments the sequence even if the
+			// sequence is exhausted. Find the final valid value of the sequence
+			// by calculating the number of valid calls to it
+			calls := (oldMinValue - oldStart) / oldIncrement
+			err := setSequenceVal(oldStart + calls*oldIncrement)
 			if err != nil {
 				return err
 			}
 		}
-	} else if opts.Increment > 0 && seqDesc.SequenceOpts.MaxValue > oldMaxValue {
+	} else if opts.Increment > 0 && (oldIncrement != seqDesc.SequenceOpts.Increment || seqDesc.SequenceOpts.MaxValue > oldMaxValue) {
 		sequenceVal, err := getSequenceValue()
 		if err != nil {
 			return err
 		}
 
+		if sequenceVal < oldStart {
+			err := setSequenceVal(oldStart - seqDesc.SequenceOpts.Increment)
+			if err != nil {
+				return err
+			}
+		}
+
 		if sequenceVal > oldMaxValue {
-			err := params.p.txn.Put(params.ctx, seqValueKey, oldMaxValue)
+			calls := (oldMaxValue - oldStart) / oldIncrement
+			err := setSequenceVal(oldStart + calls*oldIncrement)
 			if err != nil {
 				return err
 			}
 		}
 	}
+
 	var restartVal *int64
 	for _, option := range seqOptions {
 		if option.Name == tree.SeqOptRestart {

pkg/sql/logictest/testdata/logic_test/alter_sequence

@@ -395,3 +395,64 @@ SHOW CREATE SEQUENCE seq_alter_no_min_max_des
 ----
 table_name                create_statement
 seq_alter_no_min_max_des  CREATE SEQUENCE public.seq_alter_no_min_max_des MINVALUE -9223372036854775808 MAXVALUE -1 INCREMENT -1 START -5;
+
+subtest issue-52552
+
+statement ok
+CREATE SEQUENCE test_52552_asc INCREMENT BY 3 MINVALUE 1 MAXVALUE 12;
+ALTER SEQUENCE test_52552_asc INCREMENT BY 8 MINVALUE 1 MAXVALUE 12;
+
+query II
+SELECT nextval('test_52552_asc'), nextval('test_52552_asc');
+----
+1  9
+
+statement error pq: reached maximum value of sequence "test_52552_asc" \(12\)
+SELECT nextval('test_52552_asc');
+
+statement error pq: reached maximum value of sequence "test_52552_asc" \(12\)
+SELECT nextval('test_52552_asc');
+
+statement ok
+ALTER SEQUENCE test_52552_asc NO MAXVALUE;
+
+query I
+SELECT nextval('test_52552_asc');
+----
+17
+
+statement ok
+CREATE SEQUENCE test_52552_desc INCREMENT BY -5 MINVALUE 1 MAXVALUE 12;
+ALTER SEQUENCE test_52552_desc INCREMENT BY -8 MINVALUE 1 MAXVALUE 12;
+
+query II
+SELECT nextval('test_52552_desc'), nextval('test_52552_desc');
+----
+12  4
+
+statement error pq: reached minimum value of sequence "test_52552_desc" \(1\)
+SELECT nextval('test_52552_desc');
+
+statement error pq: reached minimum value of sequence "test_52552_desc" \(1\)
+SELECT nextval('test_52552_desc');
+
+statement ok
+ALTER SEQUENCE test_52552_desc NO MINVALUE;
+
+query I
+SELECT nextval('test_52552_desc');
+----
+-4
+
+statement ok
+CREATE SEQUENCE test_52552_start INCREMENT BY 3 MINVALUE 1 MAXVALUE 100 START 50;
+
+statement ok
+ALTER SEQUENCE test_52552_start INCREMENT BY 8;
+
+query I
+SELECT nextval('test_52552_start');
+----
+50
+
+subtest end