On 2023-Jan-19, Tom Lane wrote:
> I wrote:
> > Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> >> Remove some dead code in selfuncs.c
> >> RelOptInfo.userid is the same for all relations in a given inheritance
> >> tree, so the code in examine_variable() and example_simple_variable()
> >> that repeats the ACL checks on the root parent rel instead of a given
> >> leaf child relations need not recompute userid too.
>
> > This change seems rather ill-advised.
>
> Ah, sorry, -ENOCAFFEINE. It's talking about the access-as-user field,
> not the relation's owner. I agree that as querytrees are currently
> built, this is probably a safe optimization. But do we really want
> to hard-wire such a subtle assumption to gain a microscopic speed
> benefit? It's not as though GetUserId() is expensive.
Well, I didn't see it as an optimization but rather a removal of
confusing code. Given the current RTEPermissionInfo representation,
it's just not possible for an "otherrel" to have a different
access-as-user value than what "the relation mentioned in the query"
has -- by construction, they share the same RTEPermissionInfo.
If we wanted to decouple selfuncs.c from that knowledge, then what we
should be doing is obtain the RTEPermissionInfo for the relation, and
use the userid from there. But the code I deleted wasn't doing that, it
was just using the same 'onerel' all the time. It's not difficult (or
expensive) to do otherwise, but it seems somewhat pointless.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"El destino baraja y nosotros jugamos" (A. Schopenhauer)
