public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: Ayush Vatsa <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Clarification on Role Access Rights to Table Indexes
Date: Sat, 8 Mar 2025 15:08:02 -0600
Message-ID: <Z8yxsm9ZWVkHlPbV@nathan> (raw)
In-Reply-To: <CACX+KaO4R9QDxbPSxSB0jNXFsqA6Jf=UPS+tyUvT_YvuP_grVA@mail.gmail.com>
References: <[email protected]>
	<CAKFQuwZThU_Z-Zw+3mr+ecp1BVOw777dp3nXU5-wTVk3kS10gw@mail.gmail.com>
	<CA+TgmoZG71zBpLOfCGZqGhtp=88z6=YYhi54TEsCtKr3v+UpoA@mail.gmail.com>
	<[email protected]>
	<CA+Tgmob_W0iq9Kuugra3WYTO2429RMJ_+HkVukrXWOUN81QiEw@mail.gmail.com>
	<[email protected]>
	<CA+TgmoZYM2az+yCWu5DBnV50N_BE9f1r8-Doy6-tZTySeb-s+A@mail.gmail.com>
	<CACX+KaNAbOzePn710EtzH9F5xiUdBC+u59=UMab=Wr8jgDKQtw@mail.gmail.com>
	<Z8dcGMMP3-D5dobY@nathan>
	<CACX+KaO4R9QDxbPSxSB0jNXFsqA6Jf=UPS+tyUvT_YvuP_grVA@mail.gmail.com>

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






view thread (19+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Clarification on Role Access Rights to Table Indexes
  In-Reply-To: <Z8yxsm9ZWVkHlPbV@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox