Re: Allow table AMs to define their own reloptions

[Rui Zhao]

Hi Andrew,

Thanks for adding the helper for standard options, which is very helpful
for AM extensions.

I found an issue:
In a fresh backend, ALTER TABLE can change an AM-specific option without
taking the lock requested by the AM. dummy_table_am registers option_int
with AccessExclusiveLock, but I got ShareUpdateExclusiveLock on v13.

Setup in one connection:

CREATE EXTENSION dummy_table_am;
CREATE TABLE t_std (a int) USING dummy_table_am WITH (option_int = 7);

Then, in a new connection:

BEGIN;
ALTER TABLE t_std SET (option_int = 9);
SELECT mode FROM pg_locks
WHERE pid = pg_backend_pid() AND relation = 't_std'::regclass;
-- ShareUpdateExclusiveLock
ROLLBACK;

With LOAD 'dummy_table_am' before BEGIN in another new connection, the
same commands take AccessExclusiveLock. RESET has the same problem.

The reason is that ProcessUtilitySlow() calls AlterTableGetLockLevel()
before opening the relation and loading its AM. option_int is not
registered at that point.
AlterTableGetRelOptionsLockLevel() ignores the unknown name and returns
NoLock, leaving AlterTableGetLockLevel()'s ShareUpdateExclusiveLock
minimum unchanged. Loading the AM later does not upgrade that lock.

This is an existing helper issue, not a v13 regression. I reproduced it
with bloom on unpatched master at e75fcc9867f. Setup in one connection:

CREATE EXTENSION bloom;
CREATE TABLE bloom_probe (a int);
CREATE INDEX bloom_probe_idx ON bloom_probe USING bloom (a);

Then, in a new connection:

BEGIN;
ALTER INDEX bloom_probe_idx SET (length = 100);
SELECT mode FROM pg_locks
WHERE pid = pg_backend_pid() AND relation = 'bloom_probe_idx'::regclass
ORDER BY mode;
-- ShareUpdateExclusiveLock
ROLLBACK;

With LOAD 'bloom' before BEGIN in another new connection, the same
commands take AccessExclusiveLock.

With 0001 on top of v13, an option not yet registered requires
AccessExclusiveLock. Registered options keep their declared lock levels.
Until the AM is loaded, we cannot tell which lock the option requires.
This ensures the lock is strong enough. I think this is a harmless and
safe fix with minimal changes.

The TAP test checks SET and RESET for both test AMs, with and without
loading the library first, and checks that fillfactor still uses the
weaker lock.

Core regression, isolation and dummy_table_am tests passed with the
patch. Without the fix, the four cases without LOAD fail.

Regards,
Rui

nocfbot-0001-Use-a-strong-lock-for-unregistered-relation-options.patch
Description: Binary data