Hello,
On Tue, Dec 1, 2020 at 9:40 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Mon, 30 Nov 2020 21:03:45 -0300, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in
> > On 2020-Nov-26, Kyotaro Horiguchi wrote:
> >
> > > This shares RI_ConstraintInfo cache by constraints that shares the
> > > same parent constraints. But you forgot that the cache contains some
> > > members that can differ among partitions.
> > >
> > > Consider the case of attaching a partition that have experienced a
> > > column deletion.
> >
> > I think this can be solved easily in the patch, by having
> > ri_BuildQueryKey() compare the parent's fk_attnums to the parent; if
> > they are equal then use the parent's constaint_id, otherwise use the
> > child constraint. That way, the cache entry is reused in the common
> > case where they are identical.
>
> *I* think it's the direction. After an off-list discussion, we
> confirmed that even in that case the patch works as is because
> fk_attnum (or contuple.conkey) always stores key attnums compatible
> to the topmost parent when conparent has a valid value (assuming the
> current usage of fk_attnum), but I still feel uneasy to rely on that
> unclear behavior.
Hmm, I don't see what the problem is here, because it's not
RI_ConstraintInfo that is being shared among partitions of the same
parent, but the RI query (and the SPIPlanPtr for it) that is issued
through SPI. The query (and the plan) turns out to be the same no
matter what partition's RI_ConstraintInfo is first used to generate
it. What am I missing?
BTW, one problem with Kuroda-san's patch is that it's using
conparentid as the shared key, which can still lead to multiple
queries being generated when using multi-level partitioning, because
there would be many (intermediate) parent constraints in that case.
We should really be using the "root" constraint OID as the key,
because there would only be one root from which all constraints in a
given partition hierarchy would've originated. Actually, I had
written a patch a few months back to do exactly that, a polished
version of which I've attached with this email. Please take a look.
--
Amit Langote
EDB: http://www.enterprisedb.com