Re: pgsql: Support reloptions of enum type

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Support reloptions of enum type
Date: 2019-09-25 23:41:52
Message-ID: 20190925234152.GA2115@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Hi Alvaro,

On Wed, Sep 25, 2019 at 06:59:24PM +0000, Alvaro Herrera wrote:
> Support reloptions of enum type
>
> All our current in core relation options of type string (not many,
> admittedly) behave in reality like enums. But after seeing an
> implementation for enum reloptions, it's clear that strings are messier,
> so introduce the new reloption type. Switch all string options to be
> enums instead.
>
> Fortunately we have a recently introduced test module for reloptions, so
> we don't lose coverage of string reloptions, which may still be used by
> third-party modules.
>
> Authors: Nikolay Shaplov, Álvaro Herrera
> Reviewed-by: Nikita Glukhov, Aleksandr Parfenov
> Discussion: https://postgr.es/m/43332102.S2V5pIjXRx@x200m

This part from commit 773df88 is incorrect:
@@ -661,6 +700,13 @@ allocate_reloption(bits32 kinds, int type, const
char *name, const char *desc,
newoption->type = type;
newoption->lockmode = lockmode;

+ /*
+ * Set the default lock mode for this option. There is no actual way
+ * for a module to enforce it when declaring a custom relation option,
+ * so just use the highest level, which is safe for all cases.
+ */
+ newoption->lockmode = AccessExclusiveLock;

Any caller of allocate_reloption() passes down its own lockmode
definition, hence you are removing the pluggability of the API. I
think that you just got trapped by an incorrect rebase.

Do you mind if I apply the attached to fix the issue? Or perhaps you
would prefer fixing the issue yourself? I noted some inconsistencies
with the rest while on it (please see attached).
--
Michael

Attachment Content-Type Size
reloptions-lockmode.patch text/x-diff 2.4 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2019-09-25 23:56:43 Re: pgsql: Implement jsonpath .datetime() method
Previous Message Alexander Korotkov 2019-09-25 23:25:03 Re: pgsql: Implement jsonpath .datetime() method