Re: simplifying foreign key/RI checks

On Wed, Jan 27, 2021 at 5:32 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
> At Sun, 24 Jan 2021 20:51:39 +0900, Amit Langote <amitlangote09@gmail.com> wrote in
> > Here's v5.
>
> At Mon, 25 Jan 2021 18:19:56 +0900, Amit Langote <amitlangote09@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?)

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