Re: simplifying foreign key/RI checks

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 <amitlangote09(at)gmail(dot)com> wrote:

> On Sat, Mar 20, 2021 at 10:21 PM Amit Langote <amitlangote09(at)gmail(dot)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
>