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

On Tue, Oct 24, 2023 at 5:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> > So the proximate problem is that we're trying to fetch an updatedCols
> > bitmapset for a child table that has no permissions checks to make
> > and thus doesn't need an RTEPermissionInfo, so its perminfoindex is
> > zero.  It might be that this relinfo should have ri_RootResultRelInfo
> > set, but it doesn't.
>
> That seems to be a fairly good guess.  I was able to reduce the
> test case to this:
>
> drop table if exists t1 cascade;
>
> create table t1 (f1 int, f2 int, f3 int
>   , check (f1 < 10) no inherit
> );
>
> create table t2 () inherits(t1);
>
> insert into t2 select i, i+1, 0 from generate_series(1,1000) i;
>
> create index on t2(f1,f2);
>
> update t1 set f3 = 11 where f1 = 12 and f2 = 13;
>
>
> You don't see the failure without the check constraint on t1.
> What is happening is that with that, we remove t1 from the plan
> entirely, causing us to think that t2 is the root target relation,
> and then things blow up because the root should have an
> RTEPermissionInfo and doesn't.  The place where things start
> to go off the rails is in ExecInitModifyTable:
>
>          * If it's a partitioned table, the root partition doesn't appear
>          * elsewhere in the plan and its RT index is given explicitly in
>          * node->rootRelation.  Otherwise (i.e. table inheritance) the target
>          * relation is the first relation in the node->resultRelations list.
>
> node->resultRelations contains only t2, so boom.
>
> Maybe we could fix this by setting ModifyTable.rootRelation to
> the parent relation in the plain-inheritance case not only the
> partitioned case; but I've not investigated what else might be
> expecting it to be set only for partitions.

That appears to be the easiest fix to me (attached PoC makes the error
go away for me and passes check-world).

By setting rootRelation for plan-inheritance roots, there would be two
ResultRelInfos for the root relation (that is, if it's also present in
resultRelations, unlike in this case) -- one in
ModifyTableState.rootResultRelInfo and another in the 1st element of
ModifyTableState.resultRelInfo[].  That might be fine, because,
AFAICS, the partitioning-related code sites that look at the former
have checks to ensure that they don't accidentally try to access a
plain-inheritance relation as a partitioned one.  I might need to take
a closer look.

> According to this understanding, the root cause is quite old
> and the RTEPermissionInfo failure just exposes it in an obvious
> way.  I wonder whether we can find related cases that misbehave
> even before v16.

I decided to look at fire{BS|AS}Triggers() first, because we changed
them in f49b85d783f to look at ModifyTableState.rootResultRelInfo to
get the table whose triggers to fire.  They seem broken for this case:

create or replace function t1_stmt_trig_func() returns trigger as $$
begin raise notice 'updating t1'; return NULL; end; $$ language
plpgsql;
create trigger t1_stmt_trig before update on t1 execute function
t1_stmt_trig_func();

-- trigger should've been fired
update t1 set f3 = 11 where f1 = 12 and f2 = 13;
ERROR:  invalid perminfoindex 0 in RTE with relid 32799

-- partitioned roots seem fine
create table p (a int check (a = 0)) partition by list (a);
create table p1 partition of p default;
create trigger p_stmt_trig before update on p execute function
t1_stmt_trig_func();
update p set a = 1 where a = 1;
NOTICE:  updating t1
UPDATE 0

I recalled we fixed a similar case in ab5fcf2b04f.  I suspect we would
not have seen this report if we had also considered the
excluded-root-parent case in that commit, though I might be wrong.

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