database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-10-19 15:49:24 +03:00
Code Activity

Fix lookup code for REINDEX INDEX.

Browse Source 
This commit adjusts RangeVarCallbackForReindexIndex() to handle an
extremely unlikely race condition involving concurrent OID reuse.
In short, if REINDEX INDEX is executed at the same time that the
index is re-created with the same name and OID but a different
parent table OID, we might lock the wrong parent table.  To fix,
simply detect when this happens and emit an ERROR.  Unfortunately,
we can't gracefully handle this situation because we will have
already locked the index, and we must lock the parent table before
the index to avoid deadlocks.

While at it, I've replaced all but one early return in this
callback function with ERRORs that should be unreachable.  While I
haven't verified the presence of a live bug, the checks in question
appear to be unnecessary, and the early returns seem prone to
breaking the parent table locking code in subtle ways.  If nothing
else, this simplifies the code a bit.

This is a bug fix and could be back-patched, but given the presumed
rarity of the race condition and the lack of reports, I'm not going
to bother.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Jeff Davis <pgsql@j-davis.com>
Discussion: https://postgr.es/m/Z8zwVmGzXyDdkAXj%40nathan
This commit is contained in:
Nathan Bossart
2025-10-15 16:32:40 -05:00
parent af164f31b9
commit 079480dc20
1 changed files with 25 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
src/backend/commands/indexcmds.c
View File

@@ -2976,6 +2976,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
struct ReindexIndexCallbackState *state = arg;
LOCKMODE table_lockmode;
Oid table_oid;
AclResult aclresult;
/*
* Lock level here should match table lock in reindex_index() for
@@ -3000,45 +3001,44 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
if (!OidIsValid(relId))
return;
/*
* If the relation does exist, check whether it's an index. But note that
* the relation might have been dropped between the time we did the name
* lookup and now. In that case, there's nothing to do.
*/
/* If the relation does exist, check whether it's an index. */
relkind = get_rel_relkind(relId);
if (!relkind)
return;
if (relkind != RELKIND_INDEX &&
relkind != RELKIND_PARTITIONED_INDEX)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("\"%s\" is not an index", relation->relname)));
/* Check permissions */
table_oid = IndexGetRelation(relId, true);
if (OidIsValid(table_oid))
{
AclResult aclresult;
/* Look up the index's table. */
table_oid = IndexGetRelation(relId, false);
/*
* In the unlikely event that, upon retry, we get the same index OID with
* a different table OID, fail. RangeVarGetRelidExtended() will have
* already locked the index in this case, and it won't retry again, so we
* can't lock the newly discovered table OID without risking deadlock.
* Also, while this corner case is indeed possible, it is extremely
* unlikely to happen in practice, so it's probably not worth any more
* effort than this.
*/
if (relId == oldRelId && table_oid != state->locked_table_oid)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("index \"%s\" was concurrently dropped",
relation->relname)));
/* Check permissions. */
aclresult = pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, OBJECT_INDEX, relation->relname);
}
/* Lock heap before index to avoid deadlock. */
if (relId != oldRelId)
{
/*
* If the OID isn't valid, it means the index was concurrently
* dropped, which is not a problem for us; just return normally.
*/
if (OidIsValid(table_oid))
{
LockRelationOid(table_oid, table_lockmode);
state->locked_table_oid = table_oid;
}
}
}
/*
* ReindexTable