Re: CREATE SEQUENCE with RESTART option
On Wed, Jul 28, 2021 at 01:16:19PM -0400, Tom Lane wrote: > I do not see any RESTART option in SQL:2021 11.72 definition>. Since we don't document it either, there's really no > expectation that anyone would use it. Okay, good point. I was not aware of that. > I don't particularly think that we should document it, so I'm with Michael > that we don't need to do anything. This is hardly the only undocumented > corner case in PG. More regression tests for CREATE SEQUENCE may be interesting, but that's not an issue either with the ones for ALTER SEQUENCE. Let's drop the patch and move on. -- Michael signature.asc Description: PGP signature
Re: CREATE SEQUENCE with RESTART option
Fujii Masao writes:
> On 2021/07/28 23:53, Bharath Rupireddy wrote:
>> -1. IMHO, this is something creating more confusion to the user. We
>> say that we allow both START and RESTART that RESTART is accepted as a
>> consequence of our internal option handling in gram.y. Instead, I
>> recommend throwing errorConflictingDefElem or errmsg("START and
>> RESTART are mutually exclusive options"). We do throw these errors in
>> a lot of other places for various options. Others may have better
>> thoughts though.
> Per docs, CREATE SEQUENCE conforms to the SQL standard, with some exceptions.
> So I'd agree with Michael if CREATE SEQUENCE with RESTART also conforms to
> the SQL standard, but I'd agree with Bharath otherwise.
I do not see any RESTART option in SQL:2021 11.72 . Since we don't document it either, there's really no
expectation that anyone would use it.
I don't particularly think that we should document it, so I'm with Michael
that we don't need to do anything. This is hardly the only undocumented
corner case in PG.
regards, tom lane
Re: CREATE SEQUENCE with RESTART option
On 2021/07/28 23:53, Bharath Rupireddy wrote:
On Wed, Jul 28, 2021 at 11:50 AM Michael Paquier wrote:
On Mon, Jul 26, 2021 at 04:57:53PM +0900, Michael Paquier wrote:
FWIW, like Ashutosh upthread, my vote would be to do nothing here in
terms of behavior changes as this is just breaking a behavior for the
sake of breaking it, so there are chances that this is going to piss
some users that relied accidentally on the existing behavior.
In short, I would be tempted with something like the attached, that
documents RESTART in CREATE SEQUENCE, while describing its behavior
according to START. In terms of regression tests, there is already a
lot in this area with ALTER SEQUENCE, but I think that having two
tests makes sense for CREATE SEQUENCE: one for RESTART without a
value and one with, where both explicitely set START.
Thoughts?
-1. IMHO, this is something creating more confusion to the user. We
say that we allow both START and RESTART that RESTART is accepted as a
consequence of our internal option handling in gram.y. Instead, I
recommend throwing errorConflictingDefElem or errmsg("START and
RESTART are mutually exclusive options"). We do throw these errors in
a lot of other places for various options. Others may have better
thoughts though.
Per docs, CREATE SEQUENCE conforms to the SQL standard, with some exceptions.
So I'd agree with Michael if CREATE SEQUENCE with RESTART also conforms to
the SQL standard, but I'd agree with Bharath otherwise.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Re: CREATE SEQUENCE with RESTART option
On Wed, Jul 28, 2021 at 11:50 AM Michael Paquier wrote:
>
> On Mon, Jul 26, 2021 at 04:57:53PM +0900, Michael Paquier wrote:
> > FWIW, like Ashutosh upthread, my vote would be to do nothing here in
> > terms of behavior changes as this is just breaking a behavior for the
> > sake of breaking it, so there are chances that this is going to piss
> > some users that relied accidentally on the existing behavior.
>
> In short, I would be tempted with something like the attached, that
> documents RESTART in CREATE SEQUENCE, while describing its behavior
> according to START. In terms of regression tests, there is already a
> lot in this area with ALTER SEQUENCE, but I think that having two
> tests makes sense for CREATE SEQUENCE: one for RESTART without a
> value and one with, where both explicitely set START.
>
> Thoughts?
-1. IMHO, this is something creating more confusion to the user. We
say that we allow both START and RESTART that RESTART is accepted as a
consequence of our internal option handling in gram.y. Instead, I
recommend throwing errorConflictingDefElem or errmsg("START and
RESTART are mutually exclusive options"). We do throw these errors in
a lot of other places for various options. Others may have better
thoughts though.
Regards,
Bharath Rupireddy.
Re: CREATE SEQUENCE with RESTART option
On Mon, Jul 26, 2021 at 04:57:53PM +0900, Michael Paquier wrote:
> FWIW, like Ashutosh upthread, my vote would be to do nothing here in
> terms of behavior changes as this is just breaking a behavior for the
> sake of breaking it, so there are chances that this is going to piss
> some users that relied accidentally on the existing behavior.
In short, I would be tempted with something like the attached, that
documents RESTART in CREATE SEQUENCE, while describing its behavior
according to START. In terms of regression tests, there is already a
lot in this area with ALTER SEQUENCE, but I think that having two
tests makes sense for CREATE SEQUENCE: one for RESTART without a
value and one with, where both explicitely set START.
Thoughts?
--
Michael
diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out
index 71c2b0f1df..7f5835d52f 100644
--- a/src/test/regress/expected/sequence.out
+++ b/src/test/regress/expected/sequence.out
@@ -308,6 +308,23 @@ DROP SEQUENCE t1_f1_seq;
ERROR: sequence "t1_f1_seq" does not exist
-- Now OK:
DROP SEQUENCE myseq2;
+-- Interactions between START and RESTART at creation
+CREATE SEQUENCE test_seq2 START 150 RESTART 200;
+SELECT nextval('test_seq2'); -- 200, per RESTART
+ nextval
+-
+ 200
+(1 row)
+
+DROP SEQUENCE test_seq2;
+CREATE SEQUENCE test_seq2 START 50 RESTART;
+SELECT nextval('test_seq2'); -- 50, per new START value
+ nextval
+-
+ 50
+(1 row)
+
+DROP SEQUENCE test_seq2;
--
-- Alter sequence
--
diff --git a/src/test/regress/sql/sequence.sql b/src/test/regress/sql/sequence.sql
index 7928ee23ee..9d379a9d63 100644
--- a/src/test/regress/sql/sequence.sql
+++ b/src/test/regress/sql/sequence.sql
@@ -167,6 +167,14 @@ DROP SEQUENCE t1_f1_seq;
-- Now OK:
DROP SEQUENCE myseq2;
+-- Interactions between START and RESTART at creation
+CREATE SEQUENCE test_seq2 START 150 RESTART 200;
+SELECT nextval('test_seq2'); -- 200, per RESTART
+DROP SEQUENCE test_seq2;
+CREATE SEQUENCE test_seq2 START 50 RESTART;
+SELECT nextval('test_seq2'); -- 50, per new START value
+DROP SEQUENCE test_seq2;
+
--
-- Alter sequence
--
diff --git a/doc/src/sgml/ref/create_sequence.sgml b/doc/src/sgml/ref/create_sequence.sgml
index e4085804a4..1683e11d4c 100644
--- a/doc/src/sgml/ref/create_sequence.sgml
+++ b/doc/src/sgml/ref/create_sequence.sgml
@@ -25,7 +25,9 @@ CREATE [ TEMPORARY | TEMP ] SEQUENCE [ IF NOT EXISTS ] data_type ]
[ INCREMENT [ BY ] increment ]
[ MINVALUE minvalue | NO MINVALUE ] [ MAXVALUE maxvalue | NO MAXVALUE ]
-[ START [ WITH ] start ] [ CACHE cache ] [ [ NO ] CYCLE ]
+[ START [ WITH ] start ]
+[ RESTART [ [ WITH ] restart ] ]
+[ CACHE cache ] [ [ NO ] CYCLE ]
[ OWNED BY { table_name.column_name | NONE } ]
@@ -185,6 +187,22 @@ SELECT * FROM name;
+
+restart
+
+
+ The optional clause RESTART [ WITH restart ] changes the
+ start value of the sequence. When specified alongside
+ START, the value specified with
+ RESTART WITH takes priority. If
+ RESTART is specified without a value, the
+ start value of the sequence is the one defined by
+ START.
+
+
+
+
cache
signature.asc
Description: PGP signature
Re: CREATE SEQUENCE with RESTART option
On Sat, Jul 24, 2021 at 09:56:40PM +0530, Bharath Rupireddy wrote: > LGTM. PSA v2 patch. FWIW, like Ashutosh upthread, my vote would be to do nothing here in terms of behavior changes as this is just breaking a behavior for the sake of breaking it, so there are chances that this is going to piss some users that relied accidentally on the existing behavior. I think that we should document that RESTART is accepted within the set of options as a consequence of the set of options supported by gram.y, though. -- Michael signature.asc Description: PGP signature
Re: CREATE SEQUENCE with RESTART option
On Sat, Jul 24, 2021 at 3:20 AM Cary Huang wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world: tested, passed
> Implements feature: tested, passed
> Spec compliant: tested, passed
> Documentation:tested, passed
>
> Hi
>
> I have applied and run your patch, which works fine in my environment.
> Regarding your comments in the patch:
Thanks for the review.
> /*
> * Restarting a sequence while defining it doesn't make any sense
> * and it may override the START value. Allowing both START and
> * RESTART option for CREATE SEQUENCE may cause confusion to user.
> * Hence, we throw error for CREATE SEQUENCE if RESTART option is
> * specified. However, it can be used with ALTER SEQUENCE.
> */
>
> I would remove the first sentence, because it seems like a personal opinion
> to me. I am sure someone, somewhere may think it makes total sense :).
Agree.
> I would rephrase like this:
>
> /*
> * Allowing both START and RESTART option for CREATE SEQUENCE
> * could override the START value and cause confusion to user. Hence,
> * we throw an error for CREATE SEQUENCE if RESTART option is
> * specified; it can only be used with ALTER SEQUENCE.
> */
>
> just a thought.
LGTM. PSA v2 patch.
Regards,
Bharath Rupireddy.
From e726e40d4969f56eb4246a7016e3e2780aa2fbf1 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy
Date: Sat, 24 Jul 2021 16:23:42 +
Subject: [PATCH v2] Disallow RESTART option for CREATE SEQUENCE
Allowing both START and RESTART option for CREATE SEQUENCE
could override the START value and cause confusion to user.
Hence, we throw an error for CREATE SEQUENCE if RESTART option
is specified; it can only be used with ALTER SEQUENCE.
If users want their sequences to be starting from a value different
than START value, then they can specify RESTART value with ALTER
SEQUENCE.
---
src/backend/commands/sequence.c| 12
src/test/regress/expected/sequence.out | 4
src/test/regress/sql/sequence.sql | 1 +
3 files changed, 17 insertions(+)
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 72bfdc07a4..8c50f396e6 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1281,6 +1281,18 @@ init_params(ParseState *pstate, List *options, bool for_identity,
}
else if (strcmp(defel->defname, "restart") == 0)
{
+ /*
+ * Allowing both START and RESTART option for CREATE SEQUENCE
+ * could override the START value and cause confusion to user.
+ * Hence, we throw an error for CREATE SEQUENCE if RESTART option
+ * is specified; it can only be used with ALTER SEQUENCE.
+ */
+ if (isInit)
+ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("RESTART option is not allowed while creating sequence"),
+ parser_errposition(pstate, defel->location)));
+
if (restart_value)
errorConflictingDefElem(defel, pstate);
restart_value = defel;
diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out
index 71c2b0f1df..f60725a9e5 100644
--- a/src/test/regress/expected/sequence.out
+++ b/src/test/regress/expected/sequence.out
@@ -16,6 +16,10 @@ CREATE SEQUENCE sequence_testx INCREMENT BY 1 START -10;
ERROR: START value (-10) cannot be less than MINVALUE (1)
CREATE SEQUENCE sequence_testx CACHE 0;
ERROR: CACHE (0) must be greater than zero
+CREATE SEQUENCE sequence_testx RESTART 5;
+ERROR: RESTART option is not allowed while creating sequence
+LINE 1: CREATE SEQUENCE sequence_testx RESTART 5;
+ ^
-- OWNED BY errors
CREATE SEQUENCE sequence_testx OWNED BY nobody; -- nonsense word
ERROR: invalid OWNED BY option
diff --git a/src/test/regress/sql/sequence.sql b/src/test/regress/sql/sequence.sql
index 7928ee23ee..367157fe2b 100644
--- a/src/test/regress/sql/sequence.sql
+++ b/src/test/regress/sql/sequence.sql
@@ -10,6 +10,7 @@ CREATE SEQUENCE sequence_testx INCREMENT BY 1 MAXVALUE -20;
CREATE SEQUENCE sequence_testx INCREMENT BY -1 START 10;
CREATE SEQUENCE sequence_testx INCREMENT BY 1 START -10;
CREATE SEQUENCE sequence_testx CACHE 0;
+CREATE SEQUENCE sequence_testx RESTART 5;
-- OWNED BY errors
CREATE SEQUENCE sequence_testx OWNED BY nobody; -- nonsense word
--
2.25.1
Re: CREATE SEQUENCE with RESTART option
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Hi I have applied and run your patch, which works fine in my environment. Regarding your comments in the patch: /* * Restarting a sequence while defining it doesn't make any sense * and it may override the START value. Allowing both START and * RESTART option for CREATE SEQUENCE may cause confusion to user. * Hence, we throw error for CREATE SEQUENCE if RESTART option is * specified. However, it can be used with ALTER SEQUENCE. */ I would remove the first sentence, because it seems like a personal opinion to me. I am sure someone, somewhere may think it makes total sense :). I would rephrase like this: /* * Allowing both START and RESTART option for CREATE SEQUENCE * could override the START value and cause confusion to user. Hence, * we throw an error for CREATE SEQUENCE if RESTART option is * specified; it can only be used with ALTER SEQUENCE. */ just a thought. thanks! - Cary Huang HighGo Software Canada www.highgo.ca
Re: CREATE SEQUENCE with RESTART option
On Thu, Apr 8, 2021 at 3:16 PM Suraj Kharage wrote: >> > The RESTART clause in the CREATE SEQUENCE doesn't make sense >> > to me, it should be restricted, IMO. > > +1 > >> >> Thanks! Attaching a patch that throws an error if the RESTART option >> is specified with CREATE SEQUENCE. Please have a look and let me know >> if the error message wording is fine or not. Is it better to include >> the reason as to why we disallow something like "Because it may >> override the START option." in err_detail along with the error >> message? > > > Patch looks good to me. Current error message looks ok to me. > Do we need to add double quotes for RESTART word in the error message since > it is an option? Thanks for taking a look at the patch. Looks like the other options are used in the error message without quotes, see "MINVALUE (%s) is out of range for sequence data type "START value (%s) cannot be less than MINVALUE "RESTART value (%s) cannot be less than MINVALUE "CACHE (%s) must be greater than zero With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
Re: CREATE SEQUENCE with RESTART option
On Thu, Apr 8, 2021 at 2:03 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > > > > > The RESTART clause in the CREATE SEQUENCE doesn't make sense > > to me, it should be restricted, IMO. > +1 > > Thanks! Attaching a patch that throws an error if the RESTART option > is specified with CREATE SEQUENCE. Please have a look and let me know > if the error message wording is fine or not. Is it better to include > the reason as to why we disallow something like "Because it may > override the START option." in err_detail along with the error > message? > Patch looks good to me. Current error message looks ok to me. Do we need to add double quotes for RESTART word in the error message since it is an option? -- -- Thanks & Regards, Suraj kharage, edbpostgres.com
Re: CREATE SEQUENCE with RESTART option
On Thu, Apr 8, 2021 at 10:09 AM Amul Sul wrote:
>
> On Wed, Apr 7, 2021 at 6:52 PM Bharath Rupireddy
> wrote:
> >
> > On Wed, Apr 7, 2021 at 6:04 PM Ashutosh Bapat
> > wrote:
> > > At best CREATE SEQUENCE START ... RESTART ... can be a shorthand
> > > for CREATE SEQUENCE ... START; ALTER SEQUENCE ... RESTART run back to
> > > back. So it looks useful but in rare cases.
> >
> > I personally feel that let's not mix up START and RESTART in CREATE
> > SEQUENCE. If required, users will run ALTER SEQUENCE RESTART
> > separately, that will be a clean way.
> >
> > > Said all that I agree that if we are supporting CREATE SEQUENCE ...
> > > RESTART then we should document it, correctly. If that's not the
> > > intention, we should disallow RESTART with CREATE SEQUENCE.
> >
> > As I mentioned upthread, it's better to disallow (throw error) if
> > RESTART is specified for CREATE SEQUENCE. Having said that, I would
> > like to hear from others.
> >
>
> FWIW, +1.
>
> The RESTART clause in the CREATE SEQUENCE doesn't make sense
> to me, it should be restricted, IMO.
Thanks! Attaching a patch that throws an error if the RESTART option
is specified with CREATE SEQUENCE. Please have a look and let me know
if the error message wording is fine or not. Is it better to include
the reason as to why we disallow something like "Because it may
override the START option." in err_detail along with the error
message?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From 819d653cc2be54e822826f08905f6ce8ec2f8418 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy
Date: Thu, 8 Apr 2021 13:35:38 +0530
Subject: [PATCH v1] Disallow RESTART option for CREATE SEQUENCE
Restarting a sequence while defining it doesn't make any sense
and it may override the START value specified. Hence, we throw
error for CREATE SEQUENCE if RESTART option is specified.
However, it can be used with ALTER SEQUENCE.
If users want their sequences to be starting from a value different
than START value, then they can specify RESTART value with ALTER
SEQUENCE.
---
src/backend/commands/sequence.c| 13 +
src/test/regress/expected/sequence.out | 4
src/test/regress/sql/sequence.sql | 1 +
3 files changed, 18 insertions(+)
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 0415df9ccb..5b75229360 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1290,6 +1290,19 @@ init_params(ParseState *pstate, List *options, bool for_identity,
}
else if (strcmp(defel->defname, "restart") == 0)
{
+ /*
+ * Restarting a sequence while defining it doesn't make any sense
+ * and it may override the START value. Allowing both START and
+ * RESTART option for CREATE SEQUENCE may cause confusion to user.
+ * Hence, we throw error for CREATE SEQUENCE if RESTART option is
+ * specified. However, it can be used with ALTER SEQUENCE.
+ */
+ if (isInit)
+ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("RESTART option is not allowed while creating sequence"),
+ parser_errposition(pstate, defel->location)));
+
if (restart_value)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out
index 8b928b2888..b9593bc8e4 100644
--- a/src/test/regress/expected/sequence.out
+++ b/src/test/regress/expected/sequence.out
@@ -16,6 +16,10 @@ CREATE SEQUENCE sequence_testx INCREMENT BY 1 START -10;
ERROR: START value (-10) cannot be less than MINVALUE (1)
CREATE SEQUENCE sequence_testx CACHE 0;
ERROR: CACHE (0) must be greater than zero
+CREATE SEQUENCE sequence_testx RESTART 5;
+ERROR: RESTART option is not allowed while creating sequence
+LINE 1: CREATE SEQUENCE sequence_testx RESTART 5;
+ ^
-- OWNED BY errors
CREATE SEQUENCE sequence_testx OWNED BY nobody; -- nonsense word
ERROR: invalid OWNED BY option
diff --git a/src/test/regress/sql/sequence.sql b/src/test/regress/sql/sequence.sql
index 7928ee23ee..367157fe2b 100644
--- a/src/test/regress/sql/sequence.sql
+++ b/src/test/regress/sql/sequence.sql
@@ -10,6 +10,7 @@ CREATE SEQUENCE sequence_testx INCREMENT BY 1 MAXVALUE -20;
CREATE SEQUENCE sequence_testx INCREMENT BY -1 START 10;
CREATE SEQUENCE sequence_testx INCREMENT BY 1 START -10;
CREATE SEQUENCE sequence_testx CACHE 0;
+CREATE SEQUENCE sequence_testx RESTART 5;
-- OWNED BY errors
CREATE SEQUENCE sequence_testx OWNED BY nobody; -- nonsense word
--
2.25.1
Re: CREATE SEQUENCE with RESTART option
On Wed, Apr 7, 2021 at 6:52 PM Bharath Rupireddy wrote: > > On Wed, Apr 7, 2021 at 6:04 PM Ashutosh Bapat > wrote: > > At best CREATE SEQUENCE START ... RESTART ... can be a shorthand > > for CREATE SEQUENCE ... START; ALTER SEQUENCE ... RESTART run back to > > back. So it looks useful but in rare cases. > > I personally feel that let's not mix up START and RESTART in CREATE > SEQUENCE. If required, users will run ALTER SEQUENCE RESTART > separately, that will be a clean way. > > > Said all that I agree that if we are supporting CREATE SEQUENCE ... > > RESTART then we should document it, correctly. If that's not the > > intention, we should disallow RESTART with CREATE SEQUENCE. > > As I mentioned upthread, it's better to disallow (throw error) if > RESTART is specified for CREATE SEQUENCE. Having said that, I would > like to hear from others. > FWIW, +1. The RESTART clause in the CREATE SEQUENCE doesn't make sense to me, it should be restricted, IMO. Regards, Amul
Re: CREATE SEQUENCE with RESTART option
On Wed, Apr 7, 2021 at 6:04 PM Ashutosh Bapat wrote: > At best CREATE SEQUENCE START ... RESTART ... can be a shorthand > for CREATE SEQUENCE ... START; ALTER SEQUENCE ... RESTART run back to > back. So it looks useful but in rare cases. I personally feel that let's not mix up START and RESTART in CREATE SEQUENCE. If required, users will run ALTER SEQUENCE RESTART separately, that will be a clean way. > Said all that I agree that if we are supporting CREATE SEQUENCE ... > RESTART then we should document it, correctly. If that's not the > intention, we should disallow RESTART with CREATE SEQUENCE. As I mentioned upthread, it's better to disallow (throw error) if RESTART is specified for CREATE SEQUENCE. Having said that, I would like to hear from others. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
Re: CREATE SEQUENCE with RESTART option
On Wed, Apr 7, 2021 at 3:56 PM Bharath Rupireddy
wrote:
>
> Hi,
>
> It looks like we do allow $subject which has following behaviour:
> create sequence myseq restart 200;--> sequence is starting from
> restart value overriding start value
> create sequence myseq start 100 restart 200; --> sequence is starting
> from restart value overriding start value
> create sequence myseq start 100 restart; --> sequence is starting from
> start value no overriding of start value occurs
> create sequence myseq restart; --> sequence is starting from default
> start value no overriding of start value occurs
>
> While we have documented the "restart" option behaviour for ALTER
> SEQUENCE, we have no mention of it in the CREATE SEQUENCE docs page.
> Do we need to document the above behaviour for CREATE SEQUENCE?
> Alternatively, do we need to throw an error if the user is not
> supposed to use the "restart" option with CREATE SEQUENCE?
>
> IMO, allowing the "restart" option for CREATE SEQUENCE doesn't make
> sense when we have the "start" option, so it's better to throw an
> error.
Using restart in CREATE SEQUENCE command looks, umm, funny. But
looking at the code it makes me wonder whether it's intentional.
1567 /* RESTART [WITH] */
1568 if (restart_value != NULL)
1569 {
1570 if (restart_value->arg != NULL)
1571 seqdataform->last_value = defGetInt64(restart_value);
1572 else
1573 seqdataform->last_value = seqform->seqstart;
1574 seqdataform->is_called = false;
1575 seqdataform->log_cnt = 0;
1576 }
1577 else if (isInit)
1578 {
1579 seqdataform->last_value = seqform->seqstart;
1580 seqdataform->is_called = false;
1581 }
"restart" as the name suggests "restarts" a sequence from a given
value or start of sequence. "start" on the other hand specifies the
"start" value of sequence and is also the value used to "restart" by
default from.
So here's what will happen in each of the cases you mentioned
> create sequence myseq restart 200;--> sequence is starting from
> restart value overriding start value
the first time sequence will be used it will use value 200, but if
someone does a "restart" it will start from default start of that
sequence.
> create sequence myseq start 100 restart 200; --> sequence is starting
> from restart value overriding start value
the first time sequence will be used it will use value 200, but if
someone does a "restart" it will start from 100
> create sequence myseq start 100 restart; --> sequence is starting from
> start value no overriding of start value occurs
the first time sequence will be used it will use value 100, and if
someone does a "restart" it will start from 100
> create sequence myseq restart; --> sequence is starting from default
> start value no overriding of start value occurs
this is equivalent to "create sequence myseq"
This is the behaviour implied when we read
https://www.postgresql.org/docs/13/sql-createsequence.html and
https://www.postgresql.org/docs/13/sql-altersequence.html together.
At best CREATE SEQUENCE START ... RESTART ... can be a shorthand
for CREATE SEQUENCE ... START; ALTER SEQUENCE ... RESTART run back to
back. So it looks useful but in rare cases.
Said all that I agree that if we are supporting CREATE SEQUENCE ...
RESTART then we should document it, correctly. If that's not the
intention, we should disallow RESTART with CREATE SEQUENCE.
--
Best Wishes,
Ashutosh Bapat
CREATE SEQUENCE with RESTART option
Hi, It looks like we do allow $subject which has following behaviour: create sequence myseq restart 200;--> sequence is starting from restart value overriding start value create sequence myseq start 100 restart 200; --> sequence is starting from restart value overriding start value create sequence myseq start 100 restart; --> sequence is starting from start value no overriding of start value occurs create sequence myseq restart; --> sequence is starting from default start value no overriding of start value occurs While we have documented the "restart" option behaviour for ALTER SEQUENCE, we have no mention of it in the CREATE SEQUENCE docs page. Do we need to document the above behaviour for CREATE SEQUENCE? Alternatively, do we need to throw an error if the user is not supposed to use the "restart" option with CREATE SEQUENCE? IMO, allowing the "restart" option for CREATE SEQUENCE doesn't make sense when we have the "start" option, so it's better to throw an error. Thoughts? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
