Hi, + skip = !ExecLockTableTuple(erm->relation, &tid, markSlot, + estate->es_snapshot, estate->es_output_cid, + lockmode, erm->waitPolicy, &epq_needed); + if (skip)
It seems the variable skip is only used above. The variable is not needed - if statement can directly check the return value. + * Locks tuple with given TID with given lockmode following given wait given appears three times in the above sentence. Maybe the following is bit easier to read: Locks tuple with the specified TID, lockmode following given wait policy + * Checks whether a tuple containing the same unique key as extracted from the + * tuple provided in 'slot' exists in 'pk_rel'. I think 'same' is not needed here since the remaining part of the sentence has adequately identified the key. + if (leaf_pk_rel == NULL) + goto done; It would be better to avoid goto by including the cleanup statements in the if block and return. + if (index_getnext_slot(scan, ForwardScanDirection, outslot)) + found = true; + + /* Found tuple, try to lock it in key share mode. */ + if (found) Since found is only assigned in one place, the two if statements can be combined into one. Cheers On Fri, Apr 2, 2021 at 5:46 AM Amit Langote <amitlangot...@gmail.com> wrote: > On Sat, Mar 20, 2021 at 10:21 PM Amit Langote <amitlangot...@gmail.com> > wrote: > > Updated patches attached. Sorry about the delay. > > Rebased over the recent DETACH PARTITION CONCURRENTLY work. > Apparently, ri_ReferencedKeyExists() was using the wrong snapshot. > > -- > Amit Langote > EDB: http://www.enterprisedb.com >
