> On Monday, July 29, 2024 6:59 PM Dilip Kumar <dilipbal...@gmail.com> > wrote: > > > > On Mon, Jul 29, 2024 at 11:44 AM Zhijie Hou (Fujitsu) > > <houzj.f...@fujitsu.com> wrote: > > > > > > > I was going through v7-0001, and I have some initial comments. > > Thanks for the comments ! > > > > > @@ -536,11 +542,9 @@ ExecCheckIndexConstraints(ResultRelInfo > > *resultRelInfo, TupleTableSlot *slot, > > ExprContext *econtext; > > Datum values[INDEX_MAX_KEYS]; > > bool isnull[INDEX_MAX_KEYS]; > > - ItemPointerData invalidItemPtr; > > bool checkedIndex = false; > > > > ItemPointerSetInvalid(conflictTid); > > - ItemPointerSetInvalid(&invalidItemPtr); > > > > /* > > * Get information from the result relation info structure. > > @@ -629,7 +633,7 @@ ExecCheckIndexConstraints(ResultRelInfo > > *resultRelInfo, TupleTableSlot *slot, > > > > satisfiesConstraint = > > check_exclusion_or_unique_constraint(heapRelation, indexRelation, > > - indexInfo, &invalidItemPtr, > > + indexInfo, &slot->tts_tid, > > values, isnull, estate, false, > > CEOUC_WAIT, true, > > conflictTid); > > > > What is the purpose of this change? I mean > > 'check_exclusion_or_unique_constraint' says that 'tupleid' > > should be invalidItemPtr if the tuple is not yet inserted and > > ExecCheckIndexConstraints is called by ExecInsert before inserting the > tuple. > > So what is this change? > > Because the function ExecCheckIndexConstraints() is now invoked after > inserting a tuple (in the patch). So, we need to ignore the newly inserted > tuple > when checking conflict in check_exclusion_or_unique_constraint(). > > > Would this change ExecInsert's behavior as well? > > Thanks for pointing it out, I will check and reply.
After checking, I think it may affect ExecInsert's behavior if the slot passed to ExecCheckIndexConstraints() comes from other tables (e.g. when executing INSERT INTO SELECT FROM othertbl), because the slot->tts_tid points to a valid position from another table in this case, which can cause the check_exclusion_or_unique_constraint to skip a tuple unexpectedly). I thought about two ideas to fix this: One is to reset the slot->tts_tid before calling ExecCheckIndexConstraints() in ExecInsert(), but I feel a bit uncomfortable to this since it is touching existing logic. So, another idea is to just add a new parameter 'tupletid' in ExecCheckIndexConstraints(), then pass tupletid=InvalidOffsetNumber in when invoke the function in ExecInsert() and pass a valid tupletid in the new code paths in the patch. The new 'tupletid' will be passed to check_exclusion_or_unique_constraint to skip the target tuple. I feel the second one maybe better. What do you think ? Best Regards, Hou zj
