Clean up possible memory leakage in nodeSubplan

diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index dbb824fe64cf71b4e79fe5c773b864f94cfd9cd9..65c42b61b3b23f100870f87569c30ff1af76f912 100644 (file)
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -6,7 +6,7 @@
  * Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/executor/nodeSubplan.c,v 1.16 1999/11/15 02:00:01 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/executor/nodeSubplan.c,v 1.17 1999/11/15 03:28:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -107,9 +107,18 @@ ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext, bool *isNull)
            if (found)
                elog(ERROR, "ExecSubPlan: more than one tuple returned by expression subselect");
            found = true;
-           /* XXX need to copy tuple in case pass by ref */
-           /* XXX need to ref-count the tuple to avoid mem leak! */
+           /*
+            * We need to copy the subplan's tuple in case the result is of
+            * pass-by-ref type --- our return value will point into this
+            * copied tuple!  Can't use the subplan's instance of the tuple
+            * since it won't still be valid after next ExecProcNode() call.
+            * node->curTuple keeps track of the copied tuple for eventual
+            * freeing.
+            */
            tup = heap_copytuple(tup);
+           if (node->curTuple)
+               pfree(node->curTuple);
+           node->curTuple = tup;
            result = heap_getattr(tup, col, tdesc, isNull);
            /* keep scanning subplan to make sure there's only one tuple */
            continue;
@@ -253,10 +262,13 @@ ExecInitSubPlan(SubPlan *node, EState *estate, Plan *parent)
        ExecCreateTupleTable(ExecCountSlotsNode(node->plan) + 10);
    sp_estate->es_snapshot = estate->es_snapshot;
 
+   node->shutdown = false;
+   node->curTuple = NULL;
+
    if (!ExecInitNode(node->plan, sp_estate, NULL))
        return false;
 
-   node->shutdown = true;
+   node->shutdown = true;      /* now we need to shutdown the subplan */
 
    /*
     * If this plan is un-correlated or undirect correlated one and want
@@ -332,13 +344,15 @@ ExecSetParamPlan(SubPlan *node)
        found = true;
 
        /*
-        * If this is uncorrelated subquery then its plan will be closed
-        * (see below) and this tuple will be free-ed - bad for not byval
-        * types... But is free-ing possible in the next ExecProcNode in
-        * this loop ? Who knows... Someday we'll keep track of saved
-        * tuples...
+        * We need to copy the subplan's tuple in case any of the params
+        * are pass-by-ref type --- the pointers stored in the param structs
+        * will point at this copied tuple!  node->curTuple keeps track
+        * of the copied tuple for eventual freeing.
         */
        tup = heap_copytuple(tup);
+       if (node->curTuple)
+           pfree(node->curTuple);
+       node->curTuple = tup;
 
        foreach(lst, node->setParam)
        {
@@ -387,13 +401,16 @@ ExecSetParamPlan(SubPlan *node)
 void
 ExecEndSubPlan(SubPlan *node)
 {
-
    if (node->shutdown)
    {
        ExecEndNode(node->plan, node->plan);
        node->shutdown = false;
    }
-
+   if (node->curTuple)
+   {
+       pfree(node->curTuple);
+       node->curTuple = NULL;
+   }
 }
 
 void

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index d353df58bdc088bbd8bfc67cd0e9afaa773acf8d..5f23a954b13a46325b88f3900c77021b25017571 100644 (file)
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.95 1999/11/15 02:00:01 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.96 1999/11/15 03:28:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -551,6 +551,10 @@ _copySubPlan(SubPlan *from)
    newnode->parParam = listCopy(from->parParam);
    Node_Copy(from, newnode, sublink);
 
+   /* do not copy execution state */
+   newnode->shutdown = false;
+   newnode->curTuple = NULL;
+
    return newnode;
 }
 

diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 2e3d67da6081635bcd89d9b93ab5f90af065b7d4..fccb9d316087711bf93b76893949d3d6cc53cc5d 100644 (file)
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.50 1999/10/07 04:23:04 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.51 1999/11/15 03:28:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -406,9 +406,13 @@ _equalIndexScan(IndexScan *a, IndexScan *b)
 static bool
 _equalSubPlan(SubPlan *a, SubPlan *b)
 {
+   /* should compare plans, but have to settle for comparing plan IDs */
    if (a->plan_id != b->plan_id)
        return false;
 
+   if (!equal(a->rtable, b->rtable))
+       return false;
+
    if (!equal(a->sublink, b->sublink))
        return false;
 

diff --git a/src/backend/nodes/freefuncs.c b/src/backend/nodes/freefuncs.c
index cad49be76bd46a26121c948bf801246fd1648a54..db09b6700bc3170723a648688b18a6236393fcf4 100644 (file)
--- a/src/backend/nodes/freefuncs.c
+++ b/src/backend/nodes/freefuncs.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/nodes/Attic/freefuncs.c,v 1.26 1999/08/21 03:48:57 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/nodes/Attic/freefuncs.c,v 1.27 1999/11/15 03:28:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -441,6 +441,9 @@ _freeSubPlan(SubPlan *node)
    freeList(node->parParam);
    freeObject(node->sublink);
 
+   if (node->curTuple)
+       pfree(node->curTuple);
+
    pfree(node);
 }
 

diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index e4dc5fde21018787368d10ae68b8337ef550b1eb..00e7025917da6842aabd6bcfee445b3059b07f63 100644 (file)
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: plannodes.h,v 1.32 1999/11/15 02:00:13 tgl Exp $
+ * $Id: plannodes.h,v 1.33 1999/11/15 03:28:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -352,13 +352,18 @@ typedef struct SubPlan
                                 * funcs for plan nodes... actually, we
                                 * could put *plan itself somewhere else
                                 * (TopPlan node ?)... */
-   List       *rtable;         /* range table */
+   List       *rtable;         /* range table for subselect */
+   /* setParam and parParam are lists of integers (param IDs) */
    List       *setParam;       /* non-correlated EXPR & EXISTS subqueries
                                 * have to set some Params for paren Plan */
    List       *parParam;       /* indices of corr. Vars from parent plan */
    SubLink    *sublink;        /* SubLink node from parser; holds info about
                                 * what to do with subselect's results */
-   bool        shutdown;       /* shutdown plan if TRUE */
+   /*
+    * Remaining fields are working state for executor; not used in planning
+    */
+   bool        shutdown;       /* TRUE = need to shutdown plan */
+   HeapTuple   curTuple;       /* copy of most recent tuple from subplan */
 } SubPlan;
 
 #endif  /* PLANNODES_H */