| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> | 
|---|---|
| To: | Cary Huang <cary(dot)huang(at)highgo(dot)ca> | 
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> | 
| Subject: | Re: CREATE SEQUENCE with RESTART option | 
| Date: | 2021-07-24 16:26:40 | 
| Message-ID: | CALj2ACUVGUz-azDLRFoi42vZoxvcoTh8UyR+CePzkpi8Tb8mWA@mail.gmail.com | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
On Sat, Jul 24, 2021 at 3:20 AM Cary Huang <cary(dot)huang(at)highgo(dot)ca> 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.
| Attachment | Content-Type | Size | 
|---|---|---|
| v2-0001-Disallow-RESTART-option-for-CREATE-SEQUENCE.patch | application/x-patch | 3.0 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Justin Pryzby | 2021-07-24 16:46:17 | Re: DROP INDEX CONCURRENTLY on partitioned index | 
| Previous Message | Tom Lane | 2021-07-24 16:20:23 | Re: Configure with thread sanitizer fails the thread test |