Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx

On Wed, Oct 25, 2023 at 4:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> > I suggest that it might be cleaner if we make rootRelation point
> > to the original appendrel (that is, the RTE entry with inh = true).
> > That would be exactly parallel to the partitioning situation, in
> > that that RTE is not scanned in the plan.  But it's for the same
> > table as what's normally the first result table, so it should have
> > the same permissions info.
>
> After looking closer I see that that's exactly what's happening
> in your patch, so it should all be good.  We can make the code in
> grouping_planner be simpler rather than more complicated, though.
>
> I went ahead and pushed that to v14 and up, with adjustments of
> relevant comments and a test case.

Thanks.

>  There are still loose ends
> here, though:
>
> * The wrong-table-for-triggers bug occurs pre-v14, but the patch
> doesn't come close to applying because both the planner and
> executor look quite a bit different in this area.  We could
> devise a separate patch maybe, but given the lack of field
> complaints I'm not sure this is worth doing.  I'd be afraid
> to put such a patch into v11 at this point anyway, given that
> there will be no opportunity to fix any new bugs after November.

Yeah, it might not be worthwhile to try to back-patch this to 12 and 13.

> * I'm still seeing the extra RTEPermissionsInfo.  It looks like that's
> a consequence of this kluge in expand_single_inheritance_child:
>
>     /*
>      * No permission checking for the child RTE unless it's the parent
>      * relation in its child role, which only applies to traditional
>      * inheritance.
>      */
>     if (childOID != parentOID)
>         childrte->perminfoindex = 0;
>
> I suspect that now this should just unconditionally clear
> childrte->perminfoindex, but it's minor cleanup not a bug fix
> so I didn't pursue that in the initial patch.

Would you like me to apply something like the attached?

> * It seems like ModifyTable.nominalRelation and
> ModifyTable.rootRelation are pretty darn redundant.  Maybe we
> should make an effort to get rid of one of them.  Or maybe
> it's not worth the trouble.

We had a discussion on unifying the two before:
https://www.postgresql.org/message-id/12148.1538938507%40sss.pgh.pa.us

I'm fine with leaving that as-is.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com