sql: make cached sequences bounds-related semantics match pg semantics

Previously, cached sequences did not obey pg semantics
with respect to exceeding bounds. For example, if there
were a sequence with a cache size of 5 and max value of 2,
calling nextval(...) would immediately cause an error due
to exceeded bounds. According to postgres, nextval(...)
should return 1, then 2, then finally return an error due
to the max value of 2 being reached. This change alters
sequence caching behavior to match the above semantics.

Additionally, this change now makes it possible for sequences
to exceed the bounds set by MAXVALUE and MINVALUE. This is
because calling nextval(...) should be as fast as possible,
and the fastest way to do this is to let nextval(...) always
succeed on incrementing a sequence. Despite this, a user
will never see any values that exceed a sequence's bounds.
To make a sequence incrementable 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. If the value of a sequence exceeds its bounds,
   it must be restored to the original MinValue or MaxValue in
   the same transaction as the schema change.
This change adds code to handle case 2 above.

Release note: None

pkg/sql/alter_sequence.go

@@ -63,12 +63,65 @@ func (n *alterSequenceNode) startExec(params runParams) error {
 	telemetry.Inc(sqltelemetry.SchemaChangeAlterCounter("sequence"))
 	desc := n.seqDesc
 
+	oldMinValue := desc.SequenceOpts.MinValue
+	oldMaxValue := desc.SequenceOpts.MaxValue
+
 	err := assignSequenceOptions(
 		desc.SequenceOpts, n.n.Options, false /* setDefaults */, &params, desc.GetID(), desc.ParentID,
 	)
 	if err != nil {
 		return err
 	}
+	opts := desc.SequenceOpts
+	seqValueKey := params.p.ExecCfg().Codec.SequenceKey(uint32(desc.ID))
+	if err != nil {
+		return err
+	}
+
+	getSequenceValue := func() (int64, error) {
+		kv, err := params.p.txn.Get(params.ctx, seqValueKey)
+		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:
+	// 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.
+	// The code below handles the second case.
+
+	// The sequence is decreasing and the minvalue is being decreased.
+	if opts.Increment < 0 && desc.SequenceOpts.MinValue < oldMinValue {
+		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 sequenceVal < oldMinValue {
+			err := params.p.txn.Put(params.ctx, seqValueKey, oldMinValue)
+			if err != nil {
+				return err
+			}
+		}
+	} else if opts.Increment > 0 && desc.SequenceOpts.MaxValue > oldMaxValue {
+		sequenceVal, err := getSequenceValue()
+		if err != nil {
+			return err
+		}
+
+		if sequenceVal > oldMaxValue {
+			err := params.p.txn.Put(params.ctx, seqValueKey, oldMaxValue)
+			if err != nil {
+				return err
+			}
+		}
+	}
 
 	if err := params.p.writeSchemaChange(
 		params.ctx, n.seqDesc, descpb.InvalidMutationID, tree.AsStringWithFQNames(n.n, params.Ann()),

pkg/sql/logictest/testdata/logic_test/sequences

@@ -1544,3 +1544,202 @@ SELECT last_value FROM cache_test
 
 statement ok
 DROP SEQUENCE cache_test
+
+subtest cached_sequences_with_bounds_increasing
+
+statement ok
+CREATE SEQUENCE cached_upper_bound_test MAXVALUE 4 START WITH 2 CACHE 5 INCREMENT BY 2;
+
+# Without this MAXVALUE CONSTRAINT, the values 2,4,6,8,10 would be cached
+# and the underlying sequence would be set to 10. Given the constraint, only
+# 2 and 4 are cached, but the underlying sequence is still set to 10.
+
+query I
+SELECT nextval('cached_upper_bound_test');
+----
+2
+
+query I
+SELECT nextval('cached_upper_bound_test');
+----
+4
+
+query I
+SELECT last_value FROM cached_upper_bound_test;
+----
+10
+
+# nextval() should return errors while still incrementing the underlying sequence
+# by 10 each time (cache size of 5 * increment of 2). Despite the underlying sequence changing,
+# currval() and lastval() should not change.
+
+statement error pgcode 2200H pq: nextval\(\): reached maximum value of sequence "cached_upper_bound_test" \(4\)
+SELECT nextval('cached_upper_bound_test');
+
+statement error pgcode 2200H pq: nextval\(\): reached maximum value of sequence "cached_upper_bound_test" \(4\)
+SELECT nextval('cached_upper_bound_test');
+
+query I
+SELECT lastval();
+----
+4
+
+query I
+SELECT currval('cached_upper_bound_test');
+----
+4
+
+query I
+SELECT last_value FROM cached_upper_bound_test;
+----
+30
+
+# Performing a schema change on the sequence to increase the bounds should reset the underlying
+# sequence to its original MAXVALUE.
+
+statement ok
+ALTER SEQUENCE cached_upper_bound_test MAXVALUE 100;
+
+query I
+SELECT last_value FROM cached_upper_bound_test;
+----
+4
+
+# Calling nextval() should cache the values 6,8,10,12,14 while incrementing
+# the underlying sequence to 14.
+
+query I
+SELECT nextval('cached_upper_bound_test');
+----
+6
+
+query I
+SELECT last_value FROM cached_upper_bound_test;
+----
+14
+
+query I
+SELECT lastval();
+----
+6
+
+query I
+SELECT currval('cached_upper_bound_test');
+----
+6
+
+statement ok
+drop sequence cached_upper_bound_test;
+
+# This test is the same as cached_sequences_with_bounds_increasing, except it uses negative values.
+subtest cached_sequences_with_bounds_decreasing
+
+statement ok
+CREATE SEQUENCE cached_lower_bound_test MINVALUE -4 START WITH -2 CACHE 5 INCREMENT BY -2;
+
+query I
+SELECT nextval('cached_lower_bound_test');
+----
+-2
+
+
+query I
+SELECT nextval('cached_lower_bound_test');
+----
+-4
+
+query I
+SELECT last_value FROM cached_lower_bound_test;
+----
+-10
+
+statement error pgcode 2200H pq: nextval\(\): reached minimum value of sequence "cached_lower_bound_test" \(-4\)
+SELECT nextval('cached_lower_bound_test');
+
+statement error pgcode 2200H pq: nextval\(\): reached minimum value of sequence "cached_lower_bound_test" \(-4\)
+SELECT nextval('cached_lower_bound_test');
+
+query I
+SELECT last_value FROM cached_lower_bound_test;
+----
+-30
+
+statement ok
+ALTER SEQUENCE cached_lower_bound_test MINVALUE -100;
+
+query I
+SELECT last_value FROM cached_lower_bound_test;
+----
+-4
+
+query I
+SELECT nextval('cached_lower_bound_test');
+----
+-6
+
+query I
+SELECT last_value FROM cached_lower_bound_test;
+----
+-14
+
+statement ok
+DROP SEQUENCE cached_lower_bound_test;
+
+# This test is the same as cached_sequences_with_bounds_decreasing, except it uses both positive and negative values.
+subtest cached_sequences_with_bounds_middle
+
+statement ok
+CREATE SEQUENCE cached_lower_bound_test_2 MINVALUE -2 MAXVALUE 2 START WITH 2 CACHE 5 INCREMENT BY -2;
+
+query I
+SELECT nextval('cached_lower_bound_test_2');
+----
+2
+
+
+query I
+SELECT nextval('cached_lower_bound_test_2');
+----
+0
+
+query I
+SELECT nextval('cached_lower_bound_test_2');
+----
+-2
+
+query I
+SELECT last_value FROM cached_lower_bound_test_2;
+----
+-6
+
+statement error pgcode 2200H pq: nextval\(\): reached minimum value of sequence "cached_lower_bound_test_2" \(-2\)
+SELECT nextval('cached_lower_bound_test_2');
+
+statement error pgcode 2200H pq: nextval\(\): reached minimum value of sequence "cached_lower_bound_test_2" \(-2\)
+SELECT nextval('cached_lower_bound_test_2');
+
+query I
+SELECT last_value FROM cached_lower_bound_test_2;
+----
+-26
+
+statement ok
+ALTER SEQUENCE cached_lower_bound_test_2 MINVALUE -100;
+
+query I
+SELECT last_value FROM cached_lower_bound_test_2;
+----
+-2
+
+query I
+SELECT nextval('cached_lower_bound_test_2');
+----
+-4
+
+query I
+SELECT last_value FROM cached_lower_bound_test_2;
+----
+-12
+
+statement ok
+DROP SEQUENCE cached_lower_bound_test_2;

pkg/sql/sequence.go

@@ -145,7 +145,7 @@ func (p *planner) incrementSequenceUsingCache(
 
 	cacheSize := seqOpts.EffectiveCacheSize()
 
-	fetchNextValues := func() (currentValue int64, incrementAmount int64, sizeOfCache int64, err error) {
+	fetchNextValues := func() (currentValue, incrementAmount, sizeOfCache int64, err error) {
 		seqValueKey := p.ExecCfg().Codec.SequenceKey(uint32(descriptor.GetID()))
 
 		endValue, err := kv.IncrementValRetryable(
@@ -157,8 +157,29 @@ func (p *planner) incrementSequenceUsingCache(
 			}
 			return 0, 0, 0, err
 		}
+
+		// This sequence has exceeded its bounds after performing this increment.
 		if endValue > seqOpts.MaxValue || endValue < seqOpts.MinValue {
-			return 0, 0, 0, boundsExceededError(descriptor)
+			// If the sequence exceeded its bounds prior to the increment, then return an error.
+			if (seqOpts.Increment > 0 && endValue-seqOpts.Increment*cacheSize >= seqOpts.MaxValue) ||
+				(seqOpts.Increment < 0 && endValue-seqOpts.Increment*cacheSize <= seqOpts.MinValue) {
+				return 0, 0, 0, boundsExceededError(descriptor)
+			}
+			// Otherwise, values between the limit and the value prior to incrementing can be cached.
+			limit := seqOpts.MaxValue
+			if seqOpts.Increment < 0 {
+				limit = seqOpts.MinValue
+			}
+			abs := func(i int64) int64 {
+				if i < 0 {
+					return -i
+				}
+				return i
+			}
+			currentValue = endValue - seqOpts.Increment*(cacheSize-1)
+			incrementAmount = seqOpts.Increment
+			sizeOfCache = abs(limit-(endValue-seqOpts.Increment*cacheSize)) / abs(seqOpts.Increment)
+			return currentValue, incrementAmount, sizeOfCache, nil
 		}
 
 		return endValue - seqOpts.Increment*(cacheSize-1), seqOpts.Increment, cacheSize, nil