Fix two issues in plpython's handling of composite results.

Dropped columns within a composite type were not handled correctly.
Also, we did not check for whether a composite result type had changed
since we cached the information about it.

Jan Urbański, per a bug report from Jean-Baptiste Quenot

diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out
index 7c600896ecf4f5dd14d1c7ebe947759d697fdd15..0bcc46c55d1181a512ed1a38d7532f75de851505 100644 (file)
--- 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 17c1a544ecb0e6254ee770f9397c740a8a7b71ec..b007f4ac6b5cab8080206f37e4dd35140ad37df1 100644 (file)
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -1449,6 +1449,44 @@ PLy_function_delete_args(PLyProcedure *proc)
                        PyDict_DelItemString(proc->globals, proc->argnames[i]);
 }
 
+/*
+ * Check if our cached information about a datatype is still valid
+ */
+static bool
+PLy_procedure_argument_valid(PLyTypeInfo *arg)
+{
+       HeapTuple       relTup;
+       bool            valid;
+
+       /* Nothing to cache unless type is composite */
+       if (arg->is_rowtype != 1)
+               return true;
+
+       /*
+        * Zero typ_relid means that we got called on an output argument of a
+        * function returning a unnamed record type; the info for it can't change.
+        */
+       if (!OidIsValid(arg->typ_relid))
+               return true;
+
+       /* Else we should have some cached data */
+       Assert(TransactionIdIsValid(arg->typrel_xmin));
+       Assert(ItemPointerIsValid(&arg->typrel_tid));
+
+       /* Get the pg_class tuple for the data type */
+       relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(arg->typ_relid));
+       if (!HeapTupleIsValid(relTup))
+               elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
+
+       /* If it has changed, the cached data 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
  */
@@ -1465,39 +1503,21 @@ PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
                  ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
                return false;
 
+       /* Else check the input argument datatypes */
        valid = true;
-       /* If there are composite input arguments, they might have changed */
        for (i = 0; i < proc->nargs; i++)
        {
-               Oid                     relid;
-               HeapTuple       relTup;
+               valid = PLy_procedure_argument_valid(&proc->args[i]);
 
                /* 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;
-
-               ReleaseSysCache(relTup);
        }
 
+       /* if the output type is composite, it might have changed */
+       if (valid)
+               valid = PLy_procedure_argument_valid(&proc->result);
+
        return valid;
 }
 
@@ -1661,10 +1681,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 returns record, those I/O functions will be set up
+                * when the function is first called.
                 */
                if (procStruct->pronargs)
                {
@@ -1926,7 +1945,7 @@ PLy_input_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc)
         * RECORDOID means we got called to create input functions for a tuple
         * fetched by plpy.execute or for an anonymous record type
         */
-       if (desc->tdtypeid != RECORDOID && !TransactionIdIsValid(arg->typrel_xmin))
+       if (desc->tdtypeid != RECORDOID)
        {
                HeapTuple       relTup;
 
@@ -1936,7 +1955,7 @@ PLy_input_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc)
                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 */
+               /* Remember XMIN and TID for later validation if cache is still OK */
                arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
                arg->typrel_tid = relTup->t_self;
 
@@ -2010,6 +2029,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 an
+        * anonymous record type
+        */
+       if (desc->tdtypeid != RECORDOID)
+       {
+               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);
+
+               /* Remember XMIN and TID for later validation if cache is still OK */
+               arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
+               arg->typrel_tid = relTup->t_self;
+
+               ReleaseSysCache(relTup);
+       }
+
        for (i = 0; i < desc->natts; i++)
        {
                HeapTuple       typeTup;
@@ -2630,7 +2672,11 @@ PLyMapping_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping)
                PLyObToDatum *att;
 
                if (desc->attrs[i]->attisdropped)
+               {
+                       values[i] = (Datum) 0;
+                       nulls[i] = true;
                        continue;
+               }
 
                key = NameStr(desc->attrs[i]->attname);
                value = NULL;
@@ -2716,7 +2762,11 @@ PLySequence_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence)
                PLyObToDatum *att;
 
                if (desc->attrs[i]->attisdropped)
+               {
+                       values[i] = (Datum) 0;
+                       nulls[i] = true;
                        continue;
+               }
 
                value = NULL;
                att = &info->out.r.atts[i];
@@ -2779,7 +2829,11 @@ PLyGenericObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *object)
                PLyObToDatum *att;
 
                if (desc->attrs[i]->attisdropped)
+               {
+                       values[i] = (Datum) 0;
+                       nulls[i] = true;
                        continue;
+               }
 
                key = NameStr(desc->attrs[i]->attname);
                value = NULL;

diff --git a/src/pl/plpython/sql/plpython_record.sql b/src/pl/plpython/sql/plpython_record.sql
index d727e6054ffcc2104dc9fed2930d894d40357606..8df65fbfe1f3af380a1f4fee97e99fad04436ab2 100644 (file)
--- 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