Re: Proper object locking for GRANT/REVOKE

Hi,

On Sat, Nov 09, 2024 at 01:43:13PM +0100, Peter Eisentraut wrote:
> On 31.10.24 15:26, Bertrand Drouvot wrote:
> > + address = get_object_address(objtype, lfirst(cell), &relation, lockmode, false);
> > + Assert(relation == NULL);
> >
> > Worth to explain why we do expect relation to be NULL here? (the comment on top
> > of get_object_address() says it all, but maybe a few words here could be worth
> > it).
>
> There are several other callers with this pattern.

Right. And most of those places declare a Relation prior calling get_object_address()
_only_ for a following assertion.

> Maybe it would be better to push the assertion into get_object_address(),
> something like
>
> Assert(!relation || relp)
>
> near the end.

That looks like a good idea to me, that would make the code cleaner and easier
to understand.

> Meaning, if you pass NULL for the relp argument, then you
> don't expect a relation. This is kind of what will happen now anyway,
> except with a segfault instead of an assertion.

Yeah, I like it.

So, something like the attached (provided as .txt file to no mess up the CF bot
entry related to this thread) could be applied before?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com