Horton

Re: Clarification on Role Access Rights to Table Indexes

Nathan Bossart <nathandbossart@gmail.com>

View thread View on postgr.es
From: Nathan Bossart <nathandbossart@gmail.com>
To: Ayush Vatsa <ayushvatsa1810@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, "David G. Johnston" <david.g.johnston@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-03-08T21:08:02Z
Lists: pgsql-hackers, pgsql-general 
On Sat, Mar 08, 2025 at 08:34:40PM +0530, Ayush Vatsa wrote:
>> I'm wondering whether setting missing_ok to true is correct here.  IIUC we
>> should have an AccessShareLock on the index, but I don't know if that's
>> enough protection.
> 
> Since we are already opening the relation with rel = relation_open(relOid,
> AccessShareLock);,
> if relOid does not exist, it will throw an error. If it does exist, we
> acquire an AccessShareLock,
> preventing it from being dropped.
> 
> By the time we reach IndexGetRelation(), we can be confident that relOid
> exists and is
> protected by the lock. Given this, it makes sense to keep missing_ok = false
> here.
> 
> Let me know if you agree or if you see any scenario where
> missing_ok = true would be preferable-I can update the condition
> accordingly.

Right, we will have a lock on the index, but my concern is that we won't
have a lock on its table.  I was specifically concerned that a concurrent
DROP TABLE could cause IndexGetRelation() to fail, i.e., emit a gross
"cache lookup failed" error.  From a quick test and skim of the relevant
code, I think your patch is fine, though.  IndexGetRelation() retrieves the
table OID from pg_index, so the OID should definitely be valid.  And IIUC
DROP TABLE first acquires a lock on the table and its dependent objects
(e.g., indexes) before any actual deletions, so AFAICT there's no problem
with using it in pg_class_aclcheck() and get_rel_name(), either.

-- 
nathan

Commits

  1. Fix privilege checks for pg_prewarm() on indexes.

  2. Fix lookup code for REINDEX INDEX.

  3. Fix redefinition of typedef RangeVar.

  4. Fix lookups in pg_{clear,restore}_{attribute,relation}_stats().

  5. dblink: Avoid locking relation before privilege check.