Re: MERGE issues around inheritance

Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> 于2025年5月26日周一 04:10写道：

> On Sun, 25 May 2025 at 13:41, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
> wrote:
> >
> > On Sun, 25 May 2025 at 13:06, Tender Wang <tndrwang(at)gmail(dot)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.
>

Thanks for the information. Passing resultRelInfo to ExecInsert() is wrong.

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

I tested the attached patch, and there's no problem anymore. LGTM.

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

Agreed.

--
Thanks,
Tender Wang