]> granicus.if.org Git - postgresql/commitdiff
nodeAppend tried to deal with multiple result relations, but apparently it never
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 10 Jun 2000 05:16:38 +0000 (05:16 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 10 Jun 2000 05:16:38 +0000 (05:16 +0000)
really worked.  Until now.

src/backend/executor/execMain.c
src/backend/executor/nodeAppend.c

index 7b17efc0b8f2396b647ef75a0b8534240d91f519..78b006de4217cb3877df2fb4633ca0530ddd36cf 100644 (file)
@@ -27,7 +27,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.115 2000/05/30 00:49:44 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.116 2000/06/10 05:16:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -50,8 +50,7 @@ static TupleDesc InitPlan(CmdType operation,
                 Query *parseTree,
                 Plan *plan,
                 EState *estate);
-static void EndPlan(Plan *plan,
-               EState *estate);
+static void EndPlan(Plan *plan, EState *estate);
 static TupleTableSlot *ExecutePlan(EState *estate, Plan *plan,
                        CmdType operation,
                        int offsetTuples,
@@ -916,15 +915,8 @@ static void
 EndPlan(Plan *plan, EState *estate)
 {
        RelationInfo *resultRelationInfo;
-       Relation        intoRelationDesc;
        List       *l;
 
-       /*
-        * get information from state
-        */
-       resultRelationInfo = estate->es_result_relation_info;
-       intoRelationDesc = estate->es_into_relation_descriptor;
-
        /*
         * shut down any PlanQual processing we were doing
         */
@@ -932,42 +924,34 @@ EndPlan(Plan *plan, EState *estate)
                EndEvalPlanQual(estate);
 
        /*
-        * shut down the query
+        * shut down the node-type-specific query processing
         */
        ExecEndNode(plan, plan);
 
        /*
         * destroy the executor "tuple" table.
         */
-       {
-               TupleTable      tupleTable = (TupleTable) estate->es_tupleTable;
-
-               ExecDropTupleTable(tupleTable, true);
-               estate->es_tupleTable = NULL;
-       }
+       ExecDropTupleTable(estate->es_tupleTable, true);
+       estate->es_tupleTable = NULL;
 
        /*
-        * close the result relations if necessary, but hold locks on them
-        * until xact commit
+        * close the result relation if necessary, but hold lock on it
+        * until xact commit.  NB: must not do this till after ExecEndNode(),
+        * see nodeAppend.c ...
         */
+       resultRelationInfo = estate->es_result_relation_info;
        if (resultRelationInfo != NULL)
        {
-               Relation        resultRelationDesc;
-
-               resultRelationDesc = resultRelationInfo->ri_RelationDesc;
-               heap_close(resultRelationDesc, NoLock);
-
-               /*
-                * close indices on the result relation
-                */
+               heap_close(resultRelationInfo->ri_RelationDesc, NoLock);
+               /* close indices on the result relation, too */
                ExecCloseIndices(resultRelationInfo);
        }
 
        /*
         * close the "into" relation if necessary, again keeping lock
         */
-       if (intoRelationDesc != NULL)
-               heap_close(intoRelationDesc, NoLock);
+       if (estate->es_into_relation_descriptor != NULL)
+               heap_close(estate->es_into_relation_descriptor, NoLock);
 
        /*
         * close any relations selected FOR UPDATE, again keeping locks
index 07759293a28e1eee6b2ec57e9c0d562aa6e8db31..32cf75df3e165b7c09245256df37c3b82700b23e 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/executor/nodeAppend.c,v 1.31 2000/06/09 01:44:09 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/executor/nodeAppend.c,v 1.32 2000/06/10 05:16:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -189,6 +189,9 @@ ExecInitAppend(Append *node, EState *estate, Plan *parent)
        Plan       *initNode;
        List       *junkList;
        RelationInfo *es_rri = estate->es_result_relation_info;
+       bool            inherited_result_rel = false;
+
+       CXT1_printf("ExecInitAppend: context is %d\n", CurrentMemoryContext);
 
        /* ----------------
         *      assign execution state to node and get information
@@ -201,8 +204,8 @@ ExecInitAppend(Append *node, EState *estate, Plan *parent)
        nplans = length(appendplans);
        rtable = node->inheritrtable;
 
-       CXT1_printf("ExecInitAppend: context is %d\n", CurrentMemoryContext);
        initialized = (bool *) palloc(nplans * sizeof(bool));
+       MemSet(initialized, 0, nplans * sizeof(bool));
 
        /* ----------------
         *      create new AppendState for our append node
@@ -231,7 +234,7 @@ ExecInitAppend(Append *node, EState *estate, Plan *parent)
 #define APPEND_NSLOTS 1
        /* ----------------
         *      append nodes still have Result slots, which hold pointers
-        *      to tuples, so we have to initialize them..
+        *      to tuples, so we have to initialize them.
         * ----------------
         */
        ExecInitResultTupleSlot(estate, &appendstate->cstate);
@@ -247,34 +250,60 @@ ExecInitAppend(Append *node, EState *estate, Plan *parent)
                (node->inheritrelid == es_rri->ri_RangeTableIndex))
        {
                List       *resultList = NIL;
+               Oid                     initial_reloid = RelationGetRelid(es_rri->ri_RelationDesc);
                List       *rtentryP;
 
+               inherited_result_rel = true;
+
                foreach(rtentryP, rtable)
                {
                        RangeTblEntry *rtentry = lfirst(rtentryP);
-                       Oid                     reloid;
+                       Oid                     reloid = rtentry->relid;
                        RelationInfo *rri;
 
-                       reloid = rtentry->relid;
-                       rri = makeNode(RelationInfo);
-                       rri->ri_RangeTableIndex = es_rri->ri_RangeTableIndex;
-                       rri->ri_RelationDesc = heap_open(reloid, RowExclusiveLock);
-                       rri->ri_NumIndices = 0;
-                       rri->ri_IndexRelationDescs = NULL;      /* index descs */
-                       rri->ri_IndexRelationInfo = NULL;       /* index key info */
-
-                       if (rri->ri_RelationDesc->rd_rel->relhasindex)
-                               ExecOpenIndices(reloid, rri);
-
-                       resultList = lcons(rri, resultList);
+                       /*
+                        * We must recycle the RelationInfo already opened by InitPlan()
+                        * for the parent rel, else we will leak the associated relcache
+                        * refcount. 
+                        */
+                       if (reloid == initial_reloid)
+                       {
+                               Assert(es_rri != NULL); /* check we didn't use it already */
+                               rri = es_rri;
+                               es_rri = NULL;
+                       }
+                       else
+                       {
+                               rri = makeNode(RelationInfo);
+                               rri->ri_RangeTableIndex = node->inheritrelid;
+                               rri->ri_RelationDesc = heap_open(reloid, RowExclusiveLock);
+                               rri->ri_NumIndices = 0;
+                               rri->ri_IndexRelationDescs = NULL;      /* index descs */
+                               rri->ri_IndexRelationInfo = NULL;       /* index key info */
+
+                               /*
+                                * XXX if the operation is a DELETE then we need not open
+                                * indices, but how to tell that here?
+                                */
+                               if (rri->ri_RelationDesc->rd_rel->relhasindex)
+                                       ExecOpenIndices(reloid, rri);
+                       }
+
+                       /*
+                        * NB: the as_result_relation_info_list must be in the same
+                        * order as the rtentry list otherwise update or delete on
+                        * inheritance hierarchies won't work.
+                        */
+                       resultList = lappend(resultList, rri);
                }
-        /*
-          The as_result_relation_info_list must be in the same
-          order as the rtentry list otherwise update or delete on
-          inheritance hierarchies won't work.
-        */
-               appendstate->as_result_relation_info_list = lreverse(resultList);
+
+               appendstate->as_result_relation_info_list = resultList;
+               /* Check that we recycled InitPlan()'s RelationInfo */
+               Assert(es_rri == NULL);
+               /* Just for paranoia's sake, clear link until we set it properly */
+               estate->es_result_relation_info = NULL;
        }
+
        /* ----------------
         *      call ExecInitNode on each of the plans in our list
         *      and save the results into the array "initialized"
@@ -304,8 +333,7 @@ ExecInitAppend(Append *node, EState *estate, Plan *parent)
                 *      the one that we're looking at the subclasses of
                 * ---------------
                 */
-               if ((es_rri != (RelationInfo *) NULL) &&
-                       (node->inheritrelid == es_rri->ri_RangeTableIndex))
+               if (inherited_result_rel)
                {
                        JunkFilter *j = ExecInitJunkFilter(initNode->targetlist,
                                                                                           ExecGetTupType(initNode));
@@ -361,7 +389,6 @@ ExecProcAppend(Append *node)
 {
        EState     *estate;
        AppendState *appendstate;
-
        int                     whichplan;
        List       *appendplans;
        Plan       *subnode;
@@ -376,7 +403,6 @@ ExecProcAppend(Append *node)
        appendstate = node->appendstate;
        estate = node->plan.state;
        direction = estate->es_direction;
-
        appendplans = node->appendplans;
        whichplan = appendstate->as_whichplan;
        result_slot = appendstate->cstate.cs_ResultTupleSlot;
@@ -448,19 +474,20 @@ ExecProcAppend(Append *node)
 void
 ExecEndAppend(Append *node)
 {
+       EState     *estate;
        AppendState *appendstate;
        int                     nplans;
        List       *appendplans;
        bool       *initialized;
        int                     i;
        List       *resultRelationInfoList;
-       RelationInfo *resultRelationInfo;
 
        /* ----------------
         *      get information from the node
         * ----------------
         */
        appendstate = node->appendstate;
+       estate = node->plan.state;
        appendplans = node->appendplans;
        nplans = appendstate->as_nplans;
        initialized = appendstate->as_initialized;
@@ -471,7 +498,7 @@ ExecEndAppend(Append *node)
         */
        for (i = 0; i < nplans; i++)
        {
-               if (initialized[i] == TRUE)
+               if (initialized[i])
                        ExecEndNode((Plan *) nth(i, appendplans), (Plan *) node);
        }
 
@@ -482,6 +509,7 @@ ExecEndAppend(Append *node)
        resultRelationInfoList = appendstate->as_result_relation_info_list;
        while (resultRelationInfoList != NIL)
        {
+               RelationInfo *resultRelationInfo;
                Relation        resultRelationDesc;
 
                resultRelationInfo = (RelationInfo *) lfirst(resultRelationInfoList);
@@ -490,8 +518,13 @@ ExecEndAppend(Append *node)
                pfree(resultRelationInfo);
                resultRelationInfoList = lnext(resultRelationInfoList);
        }
-       if (appendstate->as_result_relation_info_list)
-               pfree(appendstate->as_result_relation_info_list);
+       appendstate->as_result_relation_info_list = NIL;
+       /*
+        * This next step is critical to prevent EndPlan() from trying to close
+        * an already-closed-and-deleted RelationInfo --- es_result_relation_info
+        * is pointing at one of the nodes we just zapped above.
+        */
+       estate->es_result_relation_info = NULL;
 
        /*
         * XXX should free appendstate->as_rtentries  and