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

On Fri, Feb 17, 2023 at 9:02 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> On 2022-Dec-11, Amit Langote wrote:
> > 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.
>
> I gave this a look and I thought it was clearer to have the new
> condition depend on rel->reloptkind instead parent or no.

Thanks for looking into this again. I agree the condition with
reloptkind might be better.

> I tried a few things for a new test case, but I was unable to find
> anything useful. Maybe an intermediate view, I thought; no dice.
> Maybe one with a security barrier would do? Anyway, for now I just kept
> what you added in v2.

Hmm, I'm fine with leaving the test case out if it doesn't really test
the code we're changing, as you also pointed out?

One more thing we could try is come up with a postgres_fdw test case,
because it uses the RelOptInfo.userid value for remote-costs-based
path size estimation. But adding a test case to contrib module's
suite test a core planner change might seem strange, ;-).

Attaching v4 without the test case.

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