diff --git a/pkg/sql/alter_sequence.go b/pkg/sql/alter_sequence.go index 59037f32a157..3f30d2bc58f1 100644 --- a/pkg/sql/alter_sequence.go +++ b/pkg/sql/alter_sequence.go @@ -19,7 +19,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/log/eventpb" + "github.com/cockroachdb/errors" ) type alterSequenceNode struct { @@ -66,10 +68,28 @@ func (n *alterSequenceNode) startExec(params runParams) error { oldMinValue := desc.SequenceOpts.MinValue oldMaxValue := desc.SequenceOpts.MaxValue - err := assignSequenceOptions( - desc.SequenceOpts, n.n.Options, false /* setDefaults */, ¶ms, desc.GetID(), desc.ParentID, - ) - if err != nil { + existingType := types.Int + if desc.GetSequenceOpts().AsIntegerType != "" { + switch desc.GetSequenceOpts().AsIntegerType { + case types.Int2.SQLString(): + existingType = types.Int2 + case types.Int4.SQLString(): + existingType = types.Int4 + case types.Int.SQLString(): + // Already set. + default: + return errors.AssertionFailedf("sequence has unexpected type %s", desc.GetSequenceOpts().AsIntegerType) + } + } + if err := assignSequenceOptions( + desc.SequenceOpts, + n.n.Options, + false, /* setDefaults */ + ¶ms, + desc.GetID(), + desc.ParentID, + existingType, + ); err != nil { return err } opts := desc.SequenceOpts diff --git a/pkg/sql/create_sequence.go b/pkg/sql/create_sequence.go index 6f77398beea1..ed5ee3a106df 100644 --- a/pkg/sql/create_sequence.go +++ b/pkg/sql/create_sequence.go @@ -222,8 +222,15 @@ func NewSequenceTableDesc( opts := &descpb.TableDescriptor_SequenceOpts{ Increment: 1, } - err := assignSequenceOptions(opts, sequenceOptions, true /* setDefaults */, params, id, parentID) - if err != nil { + if err := assignSequenceOptions( + opts, + sequenceOptions, + true, /* setDefaults */ + params, + id, + parentID, + nil, /* existingType */ + ); err != nil { return nil, err } desc.SequenceOpts = opts diff --git a/pkg/sql/logictest/testdata/logic_test/alter_sequence b/pkg/sql/logictest/testdata/logic_test/alter_sequence index 237a2c6695af..e4ed371c0d62 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_sequence +++ b/pkg/sql/logictest/testdata/logic_test/alter_sequence @@ -60,3 +60,174 @@ query T SELECT create_statement FROM [SHOW CREATE SEQUENCE foo] ---- CREATE SEQUENCE public.foo MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 5 START 1 + +user root + +# Alter between sequences, check it is ok. +statement ok +CREATE SEQUENCE seq_as AS int2 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_as] +---- +CREATE SEQUENCE public.seq_as AS INT2 MINVALUE 1 MAXVALUE 32767 INCREMENT 1 START 1 + +statement ok +ALTER SEQUENCE seq_as AS int4 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_as] +---- +CREATE SEQUENCE public.seq_as AS INT4 MINVALUE 1 MAXVALUE 2147483647 INCREMENT 1 START 1 + +statement ok +ALTER SEQUENCE seq_as AS int8 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_as] +---- +CREATE SEQUENCE public.seq_as AS INT8 MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 + +statement ok +ALTER SEQUENCE seq_as AS int4 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_as] +---- +CREATE SEQUENCE public.seq_as AS INT4 MINVALUE 1 MAXVALUE 2147483647 INCREMENT 1 START 1 + +statement ok +ALTER SEQUENCE seq_as AS int2 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_as] +---- +CREATE SEQUENCE public.seq_as AS INT2 MINVALUE 1 MAXVALUE 32767 INCREMENT 1 START 1 + +# Test ALTER SEQUENCE AS when downgrading sizes behaves appropriately if MINVALUE/MAXVALUE/START is set. + +statement ok +CREATE SEQUENCE seq_int4_max_high AS int4 MAXVALUE 99999 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_max_high] +---- +CREATE SEQUENCE public.seq_int4_max_high AS INT4 MINVALUE 1 MAXVALUE 99999 INCREMENT 1 START 1 + +statement error MAXVALUE \(99999\) must be less than \(32767\) for type INT2 +ALTER SEQUENCE seq_int4_max_high AS int2 + +statement ok +ALTER SEQUENCE seq_int4_max_high AS int8 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_max_high] +---- +CREATE SEQUENCE public.seq_int4_max_high AS INT8 MINVALUE 1 MAXVALUE 99999 INCREMENT 1 START 1 + +statement ok +CREATE SEQUENCE seq_int4_min_high AS int4 MINVALUE 99999 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_min_high] +---- +CREATE SEQUENCE public.seq_int4_min_high AS INT4 MINVALUE 99999 MAXVALUE 2147483647 INCREMENT 1 START 99999 + +statement error MINVALUE \(99999\) must be less than \(32767\) for type INT2 +ALTER SEQUENCE seq_int4_min_high AS int2 + +statement ok +ALTER SEQUENCE seq_int4_min_high AS int8 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_min_high] +---- +CREATE SEQUENCE public.seq_int4_min_high AS INT8 MINVALUE 99999 MAXVALUE 9223372036854775807 INCREMENT 1 START 99999 + +statement ok +CREATE SEQUENCE seq_int4_start_high AS int4 START 99999 + +statement error START value \(99999\) cannot be greater than MAXVALUE \(32767\) +ALTER SEQUENCE seq_int4_start_high AS int2 + +statement ok +ALTER SEQUENCE seq_int4_start_high AS int8 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_start_high] +---- +CREATE SEQUENCE public.seq_int4_start_high AS INT8 MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 99999 + +statement ok +CREATE SEQUENCE seq_int4_min_low AS int4 MINVALUE -99999 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_min_low] +---- +CREATE SEQUENCE public.seq_int4_min_low AS INT4 MINVALUE -99999 MAXVALUE 2147483647 INCREMENT 1 START -99999 + +statement error MINVALUE \(-99999\) must be greater than \(-32768\) for type INT2 +ALTER SEQUENCE seq_int4_min_low AS int2 + +statement ok +ALTER SEQUENCE seq_int4_min_low AS int8 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_min_low] +---- +CREATE SEQUENCE public.seq_int4_min_low AS INT8 MINVALUE -99999 MAXVALUE 9223372036854775807 INCREMENT 1 START -99999 + +statement ok +CREATE SEQUENCE seq_int4_max_high_desc AS int4 MAXVALUE 99999 INCREMENT -1 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_max_high_desc] +---- +CREATE SEQUENCE public.seq_int4_max_high_desc AS INT4 MINVALUE -2147483648 MAXVALUE 99999 INCREMENT -1 START 99999 + +statement error MAXVALUE \(99999\) must be less than \(32767\) for type INT2 +ALTER SEQUENCE seq_int4_max_high_desc AS int2 + +statement ok +ALTER SEQUENCE seq_int4_max_high_desc AS int8 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_max_high_desc] +---- +CREATE SEQUENCE public.seq_int4_max_high_desc AS INT8 MINVALUE -9223372036854775808 MAXVALUE 99999 INCREMENT -1 START 99999 + +statement ok +CREATE SEQUENCE seq_int4_min_high_desc AS int4 MINVALUE -99999 INCREMENT -1 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_min_high_desc] +---- +CREATE SEQUENCE public.seq_int4_min_high_desc AS INT4 MINVALUE -99999 MAXVALUE -1 INCREMENT -1 START -1 + +statement error MINVALUE \(-99999\) must be greater than \(-32768\) for type INT2 +ALTER SEQUENCE seq_int4_min_high_desc AS int2 + +statement ok +ALTER SEQUENCE seq_int4_min_high_desc AS int8 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE seq_int4_min_high_desc] +---- +CREATE SEQUENCE public.seq_int4_min_high_desc AS INT8 MINVALUE -99999 MAXVALUE -1 INCREMENT -1 START -1 + +statement ok +CREATE SEQUENCE reverse_direction_seqas AS integer; +ALTER SEQUENCE reverse_direction_seqas AS smallint INCREMENT -1 + +query T +SELECT create_statement FROM [SHOW CREATE SEQUENCE reverse_direction_seqas] +---- +CREATE SEQUENCE public.reverse_direction_seqas AS INT2 MINVALUE -32768 MAXVALUE -1 INCREMENT -1 START -1 + +statement ok +ALTER SEQUENCE reverse_direction_seqas AS int INCREMENT 1 + +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 diff --git a/pkg/sql/sequence.go b/pkg/sql/sequence.go index 13aa750fdc4a..30770858a09c 100644 --- a/pkg/sql/sequence.go +++ b/pkg/sql/sequence.go @@ -363,52 +363,75 @@ func readOnlyError(s string) error { "cannot execute %s in a read-only transaction", s) } +func getSequenceIntegerBounds( + integerType *types.T, +) (lowerIntBound int64, upperIntBound int64, err error) { + switch integerType { + case types.Int2: + return math.MinInt16, math.MaxInt16, nil + case types.Int4: + return math.MinInt32, math.MaxInt32, nil + case types.Int: + return math.MinInt64, math.MaxInt64, nil + } + + return 0, 0, pgerror.Newf( + pgcode.InvalidParameterValue, + "CREATE SEQUENCE option AS received type %s, must be integer", + integerType, + ) +} + func setSequenceIntegerBounds( opts *descpb.TableDescriptor_SequenceOpts, integerType *types.T, isAscending bool, - lowerIntBound *int64, - upperIntBound *int64, + setMinValue bool, + setMaxValue bool, ) error { - opts.MinValue = math.MinInt64 - opts.MaxValue = math.MaxInt64 + var minValue int64 = math.MinInt64 + var maxValue int64 = math.MaxInt64 if isAscending { - opts.MinValue = 1 + minValue = 1 switch integerType { case types.Int2: - opts.MaxValue = math.MaxInt16 - *upperIntBound = math.MaxInt16 - *lowerIntBound = math.MinInt16 + maxValue = math.MaxInt16 case types.Int4: - opts.MaxValue = math.MaxInt32 - *upperIntBound = math.MaxInt32 - *lowerIntBound = math.MinInt32 + maxValue = math.MaxInt32 case types.Int: // Do nothing, it's the default. default: - return pgerror.Newf(pgcode.InvalidParameterValue, - "CREATE SEQUENCE option AS received type %s, must be integer", integerType) + return pgerror.Newf( + pgcode.InvalidParameterValue, + "CREATE SEQUENCE option AS received type %s, must be integer", + integerType, + ) } } else { - opts.MaxValue = -1 + maxValue = -1 switch integerType { case types.Int2: - opts.MinValue = math.MinInt16 - *upperIntBound = math.MaxInt16 - *lowerIntBound = math.MinInt16 + minValue = math.MinInt16 case types.Int4: - opts.MinValue = math.MinInt32 - *upperIntBound = math.MaxInt32 - *lowerIntBound = math.MinInt32 + minValue = math.MinInt32 case types.Int: // Do nothing, it's the default. default: - return pgerror.Newf(pgcode.InvalidParameterValue, - "CREATE SEQUENCE option AS received type %s, must be integer", integerType) + return pgerror.Newf( + pgcode.InvalidParameterValue, + "CREATE SEQUENCE option AS received type %s, must be integer", + integerType, + ) } } + if setMinValue { + opts.MinValue = minValue + } + if setMaxValue { + opts.MaxValue = maxValue + } return nil } @@ -421,13 +444,13 @@ func assignSequenceOptions( params *runParams, sequenceID descpb.ID, sequenceParentID descpb.ID, + existingType *types.T, ) error { + wasAscending := opts.Increment > 0 + // Set the default integer type of a sequence. var integerType = types.Int - var upperIntBound = int64(math.MaxInt64) - var lowerIntBound = int64(math.MinInt64) - // All other defaults are dependent on the value of increment // and the AS integerType. (i.e. whether the sequence is ascending // or descending, bigint vs. smallint) @@ -460,9 +483,36 @@ func assignSequenceOptions( opts.CacheSize = 1 } + lowerIntBound, upperIntBound, err := getSequenceIntegerBounds(integerType) + if err != nil { + return err + } + // Set default MINVALUE and MAXVALUE if AS option value for integer type is specified. if opts.AsIntegerType != "" { - if err := setSequenceIntegerBounds(opts, integerType, isAscending, &lowerIntBound, &upperIntBound); err != nil { + // We change MINVALUE and MAXVALUE if it is the originally set to the default during ALTER. + setMinValue := setDefaults + setMaxValue := setDefaults + if !setDefaults && existingType != nil { + existingLowerIntBound, existingUpperIntBound, err := getSequenceIntegerBounds(existingType) + if err != nil { + return err + } + if (wasAscending && opts.MinValue == 1) || (!wasAscending && opts.MinValue == existingLowerIntBound) { + setMinValue = true + } + if (wasAscending && opts.MaxValue == existingUpperIntBound) || (!wasAscending && opts.MaxValue == -1) { + setMaxValue = true + } + } + + if err := setSequenceIntegerBounds( + opts, + integerType, + isAscending, + setMinValue, + setMaxValue, + ); err != nil { return err } } @@ -496,21 +546,11 @@ func assignSequenceOptions( // A value of nil represents the user explicitly saying `NO MINVALUE`. if option.IntVal != nil { opts.MinValue = *option.IntVal - if *option.IntVal < lowerIntBound { - return pgerror.Newf(pgcode.InvalidParameterValue, - "MINVALUE (%d) must be greater than (%d) for type %s", - *option.IntVal, lowerIntBound, integerType.SQLString()) - } } case tree.SeqOptMaxValue: // A value of nil represents the user explicitly saying `NO MAXVALUE`. if option.IntVal != nil { opts.MaxValue = *option.IntVal - if *option.IntVal > upperIntBound { - return pgerror.Newf(pgcode.InvalidParameterValue, - "MAXVALUE (%d) must be less than (%d) for type %s", - *option.IntVal, upperIntBound, integerType.SQLString()) - } } case tree.SeqOptStart: opts.Start = *option.IntVal @@ -559,25 +599,71 @@ func assignSequenceOptions( } } - // If start option not specified, set it to MinValue (for ascending sequences) - // or MaxValue (for descending sequences). - if _, startSeen := optionsSeen[tree.SeqOptStart]; !startSeen { - if opts.Increment > 0 { - opts.Start = opts.MinValue - } else { - opts.Start = opts.MaxValue + if setDefaults || (wasAscending && opts.Start == 1) || (!wasAscending && opts.Start == -1) { + // If start option not specified, set it to MinValue (for ascending sequences) + // or MaxValue (for descending sequences). + // We only do this if we're setting it for the first time, or the sequence was + // ALTERed with the default original values. + if _, startSeen := optionsSeen[tree.SeqOptStart]; !startSeen { + if opts.Increment > 0 { + opts.Start = opts.MinValue + } else { + opts.Start = opts.MaxValue + } } } + if opts.MinValue < lowerIntBound { + return pgerror.Newf( + pgcode.InvalidParameterValue, + "MINVALUE (%d) must be greater than (%d) for type %s", + opts.MinValue, + lowerIntBound, + integerType.SQLString(), + ) + } + if opts.MaxValue < lowerIntBound { + return pgerror.Newf( + pgcode.InvalidParameterValue, + "MAXVALUE (%d) must be greater than (%d) for type %s", + opts.MaxValue, + lowerIntBound, + integerType.SQLString(), + ) + } + if opts.MinValue > upperIntBound { + return pgerror.Newf( + pgcode.InvalidParameterValue, + "MINVALUE (%d) must be less than (%d) for type %s", + opts.MinValue, + upperIntBound, + integerType.SQLString(), + ) + } + if opts.MaxValue > upperIntBound { + return pgerror.Newf( + pgcode.InvalidParameterValue, + "MAXVALUE (%d) must be less than (%d) for type %s", + opts.MaxValue, + upperIntBound, + integerType.SQLString(), + ) + } if opts.Start > opts.MaxValue { return pgerror.Newf( pgcode.InvalidParameterValue, - "START value (%d) cannot be greater than MAXVALUE (%d)", opts.Start, opts.MaxValue) + "START value (%d) cannot be greater than MAXVALUE (%d)", + opts.Start, + opts.MaxValue, + ) } if opts.Start < opts.MinValue { return pgerror.Newf( pgcode.InvalidParameterValue, - "START value (%d) cannot be less than MINVALUE (%d)", opts.Start, opts.MinValue) + "START value (%d) cannot be less than MINVALUE (%d)", + opts.Start, + opts.MinValue, + ) } return nil