Re: Custom reloptions and lock modes

On Fri, Sep 20, 2019 at 7:08 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> Hence attached is a patch set to address those issues:
> - 0001 makes sure that any existing module creating a custom reloption
> has the lock mode set to AccessExclusiveMode, which would be a sane
> default anyway.  I think that we should just back-patch that and close
> any holes.
Looks good to me. The patch solves the issue and passes with
regression tests. IMHO, it should be back-patched to all the branches.

> - 0002 is a patch which we could use to extend the existing reloption
> APIs to set the lock mode used.  I am aware of the recent work done by
> Nikolay in CC to rework this API set, but I am unsure where we are
> going there, and the resulting patch is actually very short, being
> 20-line long with the current infrastructure.  That could go into
> HEAD.  Table AMs have been added in v12 so custom reloptions could
> gain more in popularity, but as we are very close to the release it
> would not be cool to break those APIs.  The patch simplicity could
> also be a reason sufficient for a back-patch, and I don't think that
> there are many users of them yet.
>
I think this is good approach for now and can be committed on the HEAD only.

One small thing:

  add_int_reloption(bl_relopt_kind, buf,
   "Number of bits generated for each index column",
-  DEFAULT_BLOOM_BITS, 1, MAX_BLOOM_BITS);
+  DEFAULT_BLOOM_BITS, 1, MAX_BLOOM_BITS,
+  AccessExclusiveLock);
Do we need a comment to explain why we're using AccessExclusiveLock in
this case?

-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com