pgsql: Support reloptions of enum type
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: /messages/by-id/43332102.S2V5pIjXRx@x200m
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/773df883e8f7543958d0d719c025b5f47c5a67f0
Modified Files
--------------
src/backend/access/common/reloptions.c | 132 +++++++++++++++++++--
src/backend/access/gist/gistbuild.c | 25 +---
src/backend/access/gist/gistutil.c | 2 +-
src/backend/commands/view.c | 18 ---
src/include/access/gist_private.h | 10 +-
src/include/access/reloptions.h | 24 ++++
src/include/commands/view.h | 2 -
src/include/utils/rel.h | 25 ++--
src/test/modules/dummy_index_am/dummy_index_am.c | 37 ++++--
src/test/modules/dummy_index_am/sql/reloptions.sql | 10 ++
src/test/regress/expected/gist.out | 2 +-
src/test/regress/expected/updatable_views.out | 2 +-
12 files changed, 214 insertions(+), 75 deletions(-)
On 2019-Sep-25, Alvaro Herrera wrote:
Support reloptions of enum type
I forgot to "git add" an expected output file. Pushing in a minute ..
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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: /messages/by-id/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
Attachments:
reloptions-lockmode.patchtext/x-diff; charset=us-asciiDownload+5-8
On Thu, Sep 26, 2019 at 08:41:52AM +0900, Michael Paquier wrote:
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).
And applied as of fbfa566.
--
Michael
Hello, sorry I didn't notice this email.
On 2019-Sep-26, Michael Paquier wrote:
This part from commit 773df88 is incorrect:
+ /* + * 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;
You're right, this was a merge blunder :-( Sorry about that.
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).
Thanks for fixing. It all looks correct to me.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services