Dean Rasheed <dean.a.rash...@gmail.com> 于2025年5月26日周一 04:10写道：
> On Sun, 25 May 2025 at 13:41, Dean Rasheed <dean.a.rash...@gmail.com> > wrote: > > > > On Sun, 25 May 2025 at 13:06, Tender Wang <tndrw...@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. > 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
