]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Previous fix for temporary file management broke returning a set from
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 29 Dec 2009 17:41:26 +0000 (17:41 +0000)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 29 Dec 2009 17:41:26 +0000 (17:41 +0000)
PL/pgSQL function within an exception handler. Make sure we use the right
resource owner when we create the tuplestore to hold returned tuples.

Simplify tuplestore API so that the caller doesn't need to be in the right
memory context when calling tuplestore_put* functions. tuplestore.c
automatically switches to the memory context used when the tuplestore was
created. Tuplesort was already modified like this earlier. This patch also
removes the now useless MemoryContextSwitch calls from callers.

Report by Aleksei on pgsql-bugs on Dec 22 2009. Backpatch to 8.1, like
the previous patch that broke this.

contrib/tablefunc/tablefunc.c
contrib/xml2/xpath.c
src/backend/commands/prepare.c
src/backend/executor/execQual.c
src/backend/executor/tstoreReceiver.c
src/backend/utils/mmgr/portalmem.c
src/backend/utils/sort/tuplestore.c
src/pl/plperl/plperl.c
src/pl/plpgsql/src/pl_exec.c
src/pl/plpgsql/src/plpgsql.h

index 26886b2af9dd328eade3052d1001bb6c90f06125..10fe9f00e59628eedcef15625afe42ee70d6de24 100644 (file)
@@ -853,7 +853,6 @@ get_crosstab_tuplestore(char *sql,
        HeapTuple       tuple;
        int                     ret;
        int                     proc;
-       MemoryContext SPIcontext;
 
        /* initialize our tuplestore */
        tupstore = tuplestore_begin_heap(true, false, work_mem);
@@ -953,10 +952,7 @@ get_crosstab_tuplestore(char *sql,
                                        /* rowid changed, flush the previous output row */
                                        tuple = BuildTupleFromCStrings(attinmeta, values);
 
-                                       /* switch to appropriate context while storing the tuple */
-                                       SPIcontext = MemoryContextSwitchTo(per_query_ctx);
                                        tuplestore_puttuple(tupstore, tuple);
-                                       MemoryContextSwitchTo(SPIcontext);
 
                                        for (j = 0; j < result_ncols; j++)
                                                xpfree(values[j]);
@@ -989,10 +985,7 @@ get_crosstab_tuplestore(char *sql,
                /* flush the last output row */
                tuple = BuildTupleFromCStrings(attinmeta, values);
 
-               /* switch to appropriate context while storing the tuple */
-               SPIcontext = MemoryContextSwitchTo(per_query_ctx);
                tuplestore_puttuple(tupstore, tuple);
-               MemoryContextSwitchTo(SPIcontext);
        }
 
        if (SPI_finish() != SPI_OK_FINISH)
@@ -1274,7 +1267,6 @@ build_tuplestore_recursively(char *key_fld,
                                                         Tuplestorestate *tupstore)
 {
        TupleDesc       tupdesc = attinmeta->tupdesc;
-       MemoryContext oldcontext;
        int                     ret;
        int                     proc;
        int                     serial_column;
@@ -1352,15 +1344,9 @@ build_tuplestore_recursively(char *key_fld,
                /* construct the tuple */
                tuple = BuildTupleFromCStrings(attinmeta, values);
 
-               /* switch to long lived context while storing the tuple */
-               oldcontext = MemoryContextSwitchTo(per_query_ctx);
-
                /* now store it */
                tuplestore_puttuple(tupstore, tuple);
 
-               /* now reset the context */
-               MemoryContextSwitchTo(oldcontext);
-
                /* increment level */
                level++;
        }
@@ -1449,15 +1435,9 @@ build_tuplestore_recursively(char *key_fld,
                        xpfree(current_key);
                        xpfree(current_key_parent);
 
-                       /* switch to long lived context while storing the tuple */
-                       oldcontext = MemoryContextSwitchTo(per_query_ctx);
-
                        /* store the tuple for later use */
                        tuplestore_puttuple(tupstore, tuple);
 
-                       /* now reset the context */
-                       MemoryContextSwitchTo(oldcontext);
-
                        heap_freetuple(tuple);
 
                        /* recurse using current_key_parent as the new start_with */
index e0b3f4155bc267b0ce288c84201305e64957bbda..548d61249d194b7dea38f203e329076c2091837b 100644 (file)
@@ -834,9 +834,7 @@ xpath_table(PG_FUNCTION_ARGS)
                {
                        /* not well-formed, so output all-NULL tuple */
                        ret_tuple = BuildTupleFromCStrings(attinmeta, values);
-                       oldcontext = MemoryContextSwitchTo(per_query_ctx);
                        tuplestore_puttuple(tupstore, ret_tuple);
-                       MemoryContextSwitchTo(oldcontext);
                        heap_freetuple(ret_tuple);
                }
                else
@@ -910,9 +908,7 @@ xpath_table(PG_FUNCTION_ARGS)
                                if (had_values)
                                {
                                        ret_tuple = BuildTupleFromCStrings(attinmeta, values);
-                                       oldcontext = MemoryContextSwitchTo(per_query_ctx);
                                        tuplestore_puttuple(tupstore, ret_tuple);
-                                       MemoryContextSwitchTo(oldcontext);
                                        heap_freetuple(ret_tuple);
                                }
 
index d5ec457732aec0adf0e8b7c80eb8d9c50f711546..03035cf7b476ba753f67b91e2c891bf3b4bd58e8 100644 (file)
@@ -10,7 +10,7 @@
  * Copyright (c) 2002-2006, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.66.2.1 2007/04/26 23:24:56 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.66.2.2 2009/12/29 17:41:25 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -717,6 +717,9 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
         */
        tupstore = tuplestore_begin_heap(true, false, work_mem);
 
+       /* generate junk in short-term context */
+       MemoryContextSwitchTo(oldcontext);
+
        /* hash table might be uninitialized */
        if (prepared_queries)
        {
@@ -730,9 +733,6 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
                        Datum           values[5];
                        bool            nulls[5];
 
-                       /* generate junk in short-term context */
-                       MemoryContextSwitchTo(oldcontext);
-
                        MemSet(nulls, 0, sizeof(nulls));
 
                        values[0] = DirectFunctionCall1(textin,
@@ -749,9 +749,6 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
                        values[4] = BoolGetDatum(prep_stmt->from_sql);
 
                        tuple = heap_form_tuple(tupdesc, values, nulls);
-
-                       /* switch to appropriate context while storing the tuple */
-                       MemoryContextSwitchTo(per_query_ctx);
                        tuplestore_puttuple(tupstore, tuple);
                }
        }
@@ -759,8 +756,6 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
        /* clean up and return the tuplestore */
        tuplestore_donestoring(tupstore);
 
-       MemoryContextSwitchTo(oldcontext);
-
        rsinfo->returnMode = SFRM_Materialize;
        rsinfo->setResult = tupstore;
        rsinfo->setDesc = tupdesc;
index f48e2ad62bfc48024a09b6c83bc7c1e822ad972b..fed6176cbe5ec66e5af59b4ed09b3eaa12bf68da 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.199.2.3 2007/08/31 18:33:47 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.199.2.4 2009/12/29 17:41:25 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1645,9 +1645,7 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
                                tuple = heap_form_tuple(tupdesc, &result, &fcinfo.isnull);
                        }
 
-                       oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
                        tuplestore_puttuple(tupstore, tuple);
-                       MemoryContextSwitchTo(oldcontext);
 
                        /*
                         * Are we done?
@@ -1699,6 +1697,7 @@ no_function_result:
                        memset(nullflags, true, natts * sizeof(bool));
                        tuple = heap_form_tuple(expectedDesc, nulldatums, nullflags);
                        MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
+
                        tuplestore_puttuple(tupstore, tuple);
                }
        }
index a1163bf86140f1fd7b13dd019b50bc116668c1a5..23815f883e7631815bf8780cbe99e97bc7c0cb98 100644 (file)
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/tstoreReceiver.c,v 1.17.2.1 2008/12/01 17:06:35 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/tstoreReceiver.c,v 1.17.2.2 2009/12/29 17:41:25 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -95,11 +95,8 @@ static void
 tstoreReceiveSlot_notoast(TupleTableSlot *slot, DestReceiver *self)
 {
        TStoreState *myState = (TStoreState *) self;
-       MemoryContext oldcxt = MemoryContextSwitchTo(myState->cxt);
 
        tuplestore_puttupleslot(myState->tstore, slot);
-
-       MemoryContextSwitchTo(oldcxt);
 }
 
 /*
index 3e3505caf05dbf94af722cc1526d20683298df0d..57ff508a91ef5ac5f7cfa4d10ce2618229ba6017 100644 (file)
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.97.2.1 2007/04/26 23:24:57 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.97.2.2 2009/12/29 17:41:25 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -854,6 +854,9 @@ pg_cursor(PG_FUNCTION_ARGS)
         */
        tupstore = tuplestore_begin_heap(true, false, work_mem);
 
+       /* generate junk in short-term context */
+       MemoryContextSwitchTo(oldcontext);
+
        hash_seq_init(&hash_seq, PortalHashTable);
        while ((hentry = hash_seq_search(&hash_seq)) != NULL)
        {
@@ -866,9 +869,6 @@ pg_cursor(PG_FUNCTION_ARGS)
                if (!portal->visible)
                        continue;
 
-               /* generate junk in short-term context */
-               MemoryContextSwitchTo(oldcontext);
-
                MemSet(nulls, 0, sizeof(nulls));
 
                values[0] = DirectFunctionCall1(textin, CStringGetDatum(portal->name));
@@ -884,16 +884,12 @@ pg_cursor(PG_FUNCTION_ARGS)
 
                tuple = heap_form_tuple(tupdesc, values, nulls);
 
-               /* switch to appropriate context while storing the tuple */
-               MemoryContextSwitchTo(per_query_ctx);
                tuplestore_puttuple(tupstore, tuple);
        }
 
        /* clean up and return the tuplestore */
        tuplestore_donestoring(tupstore);
 
-       MemoryContextSwitchTo(oldcontext);
-
        rsinfo->returnMode = SFRM_Materialize;
        rsinfo->setResult = tupstore;
        rsinfo->setDesc = tupdesc;
index ccc6580c1c7d9f404a711612a59340e54155005d..bc2178d474fa264222272d77573d2da45080f06f 100644 (file)
@@ -36,7 +36,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/sort/tuplestore.c,v 1.29.2.1 2007/08/02 17:48:54 neilc Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/sort/tuplestore.c,v 1.29.2.2 2009/12/29 17:41:25 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -46,6 +46,7 @@
 #include "access/heapam.h"
 #include "storage/buffile.h"
 #include "utils/memutils.h"
+#include "utils/resowner.h"
 #include "utils/tuplestore.h"
 
 
@@ -70,6 +71,8 @@ struct Tuplestorestate
        bool            interXact;              /* keep open through transactions? */
        long            availMem;               /* remaining memory available, in bytes */
        BufFile    *myfile;                     /* underlying file, or NULL if none */
+       MemoryContext context;          /* memory context for holding tuples */
+       ResourceOwner resowner;         /* resowner for holding temp files */
 
        /*
         * These function pointers decouple the routines that must know what kind
@@ -220,6 +223,8 @@ tuplestore_begin_common(bool randomAccess, bool interXact, int maxKBytes)
        state->interXact = interXact;
        state->availMem = maxKBytes * 1024L;
        state->myfile = NULL;
+       state->context = CurrentMemoryContext;
+       state->resowner = CurrentResourceOwner;
 
        state->memtupcount = 0;
        state->memtupsize = 1024;       /* initial guess */
@@ -245,9 +250,9 @@ tuplestore_begin_common(bool randomAccess, bool interXact, int maxKBytes)
  *
  * interXact: if true, the files used for on-disk storage persist beyond the
  * end of the current transaction.     NOTE: It's the caller's responsibility to
- * create such a tuplestore in a memory context that will also survive
- * transaction boundaries, and to ensure the tuplestore is closed when it's
- * no longer wanted.
+ * create such a tuplestore in a memory context and resource owner that will
+ * also survive transaction boundaries, and to ensure the tuplestore is closed
+ * when it's no longer wanted.
  *
  * maxKBytes: how much data to store in memory (any data beyond this
  * amount is paged to disk).  When in doubt, use work_mem.
@@ -315,6 +320,7 @@ tuplestore_puttupleslot(Tuplestorestate *state,
                                                TupleTableSlot *slot)
 {
        MinimalTuple tuple;
+       MemoryContext oldcxt = MemoryContextSwitchTo(state->context);
 
        /*
         * Form a MinimalTuple in working memory
@@ -323,6 +329,8 @@ tuplestore_puttupleslot(Tuplestorestate *state,
        USEMEM(state, GetMemoryChunkSpace(tuple));
 
        tuplestore_puttuple_common(state, (void *) tuple);
+
+       MemoryContextSwitchTo(oldcxt);
 }
 
 /*
@@ -334,17 +342,23 @@ tuplestore_puttupleslot(Tuplestorestate *state,
 void
 tuplestore_puttuple(Tuplestorestate *state, HeapTuple tuple)
 {
+       MemoryContext oldcxt = MemoryContextSwitchTo(state->context);
+
        /*
         * Copy the tuple.      (Must do this even in WRITEFILE case.)
         */
        tuple = COPYTUP(state, tuple);
 
        tuplestore_puttuple_common(state, (void *) tuple);
+
+       MemoryContextSwitchTo(oldcxt);
 }
 
 static void
 tuplestore_puttuple_common(Tuplestorestate *state, void *tuple)
 {
+       ResourceOwner oldowner;
+
        switch (state->status)
        {
                case TSS_INMEM:
@@ -389,7 +403,13 @@ tuplestore_puttuple_common(Tuplestorestate *state, void *tuple)
                        /*
                         * Nope; time to switch to tape-based operation.
                         */
-                       state->myfile = BufFileCreateTemp(state->interXact);
+
+                       /* associate the file with the store's resource owner */
+                       oldowner = CurrentResourceOwner;
+                       CurrentResourceOwner = state->resowner;
+                       state->myfile = BufFileCreateTemp(state->interXact);
+                       CurrentResourceOwner = oldowner;
+
                        state->status = TSS_WRITEFILE;
                        dumptuples(state);
                        break;
index 4fed6ee8b5197a714b1f96f7aa6e5807617940d1..5bea27ee9279c918a2282a10396a06e3a5be8f08 100644 (file)
@@ -1,7 +1,7 @@
 /**********************************************************************
  * plperl.c - perl as a procedural language for PostgreSQL
  *
- *       $PostgreSQL: pgsql/src/pl/plperl/plperl.c,v 1.123.2.9 2009/11/29 21:02:34 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plperl/plperl.c,v 1.123.2.10 2009/12/29 17:41:25 heikki Exp $
  *
  **********************************************************************/
 
@@ -1984,11 +1984,9 @@ plperl_return_next(SV *sv)
                tuple = heap_form_tuple(current_call_data->ret_tdesc, &ret, &isNull);
        }
 
-       /* Make sure to store the tuple in a long-lived memory context */
-       MemoryContextSwitchTo(rsi->econtext->ecxt_per_query_memory);
        tuplestore_puttuple(current_call_data->tuple_store, tuple);
-       MemoryContextSwitchTo(old_cxt);
 
+       MemoryContextSwitchTo(old_cxt);
        MemoryContextReset(current_call_data->tmp_cxt);
 }
 
index 3accfb8130d72393a29067e296fd47e8965b1c40..fd62d73174e61585fe27eff872647b621d000bee 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.180.2.10 2009/04/02 01:16:25 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.180.2.11 2009/12/29 17:41:25 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2058,11 +2058,7 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
 
        if (HeapTupleIsValid(tuple))
        {
-               MemoryContext oldcxt;
-
-               oldcxt = MemoryContextSwitchTo(estate->tuple_store_cxt);
                tuplestore_puttuple(estate->tuple_store, tuple);
-               MemoryContextSwitchTo(oldcxt);
 
                if (free_tuple)
                        heap_freetuple(tuple);
@@ -2076,6 +2072,7 @@ exec_init_tuple_store(PLpgSQL_execstate *estate)
 {
        ReturnSetInfo *rsi = estate->rsi;
        MemoryContext oldcxt;
+       ResourceOwner oldowner;
 
        /*
         * Check caller can handle a set result in the way we want
@@ -2087,10 +2084,20 @@ exec_init_tuple_store(PLpgSQL_execstate *estate)
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("set-valued function called in context that cannot accept a set")));
 
-       estate->tuple_store_cxt = rsi->econtext->ecxt_per_query_memory;
-
+       /*
+        * Switch to the right memory context and resource owner for storing
+        * the tuplestore for return set. If we're within a subtransaction opened
+        * for an exception-block, for example, we must still create the
+        * tuplestore in the resource owner that was active when this function was
+        * entered, and not in the subtransaction resource owner.
+        */
        oldcxt = MemoryContextSwitchTo(estate->tuple_store_cxt);
+       oldowner = CurrentResourceOwner;
+       CurrentResourceOwner = estate->tuple_store_owner;
+
        estate->tuple_store = tuplestore_begin_heap(true, false, work_mem);
+
+       CurrentResourceOwner = oldowner;
        MemoryContextSwitchTo(oldcxt);
 
        estate->rettupdesc = rsi->expectedDesc;
@@ -2202,7 +2209,16 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
        estate->exitlabel = NULL;
 
        estate->tuple_store = NULL;
-       estate->tuple_store_cxt = NULL;
+       if (rsi)
+       {
+               estate->tuple_store_cxt = rsi->econtext->ecxt_per_query_memory;
+               estate->tuple_store_owner = CurrentResourceOwner;
+       }
+       else
+       {
+               estate->tuple_store_cxt = NULL;
+               estate->tuple_store_owner = NULL;
+       }
        estate->rsi = rsi;
 
        estate->trig_nargs = 0;
index c301d5c22db5141e0cf26f350ae9bcc690caf77a..c48a66a1912c5b8ab779ce866298e2043037fa3d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.81.2.3 2008/10/09 16:35:19 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.81.2.4 2009/12/29 17:41:26 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -607,6 +607,7 @@ typedef struct
 
        Tuplestorestate *tuple_store;           /* SRFs accumulate results here */
        MemoryContext tuple_store_cxt;
+       ResourceOwner tuple_store_owner;
        ReturnSetInfo *rsi;
 
        int                     trig_nargs;