>From 3c0bf7519cad735160d9d222d6f86f84987b38b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulczer@wulczer.org>
Date: Wed, 17 Aug 2011 16:07:54 +0200
Subject: [PATCH 2/2] Guard against return type changing in PL/Python
 functions.

Functions cache their I/O routines and in case their return type is
composite, a change of the underlying type can cause the cache to
become invalid. PL/Python was already checking for composite type
changes for input arguments, now the check is extended to cover the
return type as well.

Per bug report from Jean-Baptiste Quenot.
---
 src/pl/plpython/expected/plpython_record.out |   21 ++++++
 src/pl/plpython/plpython.c                   |   93 ++++++++++++++++++-------
 src/pl/plpython/sql/plpython_record.sql      |   15 ++++
 3 files changed, 103 insertions(+), 26 deletions(-)

diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out
index 7c60089..0bcc46c 100644
--- a/src/pl/plpython/expected/plpython_record.out
+++ b/src/pl/plpython/expected/plpython_record.out
@@ -308,6 +308,27 @@ SELECT * FROM test_inout_params('test_in');
  test_in_inout
 (1 row)
 
+-- try changing the return types and call functions again
+ALTER TABLE table_record DROP COLUMN first;
+ALTER TABLE table_record DROP COLUMN second;
+ALTER TABLE table_record ADD COLUMN first text;
+ALTER TABLE table_record ADD COLUMN second int4;
+SELECT * FROM test_table_record_as('obj', 'one', 1, false);
+ first | second 
+-------+--------
+ one   |      1
+(1 row)
+
+ALTER TYPE type_record DROP ATTRIBUTE first;
+ALTER TYPE type_record DROP ATTRIBUTE second;
+ALTER TYPE type_record ADD ATTRIBUTE first text;
+ALTER TYPE type_record ADD ATTRIBUTE second int4;
+SELECT * FROM test_type_record_as('obj', 'one', 1, false);
+ first | second 
+-------+--------
+ one   |      1
+(1 row)
+
 -- errors cases
 CREATE FUNCTION test_type_record_error1() RETURNS type_record AS $$
     return { 'first': 'first' }
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 90d3c47..a254ffa 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -1489,6 +1489,42 @@ PLy_function_delete_args(PLyProcedure *proc)
 			PyDict_DelItemString(proc->globals, proc->argnames[i]);
 }
 
+static bool
+PLy_procedure_argument_valid(PLyTypeInfo *arg)
+{
+	Oid			relid;
+	HeapTuple	relTup;
+	bool		valid;
+
+	/* Only check input arguments that are composite */
+	if (arg->is_rowtype != 1) {
+		return true;
+	}
+
+	/* An uninitialised typ_relid means that we got called on an output
+	 * argument of a function returning a unnamed record type */
+	if (!OidIsValid(arg->typ_relid)) {
+		return true;
+	}
+
+	Assert(TransactionIdIsValid(arg->typrel_xmin));
+	Assert(ItemPointerIsValid(&arg->typrel_tid));
+
+	/* Get the pg_class tuple for the argument type */
+	relid = arg->typ_relid;
+	relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+	if (!HeapTupleIsValid(relTup))
+		elog(ERROR, "cache lookup failed for relation %u", relid);
+
+	/* If it has changed, the function is not valid */
+	valid = (arg->typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
+			 ItemPointerEquals(&arg->typrel_tid, &relTup->t_self));
+
+	ReleaseSysCache(relTup);
+
+	return valid;
+}
+
 /*
  * Decide whether a cached PLyProcedure struct is still valid
  */
@@ -1509,33 +1545,16 @@ PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
 	/* If there are composite input arguments, they might have changed */
 	for (i = 0; i < proc->nargs; i++)
 	{
-		Oid			relid;
-		HeapTuple	relTup;
-
 		/* Short-circuit on first changed argument */
 		if (!valid)
 			break;
 
-		/* Only check input arguments that are composite */
-		if (proc->args[i].is_rowtype != 1)
-			continue;
-
-		Assert(OidIsValid(proc->args[i].typ_relid));
-		Assert(TransactionIdIsValid(proc->args[i].typrel_xmin));
-		Assert(ItemPointerIsValid(&proc->args[i].typrel_tid));
-
-		/* Get the pg_class tuple for the argument type */
-		relid = proc->args[i].typ_relid;
-		relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
-		if (!HeapTupleIsValid(relTup))
-			elog(ERROR, "cache lookup failed for relation %u", relid);
-
-		/* If it has changed, the function is not valid */
-		if (!(proc->args[i].typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
-			  ItemPointerEquals(&proc->args[i].typrel_tid, &relTup->t_self)))
-			valid = false;
+		valid = PLy_procedure_argument_valid(&proc->args[i]);
+	}
 
-		ReleaseSysCache(relTup);
+	/* if the output argument is composite, it might have changed */
+	if (valid) {
+		valid = PLy_procedure_argument_valid(&proc->result);
 	}
 
 	return valid;
@@ -1701,10 +1720,9 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
 
 		/*
 		 * Now get information required for input conversion of the
-		 * procedure's arguments.  Note that we ignore output arguments here
-		 * --- since we don't support returning record, and that was already
-		 * checked above, there's no need to worry about multiple output
-		 * arguments.
+		 * procedure's arguments.  Note that we ignore output arguments here,
+		 * if the function is returning record, the I/O functions will be set
+		 * up when the function is first called.
 		 */
 		if (procStruct->pronargs)
 		{
@@ -2050,6 +2068,29 @@ PLy_output_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc)
 		arg->out.r.atts = PLy_malloc0(desc->natts * sizeof(PLyDatumToOb));
 	}
 
+	Assert(OidIsValid(desc->tdtypeid));
+
+	/*
+	 * RECORDOID means we got called to create output functions for a unnamed
+	 * record type
+	 */
+	if (desc->tdtypeid != RECORDOID && !TransactionIdIsValid(arg->typrel_xmin))
+	{
+		HeapTuple	relTup;
+
+		/* Get the pg_class tuple corresponding to the type of the output */
+		arg->typ_relid = typeidTypeRelid(desc->tdtypeid);
+		relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(arg->typ_relid));
+		if (!HeapTupleIsValid(relTup))
+			elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
+
+		/* Extract the XMIN value to later use it in PLy_procedure_valid */
+		arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
+		arg->typrel_tid = relTup->t_self;
+
+		ReleaseSysCache(relTup);
+	}
+
 	for (i = 0; i < desc->natts; i++)
 	{
 		HeapTuple	typeTup;
diff --git a/src/pl/plpython/sql/plpython_record.sql b/src/pl/plpython/sql/plpython_record.sql
index d727e60..8df65fb 100644
--- a/src/pl/plpython/sql/plpython_record.sql
+++ b/src/pl/plpython/sql/plpython_record.sql
@@ -112,6 +112,21 @@ SELECT * FROM test_in_out_params('test_in');
 SELECT * FROM test_in_out_params_multi('test_in');
 SELECT * FROM test_inout_params('test_in');
 
+-- try changing the return types and call functions again
+
+ALTER TABLE table_record DROP COLUMN first;
+ALTER TABLE table_record DROP COLUMN second;
+ALTER TABLE table_record ADD COLUMN first text;
+ALTER TABLE table_record ADD COLUMN second int4;
+
+SELECT * FROM test_table_record_as('obj', 'one', 1, false);
+
+ALTER TYPE type_record DROP ATTRIBUTE first;
+ALTER TYPE type_record DROP ATTRIBUTE second;
+ALTER TYPE type_record ADD ATTRIBUTE first text;
+ALTER TYPE type_record ADD ATTRIBUTE second int4;
+
+SELECT * FROM test_type_record_as('obj', 'one', 1, false);
 
 -- errors cases
 
-- 
1.7.5.4

