Re: A bug with ExecCheckPermissions

On Wed, Feb 8, 2023 at 16:19 Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

> On 2023-Feb-08, o(dot)tselebrovskiy(at)postgrespro(dot)ru wrote:
>
> > But if you debug function ExecCheckPermissions and look into what is
> passed
> > to function (contents of rangeTable and rteperminfos to be exact),
> > you'll see some strange behaviour:
>
> > Both of RangeTableEntries have a perminfoindex of 0 and simultaneously
> have
> > a RTEPERMISSIONINFO entry for them!
>
> Ouch. Yeah, that's not great. As you say, it doesn't really affect
> anything, and we know full well that these RTEs are ad-hoc
> manufactured. But as we claim that we still pass the RTEs for the
> benefit of hooks, then we should at least make them match.

+1. We don’t have anything in this (core) code path that would try to use
perminfoindex for these RTEs, but there might well be in the future.

I think we should also patch ExecCheckPermissions to use forboth(),
> scanning the RTEs as it goes over the perminfos, and make sure that the
> entries are consistent.

Hmm, we can’t use forboth here, because not all RTEs have the corresponding
RTEPermissionInfo, inheritance children RTEs, for example. Also, it
doesn’t make much sense to reinstate the original loop over range table and
fetch the RTEPermissionInfo for the RTEs with non-0 perminfoindex, because
the main goal of the patch was to make ExecCheckPermissions() independent
of range table length.

> --
Thanks, Amit Langote
EDB: http://www.enterprisedb.com