Fix coredump problem in plpgsql's RETURN NEXT. When a SELECT INTO

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.

4 files changed, 102 insertions, 60 deletions

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index abca504f2ae..3df2426473a 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/spi.c,v 1.83 2002/12/30 22:10:54 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/spi.c,v 1.84 2003/01/21 22:06:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -874,17 +874,59 @@ 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
@@ -895,43 +937,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;
 }
 
 /*
@@ -1448,19 +1471,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;
 }
diff --git a/src/backend/tcop/dest.c b/src/backend/tcop/dest.c
index 2face9fc101..516af21a466 100644
--- a/src/backend/tcop/dest.c
+++ b/src/backend/tcop/dest.c
@@ -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.50 2003/01/21 22:06:12 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
 };
 
 /* ----------------
diff --git a/src/include/access/printtup.h b/src/include/access/printtup.h
index e8c4fa352fc..728b0c89906 100644
--- a/src/include/access/printtup.h
+++ b/src/include/access/printtup.h
@@ -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.23 2003/01/21 22:06:12 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);
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 8e0e1ffda31..c9fd9418b44 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3,7 +3,7 @@
  *			  procedural language
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.76 2002/12/17 15:45:01 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.77 2003/01/21 22:06:12 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -1356,15 +1356,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 */
 
@@ -1478,6 +1478,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;
 
 	/*
@@ -1486,7 +1487,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;
 	}
@@ -1494,7 +1495,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);
 
@@ -1627,6 +1627,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;
@@ -2369,15 +2371,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;
 
@@ -2776,8 +2778,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
@@ -2786,7 +2788,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;
 	}
 
@@ -3353,8 +3355,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)
 	{
@@ -3372,13 +3374,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;
 		}
 
@@ -3395,6 +3418,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)
 	{