On Tue, Jul 30, 2024 at 1:49 PM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > > 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 ?
Yes, the second approach seems good to me. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
