Обсуждение: 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: https://postgr.es/m/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: 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
Вложения
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
On Thu, Sep 26, 2019 at 10:19:47PM -0300, Alvaro Herrera wrote: > Thanks for fixing. It all looks correct to me. Thanks for double-checking. -- Michael