Re: CREATE SEQUENCE with RESTART option

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: Raw Message | Whole Thread | 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

In response to

Responses

Browse pgsql-hackers by date

  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