[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#632028: postgresql 8.4.8 regression - failure to handle char(4) = bpchar



On Wed, Jun 29, 2011 at 10:32:27AM +0200, Josip Rodin wrote:
> Unfortunately this "stable" upgrade of PostgreSQL 8.4.7-0squeeze2 to
> 8.4.8-0squeeze1 introduced a bug apparently from the 9.0 branch that is
> now consistently breaking queries on our production "stable" machines.

I propose the attached debdiff as an update to stable.  The patch applies
properly during the build and the regression test suite runs successfully.
(Of course the test case shipped with the patch got reverted, though.)

Kind regards
Philipp Kern
diff -u postgresql-8.4-8.4.8/debian/changelog postgresql-8.4-8.4.8/debian/changelog
--- postgresql-8.4-8.4.8/debian/changelog
+++ postgresql-8.4-8.4.8/debian/changelog
@@ -1,3 +1,10 @@
+postgresql-8.4 (8.4.8-0squeeze2) stable; urgency=low
+
+  * Back out "Fix plpgsql's issues with dropped columns in rowtypes in 8.4
+    branch.", which introduces a regression.  (Closes: #632028)
+
+ -- Philipp Kern <pkern@debian.org>  Wed, 29 Jun 2011 11:45:11 +0200
+
 postgresql-8.4 (8.4.8-0squeeze1) stable; urgency=low
 
   * New upstream bug fix release: (Closes: #626559)
only in patch2:
unchanged:
--- postgresql-8.4-8.4.8.orig/debian/patches/15-debian-bug-632028.patch
+++ postgresql-8.4-8.4.8/debian/patches/15-debian-bug-632028.patch
@@ -0,0 +1,755 @@
+commit 626efda94f53c6285c02552da3fae924f40508bd
+Author: Philipp Kern <pkern@debian.org>
+Date:   Wed Jun 29 11:43:04 2011 +0200
+
+    Revert "Fix plpgsql's issues with dropped columns in rowtypes in 8.4 branch."
+    
+    This caused a regression.  See Debian bug #632028 for details.
+    
+    This reverts commit 5d3853a7fa40b28b44b14084863fd83a188c9a9e.
+
+diff --git a/src/backend/access/common/Makefile b/src/backend/access/common/Makefile
+index a80ee38..9e05a6a 100644
+--- a/src/backend/access/common/Makefile
++++ b/src/backend/access/common/Makefile
+@@ -12,7 +12,6 @@ subdir = src/backend/access/common
+ top_builddir = ../../../..
+ include $(top_builddir)/src/Makefile.global
+ 
+-OBJS = heaptuple.o indextuple.o printtup.o reloptions.o scankey.o \
+-	tupconvert.o tupdesc.o
++OBJS = heaptuple.o indextuple.o printtup.o reloptions.o scankey.o tupdesc.o  
+ 
+ include $(top_srcdir)/src/backend/common.mk
+diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c
+deleted file mode 100644
+index 34e5f11..0000000
+--- a/src/backend/access/common/tupconvert.c
++++ /dev/null
+@@ -1,365 +0,0 @@
+-/*-------------------------------------------------------------------------
+- *
+- * tupconvert.c
+- *	  Tuple conversion support.
+- *
+- * These functions provide conversion between rowtypes that are logically
+- * equivalent but might have columns in a different order or different sets
+- * of dropped columns.	There is some overlap of functionality with the
+- * executor's "junkfilter" routines, but these functions work on bare
+- * HeapTuples rather than TupleTableSlots.
+- *
+- * Portions Copyright (c) 1996-2011, PostgreSQL Global Development Group
+- * Portions Copyright (c) 1994, Regents of the University of California
+- *
+- *
+- * IDENTIFICATION
+- *	  src/backend/access/common/tupconvert.c
+- *
+- *-------------------------------------------------------------------------
+- */
+-#include "postgres.h"
+-
+-#include "access/tupconvert.h"
+-#include "utils/builtins.h"
+-
+-
+-/*
+- * The conversion setup routines have the following common API:
+- *
+- * The setup routine checks whether the given source and destination tuple
+- * descriptors are logically compatible.  If not, it throws an error.
+- * If so, it returns NULL if they are physically compatible (ie, no conversion
+- * is needed), else a TupleConversionMap that can be used by do_convert_tuple
+- * to perform the conversion.
+- *
+- * The TupleConversionMap, if needed, is palloc'd in the caller's memory
+- * context.  Also, the given tuple descriptors are referenced by the map,
+- * so they must survive as long as the map is needed.
+- *
+- * The caller must supply a suitable primary error message to be used if
+- * a compatibility error is thrown.  Recommended coding practice is to use
+- * gettext_noop() on this string, so that it is translatable but won't
+- * actually be translated unless the error gets thrown.
+- *
+- *
+- * Implementation notes:
+- *
+- * The key component of a TupleConversionMap is an attrMap[] array with
+- * one entry per output column.  This entry contains the 1-based index of
+- * the corresponding input column, or zero to force a NULL value (for
+- * a dropped output column).  The TupleConversionMap also contains workspace
+- * arrays.
+- */
+-
+-
+-/*
+- * Set up for tuple conversion, matching input and output columns by
+- * position.  (Dropped columns are ignored in both input and output.)
+- *
+- * Note: the errdetail messages speak of indesc as the "returned" rowtype,
+- * outdesc as the "expected" rowtype.  This is okay for current uses but
+- * might need generalization in future.
+- */
+-TupleConversionMap *
+-convert_tuples_by_position(TupleDesc indesc,
+-						   TupleDesc outdesc,
+-						   const char *msg)
+-{
+-	TupleConversionMap *map;
+-	AttrNumber *attrMap;
+-	int			nincols;
+-	int			noutcols;
+-	int			n;
+-	int			i;
+-	int			j;
+-	bool		same;
+-
+-	/* Verify compatibility and prepare attribute-number map */
+-	n = outdesc->natts;
+-	attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+-	j = 0;						/* j is next physical input attribute */
+-	nincols = noutcols = 0;		/* these count non-dropped attributes */
+-	same = true;
+-	for (i = 0; i < n; i++)
+-	{
+-		Form_pg_attribute att = outdesc->attrs[i];
+-		Oid			atttypid;
+-		int32		atttypmod;
+-
+-		if (att->attisdropped)
+-			continue;			/* attrMap[i] is already 0 */
+-		noutcols++;
+-		atttypid = att->atttypid;
+-		atttypmod = att->atttypmod;
+-		for (; j < indesc->natts; j++)
+-		{
+-			att = indesc->attrs[j];
+-			if (att->attisdropped)
+-				continue;
+-			nincols++;
+-			/* Found matching column, check type */
+-			if (atttypid != att->atttypid ||
+-				(atttypmod != att->atttypmod && atttypmod >= 0))
+-				ereport(ERROR,
+-						(errcode(ERRCODE_DATATYPE_MISMATCH),
+-						 errmsg_internal("%s", _(msg)),
+-						 errdetail("Returned type %s does not match expected type %s in column %d.",
+-								   format_type_with_typemod(att->atttypid,
+-															att->atttypmod),
+-								   format_type_with_typemod(atttypid,
+-															atttypmod),
+-								   noutcols)));
+-			attrMap[i] = (AttrNumber) (j + 1);
+-			j++;
+-			break;
+-		}
+-		if (attrMap[i] == 0)
+-			same = false;		/* we'll complain below */
+-	}
+-
+-	/* Check for unused input columns */
+-	for (; j < indesc->natts; j++)
+-	{
+-		if (indesc->attrs[j]->attisdropped)
+-			continue;
+-		nincols++;
+-		same = false;			/* we'll complain below */
+-	}
+-
+-	/* Report column count mismatch using the non-dropped-column counts */
+-	if (!same)
+-		ereport(ERROR,
+-				(errcode(ERRCODE_DATATYPE_MISMATCH),
+-				 errmsg_internal("%s", _(msg)),
+-				 errdetail("Number of returned columns (%d) does not match "
+-						   "expected column count (%d).",
+-						   nincols, noutcols)));
+-
+-	/*
+-	 * Check to see if the map is one-to-one and the tuple types are the same.
+-	 * (We check the latter because if they're not, we want to do conversion
+-	 * to inject the right OID into the tuple datum.)
+-	 */
+-	if (indesc->natts == outdesc->natts &&
+-		indesc->tdtypeid == outdesc->tdtypeid)
+-	{
+-		for (i = 0; i < n; i++)
+-		{
+-			if (attrMap[i] == (i + 1))
+-				continue;
+-
+-			/*
+-			 * If it's a dropped column and the corresponding input column is
+-			 * also dropped, we needn't convert.  However, attlen and attalign
+-			 * must agree.
+-			 */
+-			if (attrMap[i] == 0 &&
+-				indesc->attrs[i]->attisdropped &&
+-				indesc->attrs[i]->attlen == outdesc->attrs[i]->attlen &&
+-				indesc->attrs[i]->attalign == outdesc->attrs[i]->attalign)
+-				continue;
+-
+-			same = false;
+-			break;
+-		}
+-	}
+-	else
+-		same = false;
+-
+-	if (same)
+-	{
+-		/* Runtime conversion is not needed */
+-		pfree(attrMap);
+-		return NULL;
+-	}
+-
+-	/* Prepare the map structure */
+-	map = (TupleConversionMap *) palloc(sizeof(TupleConversionMap));
+-	map->indesc = indesc;
+-	map->outdesc = outdesc;
+-	map->attrMap = attrMap;
+-	/* preallocate workspace for Datum arrays */
+-	map->outvalues = (Datum *) palloc(n * sizeof(Datum));
+-	map->outisnull = (bool *) palloc(n * sizeof(bool));
+-	n = indesc->natts + 1;		/* +1 for NULL */
+-	map->invalues = (Datum *) palloc(n * sizeof(Datum));
+-	map->inisnull = (bool *) palloc(n * sizeof(bool));
+-	map->invalues[0] = (Datum) 0;		/* set up the NULL entry */
+-	map->inisnull[0] = true;
+-
+-	return map;
+-}
+-
+-/*
+- * Set up for tuple conversion, matching input and output columns by name.
+- * (Dropped columns are ignored in both input and output.)	This is intended
+- * for use when the rowtypes are related by inheritance, so we expect an exact
+- * match of both type and typmod.  The error messages will be a bit unhelpful
+- * unless both rowtypes are named composite types.
+- */
+-TupleConversionMap *
+-convert_tuples_by_name(TupleDesc indesc,
+-					   TupleDesc outdesc,
+-					   const char *msg)
+-{
+-	TupleConversionMap *map;
+-	AttrNumber *attrMap;
+-	int			n;
+-	int			i;
+-	bool		same;
+-
+-	/* Verify compatibility and prepare attribute-number map */
+-	n = outdesc->natts;
+-	attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
+-	for (i = 0; i < n; i++)
+-	{
+-		Form_pg_attribute att = outdesc->attrs[i];
+-		char	   *attname;
+-		Oid			atttypid;
+-		int32		atttypmod;
+-		int			j;
+-
+-		if (att->attisdropped)
+-			continue;			/* attrMap[i] is already 0 */
+-		attname = NameStr(att->attname);
+-		atttypid = att->atttypid;
+-		atttypmod = att->atttypmod;
+-		for (j = 0; j < indesc->natts; j++)
+-		{
+-			att = indesc->attrs[j];
+-			if (att->attisdropped)
+-				continue;
+-			if (strcmp(attname, NameStr(att->attname)) == 0)
+-			{
+-				/* Found it, check type */
+-				if (atttypid != att->atttypid || atttypmod != att->atttypmod)
+-					ereport(ERROR,
+-							(errcode(ERRCODE_DATATYPE_MISMATCH),
+-							 errmsg_internal("%s", _(msg)),
+-							 errdetail("Attribute \"%s\" of type %s does not match corresponding attribute of type %s.",
+-									   attname,
+-									   format_type_be(outdesc->tdtypeid),
+-									   format_type_be(indesc->tdtypeid))));
+-				attrMap[i] = (AttrNumber) (j + 1);
+-				break;
+-			}
+-		}
+-		if (attrMap[i] == 0)
+-			ereport(ERROR,
+-					(errcode(ERRCODE_DATATYPE_MISMATCH),
+-					 errmsg_internal("%s", _(msg)),
+-					 errdetail("Attribute \"%s\" of type %s does not exist in type %s.",
+-							   attname,
+-							   format_type_be(outdesc->tdtypeid),
+-							   format_type_be(indesc->tdtypeid))));
+-	}
+-
+-	/*
+-	 * Check to see if the map is one-to-one and the tuple types are the same.
+-	 * (We check the latter because if they're not, we want to do conversion
+-	 * to inject the right OID into the tuple datum.)
+-	 */
+-	if (indesc->natts == outdesc->natts &&
+-		indesc->tdtypeid == outdesc->tdtypeid)
+-	{
+-		same = true;
+-		for (i = 0; i < n; i++)
+-		{
+-			if (attrMap[i] == (i + 1))
+-				continue;
+-
+-			/*
+-			 * If it's a dropped column and the corresponding input column is
+-			 * also dropped, we needn't convert.  However, attlen and attalign
+-			 * must agree.
+-			 */
+-			if (attrMap[i] == 0 &&
+-				indesc->attrs[i]->attisdropped &&
+-				indesc->attrs[i]->attlen == outdesc->attrs[i]->attlen &&
+-				indesc->attrs[i]->attalign == outdesc->attrs[i]->attalign)
+-				continue;
+-
+-			same = false;
+-			break;
+-		}
+-	}
+-	else
+-		same = false;
+-
+-	if (same)
+-	{
+-		/* Runtime conversion is not needed */
+-		pfree(attrMap);
+-		return NULL;
+-	}
+-
+-	/* Prepare the map structure */
+-	map = (TupleConversionMap *) palloc(sizeof(TupleConversionMap));
+-	map->indesc = indesc;
+-	map->outdesc = outdesc;
+-	map->attrMap = attrMap;
+-	/* preallocate workspace for Datum arrays */
+-	map->outvalues = (Datum *) palloc(n * sizeof(Datum));
+-	map->outisnull = (bool *) palloc(n * sizeof(bool));
+-	n = indesc->natts + 1;		/* +1 for NULL */
+-	map->invalues = (Datum *) palloc(n * sizeof(Datum));
+-	map->inisnull = (bool *) palloc(n * sizeof(bool));
+-	map->invalues[0] = (Datum) 0;		/* set up the NULL entry */
+-	map->inisnull[0] = true;
+-
+-	return map;
+-}
+-
+-/*
+- * Perform conversion of a tuple according to the map.
+- */
+-HeapTuple
+-do_convert_tuple(HeapTuple tuple, TupleConversionMap *map)
+-{
+-	AttrNumber *attrMap = map->attrMap;
+-	Datum	   *invalues = map->invalues;
+-	bool	   *inisnull = map->inisnull;
+-	Datum	   *outvalues = map->outvalues;
+-	bool	   *outisnull = map->outisnull;
+-	int			outnatts = map->outdesc->natts;
+-	int			i;
+-
+-	/*
+-	 * Extract all the values of the old tuple, offsetting the arrays so that
+-	 * invalues[0] is left NULL and invalues[1] is the first source attribute;
+-	 * this exactly matches the numbering convention in attrMap.
+-	 */
+-	heap_deform_tuple(tuple, map->indesc, invalues + 1, inisnull + 1);
+-
+-	/*
+-	 * Transpose into proper fields of the new tuple.
+-	 */
+-	for (i = 0; i < outnatts; i++)
+-	{
+-		int			j = attrMap[i];
+-
+-		outvalues[i] = invalues[j];
+-		outisnull[i] = inisnull[j];
+-	}
+-
+-	/*
+-	 * Now form the new tuple.
+-	 */
+-	return heap_form_tuple(map->outdesc, outvalues, outisnull);
+-}
+-
+-/*
+- * Free a TupleConversionMap structure.
+- */
+-void
+-free_conversion_map(TupleConversionMap *map)
+-{
+-	/* indesc and outdesc are not ours to free */
+-	pfree(map->attrMap);
+-	pfree(map->invalues);
+-	pfree(map->inisnull);
+-	pfree(map->outvalues);
+-	pfree(map->outisnull);
+-	pfree(map);
+-}
+diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h
+deleted file mode 100644
+index ab79f09..0000000
+--- a/src/include/access/tupconvert.h
++++ /dev/null
+@@ -1,44 +0,0 @@
+-/*-------------------------------------------------------------------------
+- *
+- * tupconvert.h
+- *	  Tuple conversion support.
+- *
+- *
+- * Portions Copyright (c) 1996-2011, PostgreSQL Global Development Group
+- * Portions Copyright (c) 1994, Regents of the University of California
+- *
+- * src/include/access/tupconvert.h
+- *
+- *-------------------------------------------------------------------------
+- */
+-#ifndef TUPCONVERT_H
+-#define TUPCONVERT_H
+-
+-#include "access/htup.h"
+-
+-
+-typedef struct TupleConversionMap
+-{
+-	TupleDesc	indesc;			/* tupdesc for source rowtype */
+-	TupleDesc	outdesc;		/* tupdesc for result rowtype */
+-	AttrNumber *attrMap;		/* indexes of input fields, or 0 for null */
+-	Datum	   *invalues;		/* workspace for deconstructing source */
+-	bool	   *inisnull;
+-	Datum	   *outvalues;		/* workspace for constructing result */
+-	bool	   *outisnull;
+-} TupleConversionMap;
+-
+-
+-extern TupleConversionMap *convert_tuples_by_position(TupleDesc indesc,
+-						   TupleDesc outdesc,
+-						   const char *msg);
+-
+-extern TupleConversionMap *convert_tuples_by_name(TupleDesc indesc,
+-					   TupleDesc outdesc,
+-					   const char *msg);
+-
+-extern HeapTuple do_convert_tuple(HeapTuple tuple, TupleConversionMap *map);
+-
+-extern void free_conversion_map(TupleConversionMap *map);
+-
+-#endif   /* TUPCONVERT_H */
+diff --git a/src/pl/plpgsql/src/nls.mk b/src/pl/plpgsql/src/nls.mk
+index de0d6c4..c1195c3 100644
+--- a/src/pl/plpgsql/src/nls.mk
++++ b/src/pl/plpgsql/src/nls.mk
+@@ -2,7 +2,7 @@
+ CATALOG_NAME	:= plpgsql
+ AVAIL_LANGUAGES	:= de es fr it ja ko ro pt_BR zh_CN zh_TW
+ GETTEXT_FILES	:= pl_comp.c pl_exec.c pl_gram.c pl_funcs.c pl_handler.c pl_scan.c
+-GETTEXT_TRIGGERS:= _ errmsg errmsg_plural:1,2 errdetail errdetail_log errdetail_plural:1,2 errhint errcontext yyerror plpgsql_yyerror
++GETTEXT_TRIGGERS:= _ errmsg errmsg_plural:1,2 errdetail errdetail_log errdetail_plural:1,2 errhint errcontext validate_tupdesc_compat:3 yyerror plpgsql_yyerror
+ 
+ .PHONY: gettext-files
+ gettext-files: distprep
+diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
+index 1374090..104765b 100644
+--- a/src/pl/plpgsql/src/pl_exec.c
++++ b/src/pl/plpgsql/src/pl_exec.c
+@@ -18,7 +18,6 @@
+ #include <ctype.h>
+ 
+ #include "access/transam.h"
+-#include "access/tupconvert.h"
+ #include "catalog/pg_proc.h"
+ #include "catalog/pg_type.h"
+ #include "executor/spi_priv.h"
+@@ -190,6 +189,8 @@ static Datum exec_simple_cast_value(Datum value, Oid valtype,
+ 					   Oid reqtype, int32 reqtypmod,
+ 					   bool isnull);
+ static void exec_init_tuple_store(PLpgSQL_execstate *estate);
++static void validate_tupdesc_compat(TupleDesc expected, TupleDesc returned,
++						const char *msg);
+ static void exec_set_found(PLpgSQL_execstate *estate, bool state);
+ static void plpgsql_create_econtext(PLpgSQL_execstate *estate);
+ static void plpgsql_destroy_econtext(PLpgSQL_execstate *estate);
+@@ -380,21 +381,14 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
+ 			 * expected result type.  XXX would be better to cache the tupdesc
+ 			 * instead of repeating get_call_result_type()
+ 			 */
+-			HeapTuple	rettup = (HeapTuple) DatumGetPointer(estate.retval);
+ 			TupleDesc	tupdesc;
+-			TupleConversionMap *tupmap;
+ 
+ 			switch (get_call_result_type(fcinfo, NULL, &tupdesc))
+ 			{
+ 				case TYPEFUNC_COMPOSITE:
+ 					/* got the expected result rowtype, now check it */
+-					tupmap = convert_tuples_by_position(estate.rettupdesc,
+-														tupdesc,
+-														gettext_noop("returned record type does not match expected record type"));
+-					/* it might need conversion */
+-					if (tupmap)
+-						rettup = do_convert_tuple(rettup, tupmap);
+-					/* no need to free map, we're about to return anyway */
++					validate_tupdesc_compat(tupdesc, estate.rettupdesc,
++											"returned record type does not match expected record type");
+ 					break;
+ 				case TYPEFUNC_RECORD:
+ 
+@@ -419,7 +413,9 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
+ 			 * Copy tuple to upper executor memory, as a tuple Datum. Make
+ 			 * sure it is labeled with the caller-supplied tuple type.
+ 			 */
+-			estate.retval = PointerGetDatum(SPI_returntuple(rettup, tupdesc));
++			estate.retval =
++				PointerGetDatum(SPI_returntuple((HeapTuple) DatumGetPointer(estate.retval),
++												tupdesc));
+ 		}
+ 		else
+ 		{
+@@ -708,20 +704,11 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
+ 		rettup = NULL;
+ 	else
+ 	{
+-		TupleConversionMap *tupmap;
+-
+-		rettup = (HeapTuple) DatumGetPointer(estate.retval);
+-		/* check rowtype compatibility */
+-		tupmap = convert_tuples_by_position(estate.rettupdesc,
+-											trigdata->tg_relation->rd_att,
+-											gettext_noop("returned row structure does not match the structure of the triggering table"));
+-		/* it might need conversion */
+-		if (tupmap)
+-			rettup = do_convert_tuple(rettup, tupmap);
+-		/* no need to free map, we're about to return anyway */
+-
++		validate_tupdesc_compat(trigdata->tg_relation->rd_att,
++								estate.rettupdesc,
++								"returned row structure does not match the structure of the triggering table");
+ 		/* Copy tuple to upper executor memory */
+-		rettup = SPI_copytuple(rettup);
++		rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval));
+ 	}
+ 
+ 	/*
+@@ -2203,7 +2190,6 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
+ 			case PLPGSQL_DTYPE_REC:
+ 				{
+ 					PLpgSQL_rec *rec = (PLpgSQL_rec *) retvar;
+-					TupleConversionMap *tupmap;
+ 
+ 					if (!HeapTupleIsValid(rec->tup))
+ 						ereport(ERROR,
+@@ -2212,16 +2198,9 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
+ 								  rec->refname),
+ 						errdetail("The tuple structure of a not-yet-assigned"
+ 								  " record is indeterminate.")));
+-					tupmap = convert_tuples_by_position(rec->tupdesc,
+-														tupdesc,
+-														gettext_noop("wrong record type supplied in RETURN NEXT"));
++					validate_tupdesc_compat(tupdesc, rec->tupdesc,
++								"wrong record type supplied in RETURN NEXT");
+ 					tuple = rec->tup;
+-					/* it might need conversion */
+-					if (tupmap)
+-					{
+-						tuple = do_convert_tuple(tuple, tupmap);
+-						free_conversion_map(tupmap);
+-					}
+ 				}
+ 				break;
+ 
+@@ -2301,7 +2280,6 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
+ {
+ 	Portal		portal;
+ 	uint32		processed = 0;
+-	TupleConversionMap *tupmap;
+ 
+ 	if (!estate->retisset)
+ 		ereport(ERROR,
+@@ -2324,9 +2302,8 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
+ 										   stmt->params);
+ 	}
+ 
+-	tupmap = convert_tuples_by_position(portal->tupDesc,
+-										estate->rettupdesc,
+-										gettext_noop("structure of query does not match function result type"));
++	validate_tupdesc_compat(estate->rettupdesc, portal->tupDesc,
++				   "structure of query does not match function result type");
+ 
+ 	while (true)
+ 	{
+@@ -2340,20 +2317,13 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
+ 		{
+ 			HeapTuple	tuple = SPI_tuptable->vals[i];
+ 
+-			if (tupmap)
+-				tuple = do_convert_tuple(tuple, tupmap);
+ 			tuplestore_puttuple(estate->tuple_store, tuple);
+-			if (tupmap)
+-				heap_freetuple(tuple);
+ 			processed++;
+ 		}
+ 
+ 		SPI_freetuptable(SPI_tuptable);
+ 	}
+ 
+-	if (tupmap)
+-		free_conversion_map(tupmap);
+-
+ 	SPI_freetuptable(SPI_tuptable);
+ 	SPI_cursor_close(portal);
+ 
+@@ -5247,6 +5217,45 @@ exec_simple_check_plan(PLpgSQL_expr *expr)
+ 	expr->expr_simple_type = exprType((Node *) tle->expr);
+ }
+ 
++/*
++ * Validates compatibility of supplied TupleDesc pair by checking number and type
++ * of attributes.
++ */
++static void
++validate_tupdesc_compat(TupleDesc expected, TupleDesc returned, const char *msg)
++{
++	int			i;
++	const char *dropped_column_type = gettext_noop("N/A (dropped column)");
++
++	if (!expected || !returned)
++		ereport(ERROR,
++				(errcode(ERRCODE_DATATYPE_MISMATCH),
++				 errmsg("%s", _(msg))));
++
++	if (expected->natts != returned->natts)
++		ereport(ERROR,
++				(errcode(ERRCODE_DATATYPE_MISMATCH),
++				 errmsg("%s", _(msg)),
++				 errdetail("Number of returned columns (%d) does not match "
++						   "expected column count (%d).",
++						   returned->natts, expected->natts)));
++
++	for (i = 0; i < expected->natts; i++)
++		if (expected->attrs[i]->atttypid != returned->attrs[i]->atttypid)
++			ereport(ERROR,
++					(errcode(ERRCODE_DATATYPE_MISMATCH),
++					 errmsg("%s", _(msg)),
++				   errdetail("Returned type %s does not match expected type "
++							 "%s in column \"%s\".",
++							 OidIsValid(returned->attrs[i]->atttypid) ?
++							 format_type_be(returned->attrs[i]->atttypid) :
++							 _(dropped_column_type),
++							 OidIsValid(expected->attrs[i]->atttypid) ?
++							 format_type_be(expected->attrs[i]->atttypid) :
++							 _(dropped_column_type),
++							 NameStr(expected->attrs[i]->attname))));
++}
++
+ /* ----------
+  * exec_set_found			Set the global found variable
+  *					to true/false
+diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
+index bbcdf12..163268c 100644
+--- a/src/test/regress/expected/plpgsql.out
++++ b/src/test/regress/expected/plpgsql.out
+@@ -3285,57 +3285,6 @@ select * from return_dquery();
+ (4 rows)
+ 
+ drop function return_dquery();
+--- test RETURN QUERY with dropped columns
+-create table tabwithcols(a int, b int, c int, d int);
+-insert into tabwithcols values(10,20,30,40),(50,60,70,80);
+-create or replace function returnqueryf()
+-returns setof tabwithcols as $$
+-begin
+-  return query select * from tabwithcols;
+-  return query execute 'select * from tabwithcols';
+-end;
+-$$ language plpgsql;
+-select * from returnqueryf();
+- a  | b  | c  | d  
+-----+----+----+----
+- 10 | 20 | 30 | 40
+- 50 | 60 | 70 | 80
+- 10 | 20 | 30 | 40
+- 50 | 60 | 70 | 80
+-(4 rows)
+-
+-alter table tabwithcols drop column b;
+-select * from returnqueryf();
+- a  | c  | d  
+-----+----+----
+- 10 | 30 | 40
+- 50 | 70 | 80
+- 10 | 30 | 40
+- 50 | 70 | 80
+-(4 rows)
+-
+-alter table tabwithcols drop column d;
+-select * from returnqueryf();
+- a  | c  
+-----+----
+- 10 | 30
+- 50 | 70
+- 10 | 30
+- 50 | 70
+-(4 rows)
+-
+-alter table tabwithcols add column d int;
+-select * from returnqueryf();
+- a  | c  | d 
+-----+----+---
+- 10 | 30 |  
+- 50 | 70 |  
+- 10 | 30 |  
+- 50 | 70 |  
+-(4 rows)
+-
+-drop function returnqueryf();
+-drop table tabwithcols;
+ -- Tests for 8.4's new RAISE features
+ create or replace function raise_test() returns void as $$
+ begin
+diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
+index 2241c1e..48bb538 100644
+--- a/src/test/regress/sql/plpgsql.sql
++++ b/src/test/regress/sql/plpgsql.sql
+@@ -2684,36 +2684,6 @@ select * from return_dquery();
+ 
+ drop function return_dquery();
+ 
+--- test RETURN QUERY with dropped columns
+-
+-create table tabwithcols(a int, b int, c int, d int);
+-insert into tabwithcols values(10,20,30,40),(50,60,70,80);
+-
+-create or replace function returnqueryf()
+-returns setof tabwithcols as $$
+-begin
+-  return query select * from tabwithcols;
+-  return query execute 'select * from tabwithcols';
+-end;
+-$$ language plpgsql;
+-
+-select * from returnqueryf();
+-
+-alter table tabwithcols drop column b;
+-
+-select * from returnqueryf();
+-
+-alter table tabwithcols drop column d;
+-
+-select * from returnqueryf();
+-
+-alter table tabwithcols add column d int;
+-
+-select * from returnqueryf();
+-
+-drop function returnqueryf();
+-drop table tabwithcols;
+-
+ -- Tests for 8.4's new RAISE features
+ 
+ create or replace function raise_test() returns void as $$

Attachment: signature.asc
Description: Digital signature


Reply to: