Re: Huge memory consumption on partitioned table with FKs - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Huge memory consumption on partitioned table with FKs
Date
Msg-id 20201203.171316.1242674483141105976.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Huge memory consumption on partitioned table with FKs  (Amit Langote <amitlangote09@gmail.com>)
Responses Re: Huge memory consumption on partitioned table with FKs
Re: Huge memory consumption on partitioned table with FKs
Re: Huge memory consumption on partitioned table with FKs
List pgsql-hackers
At Thu, 3 Dec 2020 16:41:45 +0900, Amit Langote <amitlangote09@gmail.com> wrote in 
> On Thu, Dec 3, 2020 at 2:29 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> > At Thu, 3 Dec 2020 12:27:53 +0900, Amit Langote <amitlangote09@gmail.com> wrote in
> > > On Thu, Dec 3, 2020 at 10:15 AM Kyotaro Horiguchi
> > > <horikyota.ntt@gmail.com> wrote:
> > > For the queries on the referencing side ("check" side),
> > > type/collation/attribute name determined using the above are going to
> > > be the same for all partitions in a given tree irrespective of the
> > > attribute number, because they're logically the same column.  On the
> >
> > Yes, I know that, which is what I meant by "practically" or
> > "actually", but it is not explicitly defined AFAICS.
> 
> Well, I think it's great that we don't have to worry *in this part of
> the code* about partition's fk_attnums not being congruent with the
> root parent's, because ensuring that is the responsibility of the
> other parts of the system such as DDL.  If we have any problems in
> this area, they should be dealt with by ensuring that there are no
> bugs in those other parts.

Agreed.

> > Thus that would be no longer an issue if we explicitly define that
> > "When conpraentid stores a valid value, each element of fk_attnums
> > points to logically the same attribute with the RI_ConstraintInfo for
> > the parent constraint."  Or I'd be happy if we have such a comment
> > there instead.
> 
> I saw a comment in Kuroda-san's v2 patch that is perhaps meant to
> address this point, but the placement needs to be reconsidered:

Ah, yes, that comes from my proposal.

> @@ -366,6 +368,14 @@ RI_FKey_check(TriggerData *trigdata)
>         querysep = "WHERE";
>         for (int i = 0; i < riinfo->nkeys; i++)
>         {
> +
> +           /*
> +           * We share the same plan among all relations in a partition
> +           * hierarchy.  The plan is guaranteed to be compatible since all of
> +           * the member relations are guaranteed to have the equivalent set
> +           * of foreign keys in fk_attnums[].
> +           */
> +
>             Oid         pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
>             Oid         fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
> 
> A more appropriate place for this kind of comment would be where
> fk_attnums is defined or in ri_BuildQueryKey() that is shared by
> different RI query issuing functions.

Yeah, I wanted more appropriate place for the comment.  That place
seems reasonable.

> > > referenced side ("restrict", "cascade", "set" side), as you already
> > > mentioned, fk_attnums refers to the top parent table of the
> > > referencing side, so no possibility of they being different in the
> > > various referenced partitions' RI_ConstraintInfos.
> >
> > Right. (I'm not sure I have mention that here, though:p)A
> 
> Maybe I misread but I think you did in your email dated Dec 1 where you said:
> 
> "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."

fk_attnums *doesn't* refers to to top parent talbe of the referencing
side. it refers to attributes of the partition that is compatible with
the same element of fk_attnums of the topmost parent.  Maybe I'm
misreading.


> > > On the topic of how we'd be able to share even the RI_ConstraintInfos
> > > among partitions, that would indeed look a bit more elaborate than the
> > > patch we have right now.
> >
> > Maybe just letting the hash entry for the child riinfo point to the
> > parent riinfo if all members (other than constraint_id, of course)
> > share the exactly the same values.  No need to count references since
> > we don't going to remove riinfos.
> 
> Ah, something maybe worth trying.  Although the memory we'd save by
> sharing the RI_ConstraintInfos would not add that much to the savings
> we're having by sharing the plan, because it's the plans that are a
> memory hog AFAIK.

I agree that plans are rather large but the sharable part of the
RI_ConstraintInfos is 536 bytes, I'm not sure it is small enough
comparing to the plans.  But that has somewhat large footprint.. (See
the attached)

> > > > About your patch, it calculates the root constrid at the time an
> > > > riinfo is created, but when the root-partition is further attached to
> > > > another partitioned-table after the riinfo creation,
> > > > constraint_root_id gets stale.  Of course that dones't matter
> > > > practically, though.
> > >
> > > Maybe we could also store the hash value of the root constraint OID as
> > > rootHashValue and check for that one too in
> > > InvalidateConstraintCacheCallBack().  That would take care of this
> > > unless I'm missing something.
> >
> > Seems to be sound.
> 
> Okay, thanks.
> 
> I have attached a patch in which I've tried to merge the ideas from
> both my patch and Kuroda-san's.  I liked that his patch added
> conparentid to RI_ConstraintInfo because that saves a needless
> syscache lookup for constraints that don't have a parent.  I've kept
> my idea to compute the root constraint id only once in
> ri_LoadConstraint(), not on every invocation of ri_BuildQueryKey().
> Kuroda-san, anything you'd like to add to that?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From bbdd0647452a26fdf7da313967f85241c6111fcb Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Thu, 3 Dec 2020 16:15:57 +0900
Subject: [PATCH 1/2] separte riinfo

---
 src/backend/utils/adt/ri_triggers.c | 281 ++++++++++++++++------------
 1 file changed, 161 insertions(+), 120 deletions(-)

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 02b1a3868f..3f8407c311 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -97,14 +97,10 @@
  * Information extracted from an FK pg_constraint entry.  This is cached in
  * ri_constraint_cache.
  */
-typedef struct RI_ConstraintInfo
+typedef struct RI_ConstraintParam
 {
-    Oid            constraint_id;    /* OID of pg_constraint entry (hash key) */
-    bool        valid;            /* successfully initialized? */
-    uint32        oidHashValue;    /* hash value of pg_constraint OID */
-    NameData    conname;        /* name of the FK constraint */
+    Oid            query_key;
     Oid            pk_relid;        /* referenced relation */
-    Oid            fk_relid;        /* referencing relation */
     char        confupdtype;    /* foreign key's ON UPDATE action */
     char        confdeltype;    /* foreign key's ON DELETE action */
     char        confmatchtype;    /* foreign key's match type */
@@ -114,6 +110,17 @@ typedef struct RI_ConstraintInfo
     Oid            pf_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (PK = FK) */
     Oid            pp_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (PK = PK) */
     Oid            ff_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (FK = FK) */
+    struct RI_ConstraintInfo *ownerinfo; /* owner info of this param */
+} RI_ConstraintParam;
+
+typedef struct RI_ConstraintInfo
+{
+    Oid            constraint_id;
+    bool        valid;            /* successfully initialized? */
+    uint32        oidHashValue;    /* hash value of pg_constraint OID */
+    NameData    conname;        /* name of the FK constraint */
+    Oid            fk_relid;        /* referencing relation */
+    RI_ConstraintParam *param;    /* sharable parameters  */
     dlist_node    valid_link;        /* Link in list of valid entries */
 } RI_ConstraintInfo;
 
@@ -264,7 +271,7 @@ RI_FKey_check(TriggerData *trigdata)
      * SELECT FOR KEY SHARE will get on it.
      */
     fk_rel = trigdata->tg_relation;
-    pk_rel = table_open(riinfo->pk_relid, RowShareLock);
+    pk_rel = table_open(riinfo->param->pk_relid, RowShareLock);
 
     switch (ri_NullCheck(RelationGetDescr(fk_rel), newslot, riinfo, false))
     {
@@ -283,7 +290,7 @@ RI_FKey_check(TriggerData *trigdata)
              * This is the only case that differs between the three kinds of
              * MATCH.
              */
-            switch (riinfo->confmatchtype)
+            switch (riinfo->param->confmatchtype)
             {
                 case FKCONSTR_MATCH_FULL:
 
@@ -364,17 +371,17 @@ RI_FKey_check(TriggerData *trigdata)
         appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x",
                          pk_only, pkrelname);
         querysep = "WHERE";
-        for (int i = 0; i < riinfo->nkeys; i++)
+        for (int i = 0; i < riinfo->param->nkeys; i++)
         {
-            Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
+            Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_type = RIAttType(fk_rel, riinfo->param->fk_attnums[i]);
 
             quoteOneName(attname,
-                         RIAttName(pk_rel, riinfo->pk_attnums[i]));
+                         RIAttName(pk_rel, riinfo->param->pk_attnums[i]));
             sprintf(paramname, "$%d", i + 1);
             ri_GenerateQual(&querybuf, querysep,
                             attname, pk_type,
-                            riinfo->pf_eq_oprs[i],
+                            riinfo->param->pf_eq_oprs[i],
                             paramname, fk_type);
             querysep = "AND";
             queryoids[i] = fk_type;
@@ -382,7 +389,7 @@ RI_FKey_check(TriggerData *trigdata)
         appendStringInfoString(&querybuf, " FOR KEY SHARE OF x");
 
         /* Prepare and save the plan */
-        qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
+        qplan = ri_PlanCheck(querybuf.data, riinfo->param->nkeys, queryoids,
                              &qkey, fk_rel, pk_rel);
     }
 
@@ -492,16 +499,16 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
         appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x",
                          pk_only, pkrelname);
         querysep = "WHERE";
-        for (int i = 0; i < riinfo->nkeys; i++)
+        for (int i = 0; i < riinfo->param->nkeys; i++)
         {
-            Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
+            Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
 
             quoteOneName(attname,
-                         RIAttName(pk_rel, riinfo->pk_attnums[i]));
+                         RIAttName(pk_rel, riinfo->param->pk_attnums[i]));
             sprintf(paramname, "$%d", i + 1);
             ri_GenerateQual(&querybuf, querysep,
                             attname, pk_type,
-                            riinfo->pp_eq_oprs[i],
+                            riinfo->param->pp_eq_oprs[i],
                             paramname, pk_type);
             querysep = "AND";
             queryoids[i] = pk_type;
@@ -509,7 +516,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
         appendStringInfoString(&querybuf, " FOR KEY SHARE OF x");
 
         /* Prepare and save the plan */
-        qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
+        qplan = ri_PlanCheck(querybuf.data, riinfo->param->nkeys, queryoids,
                              &qkey, fk_rel, pk_rel);
     }
 
@@ -679,19 +686,19 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
         appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x",
                          fk_only, fkrelname);
         querysep = "WHERE";
-        for (int i = 0; i < riinfo->nkeys; i++)
+        for (int i = 0; i < riinfo->param->nkeys; i++)
         {
-            Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
-            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
+            Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_type = RIAttType(fk_rel, riinfo->param->fk_attnums[i]);
+            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->param->fk_attnums[i]);
 
             quoteOneName(attname,
-                         RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                         RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
             sprintf(paramname, "$%d", i + 1);
             ri_GenerateQual(&querybuf, querysep,
                             paramname, pk_type,
-                            riinfo->pf_eq_oprs[i],
+                            riinfo->param->pf_eq_oprs[i],
                             attname, fk_type);
             if (pk_coll != fk_coll && !get_collation_isdeterministic(pk_coll))
                 ri_GenerateQualCollation(&querybuf, pk_coll);
@@ -701,7 +708,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
         appendStringInfoString(&querybuf, " FOR KEY SHARE OF x");
 
         /* Prepare and save the plan */
-        qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
+        qplan = ri_PlanCheck(querybuf.data, riinfo->param->nkeys, queryoids,
                              &qkey, fk_rel, pk_rel);
     }
 
@@ -785,19 +792,19 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
         appendStringInfo(&querybuf, "DELETE FROM %s%s",
                          fk_only, fkrelname);
         querysep = "WHERE";
-        for (int i = 0; i < riinfo->nkeys; i++)
+        for (int i = 0; i < riinfo->param->nkeys; i++)
         {
-            Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
-            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
+            Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_type = RIAttType(fk_rel, riinfo->param->fk_attnums[i]);
+            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->param->fk_attnums[i]);
 
             quoteOneName(attname,
-                         RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                         RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
             sprintf(paramname, "$%d", i + 1);
             ri_GenerateQual(&querybuf, querysep,
                             paramname, pk_type,
-                            riinfo->pf_eq_oprs[i],
+                            riinfo->param->pf_eq_oprs[i],
                             attname, fk_type);
             if (pk_coll != fk_coll && !get_collation_isdeterministic(pk_coll))
                 ri_GenerateQualCollation(&querybuf, pk_coll);
@@ -806,7 +813,7 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
         }
 
         /* Prepare and save the plan */
-        qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
+        qplan = ri_PlanCheck(querybuf.data, riinfo->param->nkeys, queryoids,
                              &qkey, fk_rel, pk_rel);
     }
 
@@ -901,22 +908,22 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
                          fk_only, fkrelname);
         querysep = "";
         qualsep = "WHERE";
-        for (int i = 0, j = riinfo->nkeys; i < riinfo->nkeys; i++, j++)
+        for (int i = 0, j = riinfo->param->nkeys; i < riinfo->param->nkeys; i++, j++)
         {
-            Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
-            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
+            Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_type = RIAttType(fk_rel, riinfo->param->fk_attnums[i]);
+            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->param->fk_attnums[i]);
 
             quoteOneName(attname,
-                         RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                         RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
             appendStringInfo(&querybuf,
                              "%s %s = $%d",
                              querysep, attname, i + 1);
             sprintf(paramname, "$%d", j + 1);
             ri_GenerateQual(&qualbuf, qualsep,
                             paramname, pk_type,
-                            riinfo->pf_eq_oprs[i],
+                            riinfo->param->pf_eq_oprs[i],
                             attname, fk_type);
             if (pk_coll != fk_coll && !get_collation_isdeterministic(pk_coll))
                 ri_GenerateQualCollation(&querybuf, pk_coll);
@@ -928,7 +935,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
         appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
 
         /* Prepare and save the plan */
-        qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys * 2, queryoids,
+        qplan = ri_PlanCheck(querybuf.data, riinfo->param->nkeys * 2, queryoids,
                              &qkey, fk_rel, pk_rel);
     }
 
@@ -1080,15 +1087,15 @@ ri_set(TriggerData *trigdata, bool is_set_null)
                          fk_only, fkrelname);
         querysep = "";
         qualsep = "WHERE";
-        for (int i = 0; i < riinfo->nkeys; i++)
+        for (int i = 0; i < riinfo->param->nkeys; i++)
         {
-            Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
-            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
-            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
+            Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_type = RIAttType(fk_rel, riinfo->param->fk_attnums[i]);
+            Oid            pk_coll = RIAttCollation(pk_rel, riinfo->param->pk_attnums[i]);
+            Oid            fk_coll = RIAttCollation(fk_rel, riinfo->param->fk_attnums[i]);
 
             quoteOneName(attname,
-                         RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                         RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
             appendStringInfo(&querybuf,
                              "%s %s = %s",
                              querysep, attname,
@@ -1096,7 +1103,7 @@ ri_set(TriggerData *trigdata, bool is_set_null)
             sprintf(paramname, "$%d", i + 1);
             ri_GenerateQual(&qualbuf, qualsep,
                             paramname, pk_type,
-                            riinfo->pf_eq_oprs[i],
+                            riinfo->param->pf_eq_oprs[i],
                             attname, fk_type);
             if (pk_coll != fk_coll && !get_collation_isdeterministic(pk_coll))
                 ri_GenerateQualCollation(&querybuf, pk_coll);
@@ -1107,7 +1114,7 @@ ri_set(TriggerData *trigdata, bool is_set_null)
         appendBinaryStringInfo(&querybuf, qualbuf.data, qualbuf.len);
 
         /* Prepare and save the plan */
-        qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
+        qplan = ri_PlanCheck(querybuf.data, riinfo->param->nkeys, queryoids,
                              &qkey, fk_rel, pk_rel);
     }
 
@@ -1217,7 +1224,7 @@ RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel,
      */
     else if (ri_nullcheck == RI_KEYS_SOME_NULL)
     {
-        switch (riinfo->confmatchtype)
+        switch (riinfo->param->confmatchtype)
         {
             case FKCONSTR_MATCH_SIMPLE:
 
@@ -1333,14 +1340,14 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
     fkrte->rellockmode = AccessShareLock;
     fkrte->requiredPerms = ACL_SELECT;
 
-    for (int i = 0; i < riinfo->nkeys; i++)
+    for (int i = 0; i < riinfo->param->nkeys; i++)
     {
         int            attno;
 
-        attno = riinfo->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
+        attno = riinfo->param->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
         pkrte->selectedCols = bms_add_member(pkrte->selectedCols, attno);
 
-        attno = riinfo->fk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
+        attno = riinfo->param->fk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
         fkrte->selectedCols = bms_add_member(fkrte->selectedCols, attno);
     }
 
@@ -1377,10 +1384,10 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
     initStringInfo(&querybuf);
     appendStringInfoString(&querybuf, "SELECT ");
     sep = "";
-    for (int i = 0; i < riinfo->nkeys; i++)
+    for (int i = 0; i < riinfo->param->nkeys; i++)
     {
         quoteOneName(fkattname,
-                     RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                     RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
         appendStringInfo(&querybuf, "%sfk.%s", sep, fkattname);
         sep = ", ";
     }
@@ -1398,20 +1405,20 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
     strcpy(pkattname, "pk.");
     strcpy(fkattname, "fk.");
     sep = "(";
-    for (int i = 0; i < riinfo->nkeys; i++)
+    for (int i = 0; i < riinfo->param->nkeys; i++)
     {
-        Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
-        Oid            fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
-        Oid            pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
-        Oid            fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
+        Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
+        Oid            fk_type = RIAttType(fk_rel, riinfo->param->fk_attnums[i]);
+        Oid            pk_coll = RIAttCollation(pk_rel, riinfo->param->pk_attnums[i]);
+        Oid            fk_coll = RIAttCollation(fk_rel, riinfo->param->fk_attnums[i]);
 
         quoteOneName(pkattname + 3,
-                     RIAttName(pk_rel, riinfo->pk_attnums[i]));
+                     RIAttName(pk_rel, riinfo->param->pk_attnums[i]));
         quoteOneName(fkattname + 3,
-                     RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                     RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
         ri_GenerateQual(&querybuf, sep,
                         pkattname, pk_type,
-                        riinfo->pf_eq_oprs[i],
+                        riinfo->param->pf_eq_oprs[i],
                         fkattname, fk_type);
         if (pk_coll != fk_coll)
             ri_GenerateQualCollation(&querybuf, pk_coll);
@@ -1422,17 +1429,17 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
      * It's sufficient to test any one pk attribute for null to detect a join
      * failure.
      */
-    quoteOneName(pkattname, RIAttName(pk_rel, riinfo->pk_attnums[0]));
+    quoteOneName(pkattname, RIAttName(pk_rel, riinfo->param->pk_attnums[0]));
     appendStringInfo(&querybuf, ") WHERE pk.%s IS NULL AND (", pkattname);
 
     sep = "";
-    for (int i = 0; i < riinfo->nkeys; i++)
+    for (int i = 0; i < riinfo->param->nkeys; i++)
     {
-        quoteOneName(fkattname, RIAttName(fk_rel, riinfo->fk_attnums[i]));
+        quoteOneName(fkattname, RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
         appendStringInfo(&querybuf,
                          "%sfk.%s IS NOT NULL",
                          sep, fkattname);
-        switch (riinfo->confmatchtype)
+        switch (riinfo->param->confmatchtype)
         {
             case FKCONSTR_MATCH_SIMPLE:
                 sep = " AND ";
@@ -1505,7 +1512,10 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
         HeapTuple    tuple = SPI_tuptable->vals[0];
         TupleDesc    tupdesc = SPI_tuptable->tupdesc;
         RI_ConstraintInfo fake_riinfo;
+        RI_ConstraintParam fake_riparam;
 
+        fake_riinfo.param = &fake_riparam;
+        
         slot = MakeSingleTupleTableSlot(tupdesc, &TTSOpsVirtual);
 
         heap_deform_tuple(tuple, tupdesc,
@@ -1522,15 +1532,15 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
          * or fk_rel's tupdesc.
          */
         memcpy(&fake_riinfo, riinfo, sizeof(RI_ConstraintInfo));
-        for (int i = 0; i < fake_riinfo.nkeys; i++)
-            fake_riinfo.fk_attnums[i] = i + 1;
+        for (int i = 0; i < fake_riinfo.param->nkeys; i++)
+            fake_riinfo.param->fk_attnums[i] = i + 1;
 
         /*
          * If it's MATCH FULL, and there are any nulls in the FK keys,
          * complain about that rather than the lack of a match.  MATCH FULL
          * disallows partially-null FK rows.
          */
-        if (fake_riinfo.confmatchtype == FKCONSTR_MATCH_FULL &&
+        if (fake_riinfo.param->confmatchtype == FKCONSTR_MATCH_FULL &&
             ri_NullCheck(tupdesc, slot, &fake_riinfo, false) != RI_KEYS_NONE_NULL)
             ereport(ERROR,
                     (errcode(ERRCODE_FOREIGN_KEY_VIOLATION),
@@ -1615,10 +1625,10 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
     initStringInfo(&querybuf);
     appendStringInfoString(&querybuf, "SELECT ");
     sep = "";
-    for (i = 0; i < riinfo->nkeys; i++)
+    for (i = 0; i < riinfo->param->nkeys; i++)
     {
         quoteOneName(fkattname,
-                     RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                     RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
         appendStringInfo(&querybuf, "%sfk.%s", sep, fkattname);
         sep = ", ";
     }
@@ -1633,20 +1643,20 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
     strcpy(pkattname, "pk.");
     strcpy(fkattname, "fk.");
     sep = "(";
-    for (i = 0; i < riinfo->nkeys; i++)
+    for (i = 0; i < riinfo->param->nkeys; i++)
     {
-        Oid            pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
-        Oid            fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
-        Oid            pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
-        Oid            fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
+        Oid            pk_type = RIAttType(pk_rel, riinfo->param->pk_attnums[i]);
+        Oid            fk_type = RIAttType(fk_rel, riinfo->param->fk_attnums[i]);
+        Oid            pk_coll = RIAttCollation(pk_rel, riinfo->param->pk_attnums[i]);
+        Oid            fk_coll = RIAttCollation(fk_rel, riinfo->param->fk_attnums[i]);
 
         quoteOneName(pkattname + 3,
-                     RIAttName(pk_rel, riinfo->pk_attnums[i]));
+                     RIAttName(pk_rel, riinfo->param->pk_attnums[i]));
         quoteOneName(fkattname + 3,
-                     RIAttName(fk_rel, riinfo->fk_attnums[i]));
+                     RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
         ri_GenerateQual(&querybuf, sep,
                         pkattname, pk_type,
-                        riinfo->pf_eq_oprs[i],
+                        riinfo->param->pf_eq_oprs[i],
                         fkattname, fk_type);
         if (pk_coll != fk_coll)
             ri_GenerateQualCollation(&querybuf, pk_coll);
@@ -1666,13 +1676,13 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
         appendStringInfoString(&querybuf, ") WHERE (");
 
     sep = "";
-    for (i = 0; i < riinfo->nkeys; i++)
+    for (i = 0; i < riinfo->param->nkeys; i++)
     {
-        quoteOneName(fkattname, RIAttName(fk_rel, riinfo->fk_attnums[i]));
+        quoteOneName(fkattname, RIAttName(fk_rel, riinfo->param->fk_attnums[i]));
         appendStringInfo(&querybuf,
                          "%sfk.%s IS NOT NULL",
                          sep, fkattname);
-        switch (riinfo->confmatchtype)
+        switch (riinfo->param->confmatchtype)
         {
             case FKCONSTR_MATCH_SIMPLE:
                 sep = " AND ";
@@ -1762,8 +1772,8 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
          * or fk_rel's tupdesc.
          */
         memcpy(&fake_riinfo, riinfo, sizeof(RI_ConstraintInfo));
-        for (i = 0; i < fake_riinfo.nkeys; i++)
-            fake_riinfo.pk_attnums[i] = i + 1;
+        for (i = 0; i < fake_riinfo.param->nkeys; i++)
+            fake_riinfo.param->pk_attnums[i] = i + 1;
 
         ri_ReportViolation(&fake_riinfo, pk_rel, fk_rel,
                            slot, tupdesc, 0, true);
@@ -1905,7 +1915,7 @@ ri_BuildQueryKey(RI_QueryKey *key, const RI_ConstraintInfo *riinfo,
      * We assume struct RI_QueryKey contains no padding bytes, else we'd need
      * to use memset to clear them.
      */
-    key->constr_id = riinfo->constraint_id;
+    key->constr_id = riinfo->param->query_key;
     key->constr_queryno = constr_queryno;
 }
 
@@ -1983,25 +1993,25 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation trig_rel, bool rel_is_pk)
     if (rel_is_pk)
     {
         if (riinfo->fk_relid != trigger->tgconstrrelid ||
-            riinfo->pk_relid != RelationGetRelid(trig_rel))
+            riinfo->param->pk_relid != RelationGetRelid(trig_rel))
             elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"",
                  trigger->tgname, RelationGetRelationName(trig_rel));
     }
     else
     {
         if (riinfo->fk_relid != RelationGetRelid(trig_rel) ||
-            riinfo->pk_relid != trigger->tgconstrrelid)
+            riinfo->param->pk_relid != trigger->tgconstrrelid)
             elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"",
                  trigger->tgname, RelationGetRelationName(trig_rel));
     }
 
-    if (riinfo->confmatchtype != FKCONSTR_MATCH_FULL &&
-        riinfo->confmatchtype != FKCONSTR_MATCH_PARTIAL &&
-        riinfo->confmatchtype != FKCONSTR_MATCH_SIMPLE)
+    if (riinfo->param->confmatchtype != FKCONSTR_MATCH_FULL &&
+        riinfo->param->confmatchtype != FKCONSTR_MATCH_PARTIAL &&
+        riinfo->param->confmatchtype != FKCONSTR_MATCH_SIMPLE)
         elog(ERROR, "unrecognized confmatchtype: %d",
-             riinfo->confmatchtype);
+             riinfo->param->confmatchtype);
 
-    if (riinfo->confmatchtype == FKCONSTR_MATCH_PARTIAL)
+    if (riinfo->param->confmatchtype == FKCONSTR_MATCH_PARTIAL)
         ereport(ERROR,
                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                  errmsg("MATCH PARTIAL not yet implemented")));
@@ -2009,6 +2019,27 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation trig_rel, bool rel_is_pk)
     return riinfo;
 }
 
+/*
+ * get_ri_constraint_root
+ *        Returns a given RI constraint's root parent
+ */
+static Oid
+get_ri_constraint_root(Oid constrOid)
+{
+    HeapTuple    tuple;
+    Oid            constrParentOid;
+
+    tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(constrOid));
+    if (!HeapTupleIsValid(tuple))
+        elog(ERROR, "cache lookup failed for constraint %u", constrOid);
+    constrParentOid = ((Form_pg_constraint) GETSTRUCT(tuple))->conparentid;
+    ReleaseSysCache(tuple);
+    if (OidIsValid(constrParentOid))
+        return get_ri_constraint_root(constrParentOid);
+
+    return constrOid;
+}
+
 /*
  * Fetch or create the RI_ConstraintInfo struct for an FK constraint.
  */
@@ -2032,11 +2063,20 @@ ri_LoadConstraintInfo(Oid constraintOid)
     riinfo = (RI_ConstraintInfo *) hash_search(ri_constraint_cache,
                                                (void *) &constraintOid,
                                                HASH_ENTER, &found);
+
     if (!found)
         riinfo->valid = false;
     else if (riinfo->valid)
         return riinfo;
 
+    /* allocate riinfo's own param memory if needed */
+    if (!found || riinfo->param->ownerinfo != riinfo)
+    {
+        riinfo->param = (RI_ConstraintParam *)
+            MemoryContextAlloc(TopMemoryContext, sizeof(RI_ConstraintParam));
+        riinfo->param->ownerinfo = riinfo;
+    }
+
     /*
      * Fetch the pg_constraint row so we can fill in the entry.
      */
@@ -2051,22 +2091,23 @@ ri_LoadConstraintInfo(Oid constraintOid)
 
     /* And extract data */
     Assert(riinfo->constraint_id == constraintOid);
+    riinfo->param->query_key = get_ri_constraint_root(riinfo->constraint_id);
     riinfo->oidHashValue = GetSysCacheHashValue1(CONSTROID,
                                                  ObjectIdGetDatum(constraintOid));
     memcpy(&riinfo->conname, &conForm->conname, sizeof(NameData));
-    riinfo->pk_relid = conForm->confrelid;
+    riinfo->param->pk_relid = conForm->confrelid;
     riinfo->fk_relid = conForm->conrelid;
-    riinfo->confupdtype = conForm->confupdtype;
-    riinfo->confdeltype = conForm->confdeltype;
-    riinfo->confmatchtype = conForm->confmatchtype;
+    riinfo->param->confupdtype = conForm->confupdtype;
+    riinfo->param->confdeltype = conForm->confdeltype;
+    riinfo->param->confmatchtype = conForm->confmatchtype;
 
     DeconstructFkConstraintRow(tup,
-                               &riinfo->nkeys,
-                               riinfo->fk_attnums,
-                               riinfo->pk_attnums,
-                               riinfo->pf_eq_oprs,
-                               riinfo->pp_eq_oprs,
-                               riinfo->ff_eq_oprs);
+                               &riinfo->param->nkeys,
+                               riinfo->param->fk_attnums,
+                               riinfo->param->pk_attnums,
+                               riinfo->param->pf_eq_oprs,
+                               riinfo->param->pp_eq_oprs,
+                               riinfo->param->ff_eq_oprs);
 
     ReleaseSysCache(tup);
 
@@ -2227,7 +2268,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo,
                          vals, nulls);
         if (oldslot)
             ri_ExtractValues(source_rel, oldslot, riinfo, source_is_pk,
-                             vals + riinfo->nkeys, nulls + riinfo->nkeys);
+                             vals + riinfo->param->nkeys, nulls + riinfo->param->nkeys);
     }
     else
     {
@@ -2320,11 +2361,11 @@ ri_ExtractValues(Relation rel, TupleTableSlot *slot,
     bool        isnull;
 
     if (rel_is_pk)
-        attnums = riinfo->pk_attnums;
+        attnums = riinfo->param->pk_attnums;
     else
-        attnums = riinfo->fk_attnums;
+        attnums = riinfo->param->fk_attnums;
 
-    for (int i = 0; i < riinfo->nkeys; i++)
+    for (int i = 0; i < riinfo->param->nkeys; i++)
     {
         vals[i] = slot_getattr(slot, attnums[i], &isnull);
         nulls[i] = isnull ? 'n' : ' ';
@@ -2361,14 +2402,14 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
     onfk = (queryno == RI_PLAN_CHECK_LOOKUPPK);
     if (onfk)
     {
-        attnums = riinfo->fk_attnums;
+        attnums = riinfo->param->fk_attnums;
         rel_oid = fk_rel->rd_id;
         if (tupdesc == NULL)
             tupdesc = fk_rel->rd_att;
     }
     else
     {
-        attnums = riinfo->pk_attnums;
+        attnums = riinfo->param->pk_attnums;
         rel_oid = pk_rel->rd_id;
         if (tupdesc == NULL)
             tupdesc = pk_rel->rd_att;
@@ -2395,7 +2436,7 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
         if (aclresult != ACLCHECK_OK)
         {
             /* Try for column-level permissions */
-            for (int idx = 0; idx < riinfo->nkeys; idx++)
+            for (int idx = 0; idx < riinfo->param->nkeys; idx++)
             {
                 aclresult = pg_attribute_aclcheck(rel_oid, attnums[idx],
                                                   GetUserId(),
@@ -2418,7 +2459,7 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
         /* Get printable versions of the keys involved */
         initStringInfo(&key_names);
         initStringInfo(&key_values);
-        for (int idx = 0; idx < riinfo->nkeys; idx++)
+        for (int idx = 0; idx < riinfo->param->nkeys; idx++)
         {
             int            fnum = attnums[idx];
             Form_pg_attribute att = TupleDescAttr(tupdesc, fnum - 1);
@@ -2508,11 +2549,11 @@ ri_NullCheck(TupleDesc tupDesc,
     bool        nonenull = true;
 
     if (rel_is_pk)
-        attnums = riinfo->pk_attnums;
+        attnums = riinfo->param->pk_attnums;
     else
-        attnums = riinfo->fk_attnums;
+        attnums = riinfo->param->fk_attnums;
 
-    for (int i = 0; i < riinfo->nkeys; i++)
+    for (int i = 0; i < riinfo->param->nkeys; i++)
     {
         if (slot_attisnull(slot, attnums[i]))
             nonenull = false;
@@ -2667,12 +2708,12 @@ ri_KeysEqual(Relation rel, TupleTableSlot *oldslot, TupleTableSlot *newslot,
     const int16 *attnums;
 
     if (rel_is_pk)
-        attnums = riinfo->pk_attnums;
+        attnums = riinfo->param->pk_attnums;
     else
-        attnums = riinfo->fk_attnums;
+        attnums = riinfo->param->fk_attnums;
 
     /* XXX: could be worthwhile to fetch all necessary attrs at once */
-    for (int i = 0; i < riinfo->nkeys; i++)
+    for (int i = 0; i < riinfo->param->nkeys; i++)
     {
         Datum        oldvalue;
         Datum        newvalue;
@@ -2714,7 +2755,7 @@ ri_KeysEqual(Relation rel, TupleTableSlot *oldslot, TupleTableSlot *newslot,
              * operator.  Changes that compare equal will still satisfy the
              * constraint after the update.
              */
-            if (!ri_AttributesEqual(riinfo->ff_eq_oprs[i], RIAttType(rel, attnums[i]),
+            if (!ri_AttributesEqual(riinfo->param->ff_eq_oprs[i], RIAttType(rel, attnums[i]),
                                     oldvalue, newvalue))
                 return false;
         }
-- 
2.18.4

From ae7f7ab7df353044b8b2878253d1bdb8adbcd054 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Thu, 3 Dec 2020 16:51:07 +0900
Subject: [PATCH 2/2] share param part of riinfo

---
 src/backend/utils/adt/ri_triggers.c | 33 +++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 3f8407c311..080e8af1a5 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -110,6 +110,8 @@ typedef struct RI_ConstraintParam
     Oid            pf_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (PK = FK) */
     Oid            pp_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (PK = PK) */
     Oid            ff_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (FK = FK) */
+
+    /* This should follow the aboves, see ri_LoadConstraintInfo */
     struct RI_ConstraintInfo *ownerinfo; /* owner info of this param */
 } RI_ConstraintParam;
 
@@ -2111,6 +2113,37 @@ ri_LoadConstraintInfo(Oid constraintOid)
 
     ReleaseSysCache(tup);
 
+    if (OidIsValid(conForm->conparentid))
+    {
+        Oid pconid = conForm->conparentid;
+        const RI_ConstraintInfo *priinfo = ri_LoadConstraintInfo(pconid);
+        RI_ConstraintParam        *p = riinfo->param;
+        RI_ConstraintParam        *pp = priinfo->param;
+
+        /* share the same parameters if identical */
+        if (memcmp(p, pp, offsetof(RI_ConstraintParam, pk_attnums)) == 0)
+        {
+            int i;
+
+            for (i = 0 ; i < p->nkeys ; i++)
+            {
+                if (p->pk_attnums[i] != pp->pk_attnums[i] ||
+                    p->fk_attnums[i] != pp->fk_attnums[i] ||
+                    p->pf_eq_oprs[i] != pp->pf_eq_oprs[i] ||
+                    p->pp_eq_oprs[i] != pp->pp_eq_oprs[i] ||
+                    p->ff_eq_oprs[i] != pp->ff_eq_oprs[i])
+                    break;
+            }
+            if (i == p->nkeys)
+            {
+                /* this riinfo can share the parameters with parent */
+                Assert(p->ownerinfo == riinfo);
+                pfree(riinfo->param);
+                riinfo->param = pp;
+            }
+        }
+    }
+
     /*
      * For efficient processing of invalidation messages below, we keep a
      * doubly-linked list, and a count, of all currently valid entries.
-- 
2.18.4


pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: Multi Inserts in CREATE TABLE AS - revived patch
Next
From: legrand legrand
Date:
Subject: Re: pg_stat_statements oddity with track = all