Custom reloptions and lock modes

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
Subject: Custom reloptions and lock modes
Date: 2019-09-20 01:38:31
Message-ID: 20190920013831.GD1844@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

This is a new thread related to the bug analyzed here:
https://www.postgresql.org/message-id/20190919083203.GC21144@paquier.xyz

And in short, if you attempt to do an ALTER TABLE with a custom
reloptions the command burns itself, like that for example this
sequence:
create extension bloom;
create table aa (a int);
create index aa_bloom ON aa USING bloom (a);
alter index aa_bloom set (length = 20);

Which results in the following error:
ERROR: XX000: unrecognized lock mode: 2139062143
LOCATION: LockAcquireExtended, lock.c:756

The root of the problem is that the set of relation options loaded
finds properly the custom options set when looking for the lock mode
to use in AlterTableGetRelOptionsLockLevel(), but we never set the
lock mode this option should use when allocating it, resulting in a
failure. The current set of APIs does not allow either to set the
lock mode associated with a custom reloption.

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.
- 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.

My take would be to use 0001 on all branches (or I am missing
something related to custom relopts manipulation?), and consider 0002
on HEAD.

Thoughts?
--
Michael

Attachment Content-Type Size
0001-Fix-failure-for-lock-mode-used-for-custom-relation-o.patch text/x-diff 2.1 KB
0002-Allow-definition-of-lock-mode-for-custom-reloptions.patch text/x-diff 6.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-09-20 01:42:33 Re: Syntax highlighting for Postgres spec files
Previous Message Michael Paquier 2019-09-20 01:01:55 Re: subscriptionCheck failures on nightjar