On Fri, Sep 20, 2019 at 7:08 AM Michael Paquier <mich...@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
