Re: Proper object locking for GRANT/REVOKE

On Mon, Jun 23, 2025 at 04:16:09PM -0700, Noah Misch wrote:
> On Wed, Jun 11, 2025 at 05:22:53PM +0200, Peter Eisentraut wrote:
>> There is an open item for PG18 for this. So here is a patch that adds a
>> comment back, mostly from your descriptions in this thread.
>>
>> > > The change to AccessShareLock at least prevents confusing "cache lookup
>> > > failed" messages, and might alleviate some security concerns about swapping
>> > > in a completely different object concurrently (even if we currently think
>> > > this is not an actual problem).
>> >
>> > Perhaps. To me, the v17 behavior smells mildly superior to the v18 behavior.
>>
>> Hmm. I think there has been a general effort to get rid of internal errors
>> like "cache lookup failed ..." and replace them with proper user-facing
>> errors. This change seems in line with that.
>
> I have seen some commits along those lines, e.g. d8a0993. They weren't adding
> lock acquisitions, though. If we've deliberately incurred lock acquisitions
> just to get rid of "cache lookup failed ...", I don't remember those commits.
>
>> An alternative, if we wanted to go back to the old behavior (other than
>> reverting altogether, since I think the refactoring is still valuable),
>> would be to allow get_object_address() to work with lockmode == NoLock. That
>> would require a bit of work, but nothing magical.
>
> That seems a bit better to me than your comment-only proposal, but either
> could be okay.

There is still an open item for this. After a read-through, I tend to
agree with Noah that it might not be worth incurring extra lock
acquisitions solely in the name of avoiding cache lookup errors. This
seems like something that could perhaps be revisited for v19, but it's
pretty late in the game for v18...

--
nathan