On Sun, 25 May 2025 at 13:41, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On Sun, 25 May 2025 at 13:06, Tender Wang <tndrwang@gmail.com> wrote:
> >
> > For a partitioned table, we must pass rootResultRelInfo to ExecInsert(). I added the check before calling
ExecInsert()
> > If it is a partitioned table, we continue to pass rootResultRelInfo. Otherwise, we pass resultRelInfo.
> > Please see the attached diff file. The patch passed all regression test cases.
> >
>
> No, I don't think that's the right fix. I'm looking at it now, and I
> think I have a fix, but it's more complicated than that. I'll post an
> update later.
>
The reason that MERGE must pass rootResultRelInfo to ExecInsert() for
a plain-inheritance table dates back to 387f9ed0a08. As that commit
demonstrates, it is possible for the parent to be excluded from the
plan, and so all of the entries in the resultRelInfo array may be for
different relations than rootResultRelInfo.
So there are indeed two related bugs here:
1. The projection built by ExecInitMerge() in the INSERT case may use
a tuple slot that's not compatible with the target table.
2. ExecInitModifyTable() does not initialize the WCO lists or
RETURNING list for rootResultRelInfo, so those never get executed.
As it happens, it is possible to construct cases where (1) causes a
crash, even without WCO's or a RETURNING list (see the first test case
in the attached patch), so this bug goes all the way back to v15,
where MERGE was introduced.
So I think we need to do something like the attached.
There is perhaps scope to reduce the code duplication between this and
ExecInitPartitionInfo(), but that'd likely make the patch bigger, so I
think it's best to leave that for now.
Regards,
Dean
