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
On Fri, Jan 22, 2021 at 3:22 PM Corey Huinker <corey.huinker@gmail.com> wrote:
>> I decided not to deviate from pk_ terminology so that the new code
>> doesn't look too different from the other code in the file. Although,
>> I guess we can at least call the main function
>> ri_ReferencedKeyExists() instead of ri_PrimaryKeyExists(), so I've
>> changed that.
>
> I think that's a nice compromise, it makes the reader aware of the concept.
>>
>> I've attached the updated patch.
>
> Missing "break" added. Check.
> Comment updated. Check.
> Function renamed. Check.
> Attribute mapping matching test (and assertion) added. Check.
> Patch applies to an as-of-today master, passes make check and check world.
> No additional regression tests required, as no new functionality is introduced.
> No docs required, as there is nothing user-facing.
Thanks for the review.
> Questions:
> 1. There's a palloc for mapped_partkey_attnums, which is never freed, is the prevailing memory context short lived enough that we don't care?
> 2. Same question for the AtrrMap map, should there be a free_attrmap().
I hadn't checked, but yes, the prevailing context is
AfterTriggerTupleContext, a short-lived one that is reset for every
trigger event tuple. I'm still inclined to explicitly free those
objects, so changed like that. While at it, I also changed
mapped_partkey_attnums to root_partattrs for readability.
Attached v4.
--
Amit Langote
EDB: http://www.enterprisedb.com
