1
0
mirror of https://github.com/postgres/postgres.git synced 2025-06-13 07:41:39 +03:00

Lock before setting relhassubclass on RELKIND_PARTITIONED_INDEX.

Commit 5b562644fe added a comment that
SetRelationHasSubclass() callers must hold this lock.  When commit
17f206fbc8 extended use of this column to
partitioned indexes, it didn't take the lock.  As the latter commit
message mentioned, we currently never reset a partitioned index to
relhassubclass=f.  That largely avoids harm from the lock omission.  The
cause for fixing this now is to unblock introducing a rule about locks
required to heap_update() a pg_class row.  This might cause more
deadlocks.  It gives minor user-visible benefits:

- If an ALTER INDEX SET TABLESPACE runs concurrently with ALTER TABLE
  ATTACH PARTITION or CREATE PARTITION OF, one transaction blocks
  instead of failing with "tuple concurrently updated".  (Many cases of
  DDL concurrency still fail that way.)

- Match ALTER INDEX ATTACH PARTITION in choosing to lock the index.

While not user-visible today, we'll need this if we ever make something
set the flag to false for a partitioned index, like ANALYZE does today
for tables.  Back-patch to v12 (all supported versions), the plan for
the commit relying on the new rule.  In back branches, add
LockOrStrongerHeldByMe() instead of adding a LockHeldByMe() parameter.

Reviewed (in an earlier version) by Robert Haas.

Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
This commit is contained in:
Noah Misch
2024-06-27 19:21:05 -07:00
parent f88cdb36c4
commit 0cecc908e9
7 changed files with 61 additions and 30 deletions

View File

@ -3483,8 +3483,15 @@ findAttrByName(const char *attributeName, const List *columns)
* SetRelationHasSubclass
* Set the value of the relation's relhassubclass field in pg_class.
*
* NOTE: caller must be holding an appropriate lock on the relation.
* ShareUpdateExclusiveLock is sufficient.
* It's always safe to set this field to true, because all SQL commands are
* ready to see true and then find no children. On the other hand, commands
* generally assume zero children if this is false.
*
* Caller must hold any self-exclusive lock until end of transaction. If the
* new value is false, caller must have acquired that lock before reading the
* evidence that justified the false value. That way, it properly waits if
* another backend is simultaneously concluding no need to change the tuple
* (new and old values are true).
*
* NOTE: an important side-effect of this operation is that an SI invalidation
* message is sent out to all backends --- including me --- causing plans
@ -3499,6 +3506,11 @@ SetRelationHasSubclass(Oid relationId, bool relhassubclass)
HeapTuple tuple;
Form_pg_class classtuple;
Assert(CheckRelationOidLockedByMe(relationId,
ShareUpdateExclusiveLock, false) ||
CheckRelationOidLockedByMe(relationId,
ShareRowExclusiveLock, true));
/*
* Fetch a modifiable copy of the tuple, modify it, update pg_class.
*/