From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
cc: Hans Buschmann <buschmann@nidsa.net>, Peter Geoghegan <pg@bowt.ie>,
        "David G. Johnston" <david.g.johnston@gmail.com>,
        "pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Subject: Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with
 relid xxxxx
In-reply-to: 
 <CA+HiwqHEdX7SSTLJw0=F3nU_BRyn2tUsw7A-cQs+=M_RL0czSw@mail.gmail.com>
References: <18147-6fc796538913ee88@postgresql.org>
 <CAKFQuwYFq-Q2rxbXXBmdHCAEFm-Pu_qhPAEi-PO+LmjtYN2hhg@mail.gmail.com>
 <2796464.1696525626@sss.pgh.pa.us>
 <9fdfed847859435e8641e613198be9e3@nidsa.net>
 <3319880.1696778678@sss.pgh.pa.us>
 <3c9fe737f6aa4ae9b0dc19bffd97e106@nidsa.net>
 <408285.1698085850@sss.pgh.pa.us> <428393.1698094765@sss.pgh.pa.us>
 <CA+HiwqHEdX7SSTLJw0=F3nU_BRyn2tUsw7A-cQs+=M_RL0czSw@mail.gmail.com>
Comments: In-reply-to Amit Langote <amitlangote09@gmail.com>
	message dated "Tue, 24 Oct 2023 18:48:06 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <556851.1698158932.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Tue, 24 Oct 2023 10:48:52 -0400
Message-ID: <556852.1698158932@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/556852.1698158932%40sss.pgh.pa.us>
Precedence: bulk

Amit Langote <amitlangote09@gmail.com> writes:
> On Tue, Oct 24, 2023 at 5:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.

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.

BTW, I noted while looking at this example that there are two
RTEPermissionInfo structs with identical contents, one for the
appendrel RTE and one for the first child.  That seems kind of
unnecessary.

> 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:

Yuck.  But either proposal above would fix that, right?

			regards, tom lane