> On 2024-Oct-25, Tender Wang wrote: > > > Thanks for reporting. I can reproduce this memory invalid access on HEAD. > > After debuging codes, I found the root cause. > > In DetachPartitionFinalize(), below code: > > att = TupleDescAttr(RelationGetDescr(partRel), > > attmap->attnums[conkey[i] - 1] - 1); > > > > We should use confkey[i] -1 not conkey[i] - 1; > > Wow, how embarrasing, now that's one _really_ stupid bug and it's > totally my own. Thanks for the analysis! I'll get this patched soon.
Actually, now that I look at this again, I think this proposed fix is wrong; conkey/confkey confusion is not what the problem is. Rather, the problem is that we're applying a tuple conversion map when none should be applied. So the fix is to remove "attmap" altogether. One thing that didn't appear correct to me was that the patch was changing one constraint name so that it appeared that the constrained columns were "id, p_id" but that didn't make sense: they are "p_id, p_jd" instead.
Yeah, The constrained name change made me think about if the patch is correct.
After your explanation, I have understood it.
Then I realized that you're wrong that it's the referenced side that we're processing: what we're doing there is generate the fk_attrs list, which is the list of constrained columns (not the list of referenced columns, which is pk_attrs).
I also felt like modifying the fkpart12 test rather than adding a separate fkpart13, so I did that.
So here's a v2 for this. (Commit message needs love still, but it's a bit late here.)
The v2 LGTM.
BTW, while reviewing the v2 patch, I found the parentConTup in foreach(cell, fks) block
didn't need it anymore. We can remove the related codes.
