At Sun, 24 Jan 2021 20:51:39 +0900, Amit Langote <amitlangot...@gmail.com> 
wrote in 
> Here's v5.

At Mon, 25 Jan 2021 18:19:56 +0900, Amit Langote <amitlangot...@gmail.com> 
wrote in 
> > Anybody else want to look this patch over before I mark it Ready For 
> > Committer?
> 
> Would be nice to have others look it over.  Thanks.

This nice improvement.

0001 just looks fine.

0002:

 /* RI query type codes */
-/* these queries are executed against the PK (referenced) table: */
+/*
+ * 1 and 2  are no longer used, because PK (referenced) table is looked up
+ * directly using ri_ReferencedKeyExists().
 #define RI_PLAN_CHECK_LOOKUPPK                 1
 #define RI_PLAN_CHECK_LOOKUPPK_FROM_PK 2
 #define RI_PLAN_LAST_ON_PK                             
RI_PLAN_CHECK_LOOKUPPK_FROM_PK

However, this patch does.

+       if (!ri_ReferencedKeyExists(pk_rel, fk_rel, newslot, riinfo))
+               ri_ReportViolation(riinfo,
+                                                  pk_rel, fk_rel,
+                                                  newslot,
+                                                  NULL,
+                                                  RI_PLAN_CHECK_LOOKUPPK, 
false);

It seems to me 1 (RI_PLAN_CHECK_LOOKUPPK) is still alive. (Yeah, I
know that doesn't mean the usefulness of the macro but the mechanism
the macro suggests, but it is confusing.) On the other hand,
RI_PLAN_CHECK_LOOKUPPK_FROM_PK and RI_PLAN_LAST_ON_PK seem to be no
longer used.  (Couldn't we remove them?)

(about the latter, we can rewrite the only use of it "if
(qkey->constr_queryno <= RI_PLAN_LAST_ON_PK)" not to use the macro.)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


Reply via email to