Skip to content

Commit

Permalink
sql : alter sequence's increment and the results of 'select nextval()…
Browse files Browse the repository at this point in the history
…' were not as expected

If the sequenceVal exceeded the old MinValue or MaxValue, we must set sequenceVal to the last valid one.

   1. Values is just init, when the sequence is altered before it has ever been used.
   In this case, last valid sequenceVal is oldStart.
   Set the initial value to the oldStartValue(just like pg), when last valid sequenceVal in [newMinValue, newMaxValue].

   2. When the sequenceVal out of range result from increment, report an error.

Fixed cockroachdb#52552 cockroachdb#74127

Release note (bug fix): A bug with ALTER SEQUENCE that allowed a sequence to return values that are out-of-range has been fixed.
  • Loading branch information
mosquito2333 committed May 5, 2022
1 parent a4d141a commit a356116
Show file tree
Hide file tree
Showing 4 changed files with 323 additions and 28 deletions.
95 changes: 75 additions & 20 deletions pkg/sql/alter_sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
Expand Down Expand Up @@ -61,12 +63,34 @@ func (p *planner) AlterSequence(ctx context.Context, n *tree.AlterSequence) (pla
// and expects to see its own writes.
func (n *alterSequenceNode) ReadingOwnWrites() {}

// GetLastSequenceValue get the last valid sequence value when unavailable values are caused by increment
func GetLastSequenceValue(lastVal int64, oldIncrement int64, oldBound int64) (int64, error) {
if (lastVal > oldBound && oldIncrement > 0) || (lastVal < oldBound && oldIncrement < 0) {
lastValidVal := lastVal
figureBeyond := (lastVal - oldBound) % oldIncrement
// oldBound is the last valid sequence value
if figureBeyond == 0 {
lastValidVal = oldBound
} else {
lastValidVal = oldBound - (oldIncrement - figureBeyond)
}
return lastValidVal, nil
}
return 0, pgerror.Newf(
pgcode.SequenceGeneratorLimitExceeded,
"Select currval please, it's abnormal")
}

func (n *alterSequenceNode) startExec(params runParams) error {
telemetry.Inc(sqltelemetry.SchemaChangeAlterCounter("sequence"))
desc := n.seqDesc

oldMinValue := desc.SequenceOpts.MinValue
oldMaxValue := desc.SequenceOpts.MaxValue
oldIncrement := desc.SequenceOpts.Increment
oldStart := desc.SequenceOpts.Start
// isAlterAfterInit : true incidate alter_sequence_stmt after create_sequence_stmt without use
isAlterWithOutUse := false

existingType := types.Int
if desc.GetSequenceOpts().AsIntegerType != "" {
Expand Down Expand Up @@ -113,33 +137,64 @@ func (n *alterSequenceNode) startExec(params runParams) error {
// 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
}
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
}
// If the sequenceVal exceeded the old MinValue or MaxValue, we must set sequenceVal to the last valid one.
// 1. Values is just init, when the sequence is altered before it has ever been used
// In this case, last valid sequenceVal is oldStart.
// Set the initial value to the oldStartValue(just like pg), when last valid sequenceVal in [newMinValue, newMaxValue]
if sequenceVal == oldStart-oldIncrement {
isAlterWithOutUse = true
if oldStart > opts.MaxValue {
return pgerror.Newf(
pgcode.InvalidParameterValue,
"START value (%d) cannot be greater than MAXVALUE (%d)", oldStart, opts.MaxValue)
}
if oldStart < opts.MinValue {
return pgerror.Newf(
pgcode.InvalidParameterValue,
"START value (%d) cannot be less than MINVALUE (%d)", oldStart, opts.MinValue)
}
sequenceVal = oldStart - opts.Increment
} else if (sequenceVal < oldMinValue && oldIncrement < 0) || (sequenceVal > oldMaxValue && oldIncrement > 0) {
// 2. The sequenceVal out of range result from increment
var oldBound int64
if sequenceVal < oldMinValue && oldIncrement < 0 {
// out of oldMinValue when decrease
oldBound = oldMinValue
} else if sequenceVal > oldMaxValue && oldIncrement > 0 {
// out of oldMaxValue when increase
oldBound = oldMaxValue
}
} else if opts.Increment > 0 && desc.SequenceOpts.MaxValue > oldMaxValue {
sequenceVal, err := getSequenceValue()
sequenceVal, err = GetLastSequenceValue(sequenceVal, oldIncrement, oldBound)
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.txn.Put(params.ctx, seqValueKey, sequenceVal); err != nil {
return err
}
// When isAlterWithOutUse is true, allow (opts.Start - opts.Increment) exceed the bounds.
// The opts.Start must be in [newMinValue, newMaxValue]
if isAlterWithOutUse == false {
if sequenceVal > opts.MaxValue {
return pgerror.Newf(
pgcode.SequenceGeneratorLimitExceeded,
"MAXVALUE cannot be made to be less than the current value")
} else if sequenceVal < opts.MinValue {
return pgerror.Newf(
pgcode.SequenceGeneratorLimitExceeded,
"MINVALUE cannot be made to be exceed than the current value")
}
}
// Clear out the cache and update the last value.
params.p.sessionDataMutatorIterator.applyOnEachMutator(func(m sessionDataMutator) {
m.initSequenceCache()
m.RecordLatestSequenceVal(uint32(n.seqDesc.GetID()), sequenceVal)
})

if err := params.p.writeSchemaChange(
params.ctx, n.seqDesc, descpb.InvalidMutationID, tree.AsStringWithFQNames(n.n, params.Ann()),
Expand Down
204 changes: 204 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_sequence
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,207 @@ query T
SELECT create_statement FROM [SHOW CREATE SEQUENCE reverse_direction_seqas]
----
CREATE SEQUENCE public.reverse_direction_seqas AS INT8 MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1


################################## the case where the sequence is altered before it has ever been used
#### case 1 : Modify successfully, without START
statement ok
CREATE SEQUENCE seq_testx_1 INCREMENT BY 3 MINVALUE 1 MAXVALUE 12

query I
select last_value from seq_testx_1
----
-2

statement error pq: START option unsupported in ALTER SEQUENCE
ALTER SEQUENCE seq_testx_4 INCREMENT BY 8 MINVALUE 1 MAXVALUE 30 start 10

statement error pq: START option unsupported in ALTER SEQUENCE
ALTER SEQUENCE seq_testx_4 INCREMENT BY 8 MINVALUE 1 MAXVALUE 30 start with 10

statement ok
ALTER SEQUENCE seq_testx_1 INCREMENT BY 8 MINVALUE 1 MAXVALUE 12

query I
select last_value from seq_testx_1
----
-7

query I
SELECT nextval('seq_testx_1')
----
1

query I
select last_value from seq_testx_1
----
1

query I
SELECT nextval('seq_testx_1')
----
9

query error pq: nextval\(\): reached maximum value of sequence "seq_testx_1" \(12\)
SELECT nextval('seq_testx_1')

#### case 2 : Fail to modify, and set START when create sequence
statement ok
CREATE SEQUENCE seq_testx_2 INCREMENT BY 3 MINVALUE 1 MAXVALUE 12 START 2

query I
select last_value from seq_testx_2
----
-1

statement error pq: MINVALUE cannot be made to be exceed than the current value
ALTER SEQUENCE seq_testx_2 INCREMENT BY 1 MINVALUE 4 MAXVALUE 12

query I
select last_value from seq_testx_2
----
-1

query I
SELECT nextval('seq_testx_2')
----
2

#### case 3 : Modify successfully, and set START when create sequence
statement ok
CREATE SEQUENCE seq_testx_3 INCREMENT BY 3 MINVALUE 1 MAXVALUE 12 start 5

query I
select last_value from seq_testx_3
----
2

statement ok
ALTER SEQUENCE seq_testx_3 INCREMENT BY 8 MINVALUE 1 MAXVALUE 12

query I
select last_value from seq_testx_3
----
-3

query I
select nextval('seq_testx_3')
----
5

query error pq: nextval\(\): reached maximum value of sequence "seq_testx_3" \(12\)
select nextval('seq_testx_3')

#### case 4 : Modify successfully, and the START set when create sequence not in [newMinValue, newMaxValue]
statement ok
CREATE SEQUENCE seq_testx_4 INCREMENT BY 3 MINVALUE 1 MAXVALUE 12 start 5

statement error pq: START value \(5\) cannot be greater than MAXVALUE \(4\)
ALTER SEQUENCE seq_testx_4 INCREMENT BY 1 MINVALUE 1 MAXVALUE 4

################################## the case where the sequence is altered after it has ever been used
statement ok
CREATE SEQUENCE customer_seq_alter_1 MINVALUE -2 MAXVALUE 2 START WITH 1 CACHE 5 INCREMENT BY -2

statement error pq: MINVALUE cannot be made to be exceed than the current value
ALTER SEQUENCE customer_seq INCREMENT BY -1 MINVALUE 6 MAXVALUE 10

query error pq: nextval\(\): reached maximum value of sequence "customer_seq" \(5\)
SELECT customer_seq.nextval from dual

query I
select test.public.customer_seq.currval
----
5

statement ok
ALTER SEQUENCE customer_seq INCREMENT BY 1 MINVALUE 5 MAXVALUE 10

query I
select test.public.customer_seq.currval
----
5

query I
select test.public.customer_seq.nextval
----
6

## sequenceVal in [Minimum, Maximum], increment = -1, Minimum < oldMinimum, Maximum > oldMaximum
statement ok
ALTER SEQUENCE customer_seq INCREMENT BY -1 MINVALUE -5 MAXVALUE 15

query I
select test.public.customer_seq.currval
----
6

query I
select test.public.customer_seq.nextval
----
5

## sequenceVal not in [Minimum, Maximum], increment > 0, minimum = oldMinimum, maximum < oldMaximum
statement error pq: MAXVALUE cannot be made to be less than the current value
ALTER SEQUENCE customer_seq INCREMENT BY 1 MINVALUE -5 MAXVALUE 0

query I
select test.public.customer_seq.currval
----
5

query I
select test.public.customer_seq.nextval
----
4

## check if the latest sequence is from cache
query I
select setval('customer_seq', 10)
----
10

## sequenceVal not in [Minimum, Maximum], increment < 0 and minimum > oldMinimum
statement error pq: MAXVALUE cannot be made to be less than the current value
ALTER SEQUENCE customer_seq INCREMENT BY 1 MINVALUE 0 MAXVALUE 5

query I
select test.public.customer_seq.nextval
----
9

query I
select test.public.customer_seq.nextval
----
8

## sequenceVal in [Minimum, Maximum], increment > 0 and maximum > oldMaximum
statement ok
ALTER SEQUENCE customer_seq INCREMENT BY 3 MINVALUE 0 MAXVALUE 10

query I
select test.public.customer_seq.currval
----
8

query error pq: nextval\(\): reached maximum value of sequence "customer_seq" \(10\)
select test.public.customer_seq.nextval

query I
select test.public.customer_seq.currval
----
8

statement error pq: MINVALUE cannot be made to be exceed than the current value
ALTER SEQUENCE customer_seq INCREMENT BY 1 MINVALUE 10 MAXVALUE 15

statement ok
ALTER SEQUENCE customer_seq INCREMENT BY 1 MINVALUE 0 MAXVALUE 9

query I
select test.public.customer_seq.nextval
----
9

query error pq: nextval\(\): reached maximum value of sequence "customer_seq" \(9\)
select test.public.customer_seq.nextval
34 changes: 34 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/sequences
Original file line number Diff line number Diff line change
Expand Up @@ -2214,3 +2214,37 @@ SELECT lastval()

statement ok
END

# The value value(12) is out-of-bounds
statement ok
CREATE SEQUENCE customer_seq_check_cache_and_bounds_1 INCREMENT BY 3 MINVALUE 6 MAXVALUE 10

query I
SELECT nextval('customer_seq_check_cache_and_bounds_1')
----
6

query I
SELECT nextval('customer_seq_check_cache_and_bounds_1')
----
9

statement error pgcode 2200H pq: nextval\(\): reached maximum value of sequence "customer_seq_check_cache_and_bounds_1" \(10\)
SELECT nextval('customer_seq_check_cache_and_bounds_1')

# Set the cache to 5
statement ok
CREATE SEQUENCE customer_seq_check_cache_and_bounds_2 MINVALUE -2 MAXVALUE 2 START WITH 1 CACHE 5 INCREMENT BY -2

query I
SELECT nextval('customer_seq_check_cache_and_bounds_2')
----
1

query I
SELECT nextval('customer_seq_check_cache_and_bounds_2')
----
-1

statement error pgcode 2200H pq: nextval\(\): reached minimum value of sequence "customer_seq_check_cache_and_bounds_2" \(-2\)
SELECT nextval('customer_seq_check_cache_and_bounds_2')
Loading

0 comments on commit a356116

Please sign in to comment.