Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

On 2022-Dec-12, Amit Langote wrote:

> On Sun, Dec 11, 2022 at 6:25 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > I've attached 0001 to remove those extraneous code blocks and add a
> > comment mentioning that userid need not be recomputed.
> >
> > While staring at the build_simple_rel() bit mentioned above, I
> > realized that this code fails to set userid correctly in the
> > inheritance parent rels that are child relations of subquery parent
> > relations, such as UNION ALL subqueries. In that case, instead of
> > copying the userid (= 0) of the parent rel, the child should look up
> > its own RTEPermissionInfo, which should be there, and use the
> > checkAsUser value from there. I've attached 0002 to fix this hole. I
> > am not sure whether there's a way to add a test case for this in the
> > core suite.
>
> Ah, I realized we could just expand the test added by 553d2ec27 with a
> wrapper view (to test checkAsUser functionality) and a UNION ALL query
> over the view (to test this change).

Hmm, but if I run this test without the code change in 0002, the test
also passes (to wit: the plan still has two hash joins), so I understand
that either you're testing something that's not changed by the patch,
or the test case itself isn't really what you wanted.

As for 0001, it seems simpler to me to put the 'userid' variable in the
same scope as 'onerel', and then compute it just once and don't bother
with it at all afterwards, as in the attached.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Al principio era UNIX, y UNIX habló y dijo: "Hello world\n".
No dijo "Hello New Jersey\n", ni "Hello USA\n".