From 2dada0cc85ee36f4e2b32a0463cb75ad9466589a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 17 Aug 2011 17:07:16 -0400 Subject: [PATCH] Fix two issues in plpython's handling of composite results. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/pl/plpython/expected/plpython_record.out | 21 ++++ src/pl/plpython/plpython.c | 114 ++++++++++++++----- src/pl/plpython/sql/plpython_record.sql | 15 +++ 3 files changed, 120 insertions(+), 30 deletions(-) diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out index 7c600896ec..0bcc46c55d 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 da34a1732e..3d1f043602 100644 --- a/src/pl/plpython/plpython.c +++ b/src/pl/plpython/plpython.c @@ -1489,6 +1489,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 */ @@ -1505,39 +1543,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; } @@ -1701,10 +1721,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) { @@ -1966,7 +1985,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; @@ -1976,7 +1995,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; @@ -2050,6 +2069,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; @@ -2670,7 +2712,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; @@ -2756,7 +2802,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]; @@ -2819,7 +2869,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 d727e6054f..8df65fbfe1 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