Re: simplifying foreign key/RI checks

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

At Mon, 25 Jan 2021 18:19:56 +0900, Amit Langote  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