On Wed, Mar 10, 2021 at 8:37 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > Amit Langote <amitlangot...@gmail.com> writes: > > On Fri, Mar 5, 2021 at 6:00 AM Tom Lane <t...@sss.pgh.pa.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
v5-0001-Share-SPI-plan-between-partitions-in-some-RI-trig.patch
Description: Binary data
