Re: simplifying foreign key/RI checks

At Sun, 24 Jan 2021 20:51:39 +0900, Amit Langote <amitlangote09(at)gmail(dot)com> wrote in
> Here's v5.

At Mon, 25 Jan 2021 18:19:56 +0900, Amit Langote <amitlangote09(at)gmail(dot)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