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