Re: simplifying foreign key/RI checks

On Wed, Jan 27, 2021 at 5:32 PM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
> 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?)

Yeah, better to just remove those _PK macros and say this module no
longer runs any queries on the PK table.

How about the attached?

--
Amit Langote
EDB: http://www.enterprisedb.com