On Sat, Jan 23, 2021 at 12:52 PM Zhihong Yu <z...@yugabyte.com> wrote:
> Hi,
>
> + for (i = 0; i < riinfo->nkeys; i++)
> + {
> + Oid eq_opr = eq_oprs[i];
> + Oid typeid = RIAttType(fk_rel, riinfo->fk_attnums[i]);
> + RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid);
> +
> + if (pk_nulls[i] != 'n' &&
> OidIsValid(entry->cast_func_finfo.fn_oid))
>
> It seems the pk_nulls[i] != 'n' check can be lifted ahead of the
> assignment to the three local variables. That way, ri_HashCompareOp
> wouldn't be called when pk_nulls[i] == 'n'.
>
> + case TM_Updated:
> + if (IsolationUsesXactSnapshot())
> ...
> + case TM_Deleted:
> + if (IsolationUsesXactSnapshot())
>
> It seems the handling for TM_Updated and TM_Deleted is the same. The cases
> for these two values can be put next to each other (saving one block of
> code).
>
> Cheers
>
I'll pause on reviewing v4 until you've addressed the suggestions above.
