Joe Conway <mail@joeconway.com> writes:
> On 3/16/21 2:45 PM, Joe Conway wrote:
>> Ian, or anyone else, any comments/complaints on my changes? If not I will commit
>> and push that version sooner rather than later.
I took a quick look, and I'm afraid I don't believe this:
! * We have to check for dropped attribute first, and we rely on the
! * syscache not to notice a concurrent drop before pg_attribute_aclcheck
! * fetches the row.
What the existing code is assuming is that if we do a successful
SearchSysCache check, then an immediately following pg_xxx_aclcheck call
that needs to examine that same row will also find that row in the cache,
because there will be no need for any actual database traffic to do that.
What you've done here is changed that pattern to be
SearchSysCache(row1)
SearchSysCache(row2)
aclcheck on row1
aclcheck on row2
But that does NOT offer any such guarantee, because the row2 cache lookup
could incur a database access, which might notice that row1 has been
deleted.
I think we may have to adjust the acl.c APIs, or maybe better provide new
entry points, so that we can have variants of pg_xxx_aclcheck that won't
throw a hard error upon not finding the row. We cheesily tried to avoid
adjusting those APIs to support the semantics we need here, and we now see
that it didn't really work.
regards, tom lane
