Re: Huge memory consumption on partitioned table with FKs

On Wed, Mar 10, 2021 at 8:37 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> > On Fri, Mar 5, 2021 at 6:00 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> This claim seems false on its face:
> >>> All child constraints of a given foreign key constraint can use the
> >>> same RI query and the resulting plan, that is, no need to create as
> >>> many copies of the query and the plan as there are partitions, as
> >>> happens now due to the child constraint OID being used in the key
> >>> for ri_query_cache.
>
> > The quoted comment could have been written to be clearer about this,
> > but it's not talking about the table that is to be queried, but the
> > table whose RI trigger is being executed. In all the cases except one
> > (mentioned below), the table that is queried is the same irrespective
> > of which partition's trigger is being executed, so we're basically
> > creating the same plan separately for each partition.
>
> Hmm. So, the key point is that the values coming from the partitioned
> child table are injected into the test query as parameters, not as
> column references, thus it doesn't matter *to the test query* what
> numbers the referencing columns have in that child. We just have to
> be sure we pass the right parameter values.

Right.

> But ... doesn't the code
> use riinfo->fk_attnums[] to pull out the values to be passed?

Yes, from a slot that belongs to the child table.

> IOW, I now get the point about being able to share the SPI plans,
> but I'm still dubious about sharing the RI_ConstraintInfo cache entries.

There was actually a proposal upthread about sharing the
RI_ConstraintInfo too, but we decided to not pursue that for now.

> It looks to me like the v4 patch is now actually not sharing the
> cache entries, ie their hash key is just the child constraint OID
> same as before;

Yeah, you may see that we're only changing ri_BuildQueryKey() in the
patch affecting only ri_query_cache, but not ri_LoadConstraintInfo()
which leaves ri_constraint_cache unaffected.

> but the comments are pretty confused about this.

I've tried improving the comment in ri_BuildQueryKey() a bit to make
clear what is and what is not being shared between partitions.

> It might be simpler if you add just one new field which is the OID of
> the constraint that we're building the SPI query from, which might be
> either equal to constraint_id, or the OID of some parent constraint.
> In particular it's not clear to me why we need both constraint_parent
> and constraint_root_id.

Yeah, I think constraint_parent is a leftover from some earlier
hacking. I have removed it.

Attached updated patch.

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