]> granicus.if.org Git - postgresql/commitdiff
Fix coredump problem in plpgsql's RETURN NEXT. When a SELECT INTO
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 21 Jan 2003 22:06:36 +0000 (22:06 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 21 Jan 2003 22:06:36 +0000 (22:06 +0000)
that's selecting into a RECORD variable returns zero rows, make it
assign an all-nulls row to the RECORD; this is consistent with what
happens when the SELECT INTO target is not a RECORD.  In support of
this, tweak the SPI code so that a valid tuple descriptor is returned
even when a SPI select returns no rows.

doc/src/sgml/spi.sgml
src/backend/executor/spi.c
src/backend/tcop/dest.c
src/include/access/printtup.h
src/pl/plpgsql/src/pl_exec.c

index e25c2a259cc0250f2b7cd3aca77f43559acce994..98916b24519b88c3602ea85e748c31d8930545c1 100644 (file)
@@ -1,5 +1,5 @@
 <!--
-$Header: /cvsroot/pgsql/doc/src/sgml/spi.sgml,v 1.23 2002/09/02 06:11:42 momjian Exp $
+$Header: /cvsroot/pgsql/doc/src/sgml/spi.sgml,v 1.23.2.1 2003/01/21 22:06:36 tgl Exp $
 -->
 
 <Chapter id="spi">
@@ -476,7 +476,7 @@ You may pass multiple queries in one string or query string may be
    The actual number of tuples for which the (last) query was executed is
    returned in the global variable SPI_processed (if not <ReturnValue>SPI_OK_UTILITY</ReturnValue>).
 
-   If <ReturnValue>SPI_OK_SELECT</ReturnValue> is returned and SPI_processed &gt; 0 then you may use global
+   If <ReturnValue>SPI_OK_SELECT</ReturnValue> is returned then you may use global
    pointer SPITupleTable *SPI_tuptable to access the result tuples.
 </Para>
 
@@ -517,7 +517,7 @@ You may pass multiple queries in one string or query string may be
 <TITLE>Structures
 </TITLE>
 <Para>
-   If <ReturnValue>SPI_OK_SELECT</ReturnValue> is returned and SPI_processed &gt; 0 then you may use the global
+   If <ReturnValue>SPI_OK_SELECT</ReturnValue> is returned then you may use the global
    pointer SPITupleTable *SPI_tuptable to access the selected tuples.
 </Para>
 
index fe26df84670b3f0b46b4a2c30818dea9e8a92e5e..41f04fbb5216f57aec1939c77dd7d9ef47db8336 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/executor/spi.c,v 1.75 2002/10/14 23:49:20 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/executor/spi.c,v 1.75.2.1 2003/01/21 22:06:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -881,18 +881,60 @@ SPI_cursor_close(Portal portal)
 
 /* =================== private functions =================== */
 
+/*
+ * spi_dest_setup
+ *             Initialize to receive tuples from Executor into SPITupleTable
+ *             of current SPI procedure
+ */
+void
+spi_dest_setup(DestReceiver *self, int operation,
+                          const char *portalName, TupleDesc typeinfo)
+{
+       SPITupleTable *tuptable;
+       MemoryContext oldcxt;
+       MemoryContext tuptabcxt;
+
+       /*
+        * When called by Executor _SPI_curid expected to be equal to
+        * _SPI_connected
+        */
+       if (_SPI_curid != _SPI_connected || _SPI_connected < 0)
+               elog(FATAL, "SPI: improper call to spi_dest_setup");
+       if (_SPI_current != &(_SPI_stack[_SPI_curid]))
+               elog(FATAL, "SPI: stack corrupted in spi_dest_setup");
+
+       if (_SPI_current->tuptable != NULL)
+               elog(FATAL, "SPI: improper call to spi_dest_setup");
+
+       oldcxt = _SPI_procmem();        /* switch to procedure memory context */
+
+       tuptabcxt = AllocSetContextCreate(CurrentMemoryContext,
+                                                                         "SPI TupTable",
+                                                                         ALLOCSET_DEFAULT_MINSIZE,
+                                                                         ALLOCSET_DEFAULT_INITSIZE,
+                                                                         ALLOCSET_DEFAULT_MAXSIZE);
+       MemoryContextSwitchTo(tuptabcxt);
+
+       _SPI_current->tuptable = tuptable = (SPITupleTable *)
+               palloc(sizeof(SPITupleTable));
+       tuptable->tuptabcxt = tuptabcxt;
+       tuptable->alloced = tuptable->free = 128;
+       tuptable->vals = (HeapTuple *) palloc(tuptable->alloced * sizeof(HeapTuple));
+       tuptable->tupdesc = CreateTupleDescCopy(typeinfo);
+
+       MemoryContextSwitchTo(oldcxt);
+}
+
 /*
  * spi_printtup
  *             store tuple retrieved by Executor into SPITupleTable
  *             of current SPI procedure
- *
  */
 void
 spi_printtup(HeapTuple tuple, TupleDesc tupdesc, DestReceiver *self)
 {
        SPITupleTable *tuptable;
        MemoryContext oldcxt;
-       MemoryContext tuptabcxt;
 
        /*
         * When called by Executor _SPI_curid expected to be equal to
@@ -903,43 +945,24 @@ spi_printtup(HeapTuple tuple, TupleDesc tupdesc, DestReceiver *self)
        if (_SPI_current != &(_SPI_stack[_SPI_curid]))
                elog(FATAL, "SPI: stack corrupted in spi_printtup");
 
-       oldcxt = _SPI_procmem();        /* switch to procedure memory context */
-
        tuptable = _SPI_current->tuptable;
        if (tuptable == NULL)
-       {
-               tuptabcxt = AllocSetContextCreate(CurrentMemoryContext,
-                                                                                 "SPI TupTable",
-                                                                                 ALLOCSET_DEFAULT_MINSIZE,
-                                                                                 ALLOCSET_DEFAULT_INITSIZE,
-                                                                                 ALLOCSET_DEFAULT_MAXSIZE);
-               MemoryContextSwitchTo(tuptabcxt);
-
-               _SPI_current->tuptable = tuptable = (SPITupleTable *)
-                       palloc(sizeof(SPITupleTable));
-               tuptable->tuptabcxt = tuptabcxt;
-               tuptable->alloced = tuptable->free = 128;
-               tuptable->vals = (HeapTuple *) palloc(tuptable->alloced * sizeof(HeapTuple));
-               tuptable->tupdesc = CreateTupleDescCopy(tupdesc);
-       }
-       else
-       {
-               MemoryContextSwitchTo(tuptable->tuptabcxt);
+               elog(FATAL, "SPI: improper call to spi_printtup");
 
-               if (tuptable->free == 0)
-               {
-                       tuptable->free = 256;
-                       tuptable->alloced += tuptable->free;
-                       tuptable->vals = (HeapTuple *) repalloc(tuptable->vals,
+       oldcxt = MemoryContextSwitchTo(tuptable->tuptabcxt);
+
+       if (tuptable->free == 0)
+       {
+               tuptable->free = 256;
+               tuptable->alloced += tuptable->free;
+               tuptable->vals = (HeapTuple *) repalloc(tuptable->vals,
                                                                  tuptable->alloced * sizeof(HeapTuple));
-               }
        }
 
        tuptable->vals[tuptable->alloced - tuptable->free] = heap_copytuple(tuple);
        (tuptable->free)--;
 
        MemoryContextSwitchTo(oldcxt);
-       return;
 }
 
 /*
@@ -1457,19 +1480,10 @@ _SPI_checktuples(void)
        SPITupleTable *tuptable = _SPI_current->tuptable;
        bool            failed = false;
 
-       if (processed == 0)
-       {
-               if (tuptable != NULL)
-                       failed = true;
-       }
-       else
-       {
-               /* some tuples were processed */
-               if (tuptable == NULL)   /* spi_printtup was not called */
-                       failed = true;
-               else if (processed != (tuptable->alloced - tuptable->free))
-                       failed = true;
-       }
+       if (tuptable == NULL)   /* spi_dest_setup was not called */
+               failed = true;
+       else if (processed != (tuptable->alloced - tuptable->free))
+               failed = true;
 
        return failed;
 }
index 2face9fc1010e5ed0fffa833db905a77380632f8..058bc1676aea8974927d65070f2edaf39f9c0af5 100644 (file)
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/tcop/dest.c,v 1.49 2002/06/20 20:29:36 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/tcop/dest.c,v 1.49.2.1 2003/01/21 22:06:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -64,7 +64,7 @@ static DestReceiver debugtupDR = {
        debugtup, debugSetup, donothingCleanup
 };
 static DestReceiver spi_printtupDR = {
-       spi_printtup, donothingSetup, donothingCleanup
+       spi_printtup, spi_dest_setup, donothingCleanup
 };
 
 /* ----------------
index e8c4fa352fc75707aceb2728165f3bf28bf6108e..04dd4020653adc5d09c58c546a04471708a94978 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: printtup.h,v 1.22 2002/09/04 20:31:37 momjian Exp $
+ * $Id: printtup.h,v 1.22.2.1 2003/01/21 22:06:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -23,7 +23,9 @@ extern void debugSetup(DestReceiver *self, int operation,
 extern void debugtup(HeapTuple tuple, TupleDesc typeinfo,
                 DestReceiver *self);
 
-/* XXX this one is really in executor/spi.c */
+/* XXX these are really in executor/spi.c */
+extern void spi_dest_setup(DestReceiver *self, int operation,
+                  const char *portalName, TupleDesc typeinfo);
 extern void spi_printtup(HeapTuple tuple, TupleDesc tupdesc,
                         DestReceiver *self);
 
index 9adf2d7a2e9de13888791943a7207b985baa2949..fd4f2e2ae14d7c382533243995fcece7187a0a93 100644 (file)
@@ -3,7 +3,7 @@
  *                       procedural language
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.65 2002/10/19 22:10:58 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.65.2.1 2003/01/21 22:06:36 tgl Exp $
  *
  *       This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -1337,15 +1337,15 @@ exec_stmt_fors(PLpgSQL_execstate * estate, PLpgSQL_stmt_fors * stmt)
        exec_run_select(estate, stmt->query, 0, &portal);
 
        SPI_cursor_fetch(portal, true, 10);
-       n = SPI_processed;
        tuptab = SPI_tuptable;
+       n = SPI_processed;
 
        /*
         * If the query didn't return any rows, set the target to NULL and
         * return with FOUND = false.
         */
        if (n == 0)
-               exec_move_row(estate, rec, row, NULL, NULL);
+               exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
        else
                found = true;                   /* processed at least one tuple */
 
@@ -1459,6 +1459,7 @@ exec_stmt_select(PLpgSQL_execstate * estate, PLpgSQL_stmt_select * stmt)
         * Run the query
         */
        exec_run_select(estate, stmt->query, 1, NULL);
+       tuptab = estate->eval_tuptable;
        n = estate->eval_processed;
 
        /*
@@ -1467,7 +1468,7 @@ exec_stmt_select(PLpgSQL_execstate * estate, PLpgSQL_stmt_select * stmt)
         */
        if (n == 0)
        {
-               exec_move_row(estate, rec, row, NULL, NULL);
+               exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
                exec_eval_cleanup(estate);
                return PLPGSQL_RC_OK;
        }
@@ -1475,7 +1476,6 @@ exec_stmt_select(PLpgSQL_execstate * estate, PLpgSQL_stmt_select * stmt)
        /*
         * Put the result into the target and set found to true
         */
-       tuptab = estate->eval_tuptable;
        exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
        exec_set_found(estate, true);
 
@@ -1609,6 +1609,8 @@ exec_stmt_return_next(PLpgSQL_execstate * estate,
        {
                PLpgSQL_rec *rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
 
+               if (!HeapTupleIsValid(rec->tup))
+                       elog(ERROR, "record \"%s\" is unassigned yet", rec->refname);
                if (!compatible_tupdesc(tupdesc, rec->tupdesc))
                        elog(ERROR, "Wrong record type supplied in RETURN NEXT");
                tuple = rec->tup;
@@ -2351,15 +2353,15 @@ exec_stmt_dynfors(PLpgSQL_execstate * estate, PLpgSQL_stmt_dynfors * stmt)
         * Fetch the initial 10 tuples
         */
        SPI_cursor_fetch(portal, true, 10);
-       n = SPI_processed;
        tuptab = SPI_tuptable;
+       n = SPI_processed;
 
        /*
         * If the query didn't return any rows, set the target to NULL and
         * return with FOUND = false.
         */
        if (n == 0)
-               exec_move_row(estate, rec, row, NULL, NULL);
+               exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
        else
                found = true;
 
@@ -2758,8 +2760,8 @@ exec_stmt_fetch(PLpgSQL_execstate * estate, PLpgSQL_stmt_fetch * stmt)
         * ----------
         */
        SPI_cursor_fetch(portal, true, 1);
-       n = SPI_processed;
        tuptab = SPI_tuptable;
+       n = SPI_processed;
 
        /* ----------
         * If the FETCH didn't return a row, set the target
@@ -2768,7 +2770,7 @@ exec_stmt_fetch(PLpgSQL_execstate * estate, PLpgSQL_stmt_fetch * stmt)
         */
        if (n == 0)
        {
-               exec_move_row(estate, rec, row, NULL, NULL);
+               exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
                return PLPGSQL_RC_OK;
        }
 
@@ -3339,8 +3341,8 @@ exec_move_row(PLpgSQL_execstate * estate,
                          HeapTuple tup, TupleDesc tupdesc)
 {
        /*
-        * Record is simple - just put the tuple and its descriptor into the
-        * record
+        * Record is simple - just copy the tuple and its descriptor into the
+        * record variable
         */
        if (rec != NULL)
        {
@@ -3358,13 +3360,34 @@ exec_move_row(PLpgSQL_execstate * estate,
                if (HeapTupleIsValid(tup))
                {
                        rec->tup = heap_copytuple(tup);
-                       rec->tupdesc = CreateTupleDescCopy(tupdesc);
                        rec->freetup = true;
-                       rec->freetupdesc = true;
+               }
+               else if (tupdesc)
+               {
+                       /* If we have a tupdesc but no data, form an all-nulls tuple */
+                       char            *nulls;
+
+                       /* +1 to avoid possible palloc(0) if no attributes */
+                       nulls = (char *) palloc(tupdesc->natts * sizeof(char) + 1);
+                       memset(nulls, 'n', tupdesc->natts * sizeof(char));
+
+                       rec->tup = heap_formtuple(tupdesc, NULL, nulls);
+                       rec->freetup = true;
+
+                       pfree(nulls);
                }
                else
                {
                        rec->tup = NULL;
+               }
+
+               if (tupdesc)
+               {
+                       rec->tupdesc = CreateTupleDescCopy(tupdesc);
+                       rec->freetupdesc = true;
+               }
+               else
+               {
                        rec->tupdesc = NULL;
                }
 
@@ -3381,6 +3404,9 @@ exec_move_row(PLpgSQL_execstate * estate,
         * table, or it might have fewer if the table has had columns added by
         * ALTER TABLE. Ignore extra columns and assume NULL for missing
         * columns, the same as heap_getattr would do.
+        *
+        * If we have no tuple data at all, we'll assign NULL to all columns
+        * of the row variable.
         */
        if (row != NULL)
        {