From ba3e4157a7d0c7e963a8b800a30b9789aea6dd96 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter_e@gmx.net> Date: Thu, 26 Apr 2012 21:03:48 +0300 Subject: [PATCH] PL/Python: Accept strings in functions returning composite types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before 9.1, PL/Python functions returning composite types could return a string and it would be parsed using record_in. The 9.1 changes made PL/Python only expect dictionaries, tuples, or objects supporting getattr as output of composite functions, resulting in a regression and a confusing error message, as the strings were interpreted as sequences and the code for transforming lists to database tuples was used. Fix this by treating strings separately as before, before checking for the other types. The reason why it's important to support string to database tuple conversion is that trigger functions on tables with composite columns get the composite row passed in as a string (from record_out). Without supporting converting this back using record_in, this makes it impossible to implement pass-through behavior for these columns, as PL/Python no longer accepts strings for composite values. A better solution would be to fix the code that transforms composite inputs into Python objects to produce dictionaries that would then be correctly interpreted by the Python->PostgreSQL counterpart code. But that would be too invasive to backpatch to 9.1, and it is too late in the 9.2 cycle to attempt it. It should be revisited in the future, though. Reported as bug #6559 by Kirill Simonov. Jan UrbaĆski --- src/pl/plpython/expected/plpython_record.out | 16 ++++ src/pl/plpython/expected/plpython_trigger.out | 37 +++++++++ src/pl/plpython/plpy_exec.c | 17 +--- src/pl/plpython/plpy_typeio.c | 78 +++++++++++-------- src/pl/plpython/plpy_typeio.h | 4 +- src/pl/plpython/sql/plpython_record.sql | 10 +++ src/pl/plpython/sql/plpython_trigger.sql | 36 +++++++++ 7 files changed, 151 insertions(+), 47 deletions(-) diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out index 0bcc46c55d1..458330713a8 100644 --- a/src/pl/plpython/expected/plpython_record.out +++ b/src/pl/plpython/expected/plpython_record.out @@ -38,6 +38,8 @@ elif typ == 'obj': type_record.first = first type_record.second = second return type_record +elif typ == 'str': + return "('%s',%r)" % (first, second) $$ LANGUAGE plpythonu; CREATE FUNCTION test_in_out_params(first in text, second out text) AS $$ return first + '_in_to_out'; @@ -290,6 +292,12 @@ SELECT * FROM test_type_record_as('obj', null, null, true); | (1 row) +SELECT * FROM test_type_record_as('str', 'one', 1, false); + first | second +-------+-------- + 'one' | 1 +(1 row) + SELECT * FROM test_in_out_params('test_in'); second ------------------- @@ -355,3 +363,11 @@ ERROR: attribute "second" does not exist in Python object HINT: To return null in a column, let the returned object have an attribute named after column with value None. CONTEXT: while creating return value PL/Python function "test_type_record_error3" +CREATE FUNCTION test_type_record_error4() RETURNS type_record AS $$ + return 'foo' +$$ LANGUAGE plpythonu; +SELECT * FROM test_type_record_error4(); +ERROR: malformed record literal: "foo" +DETAIL: Missing left parenthesis. +CONTEXT: while creating return value +PL/Python function "test_type_record_error4" diff --git a/src/pl/plpython/expected/plpython_trigger.out b/src/pl/plpython/expected/plpython_trigger.out index 2ef66a8f06d..58aa24b62cb 100644 --- a/src/pl/plpython/expected/plpython_trigger.out +++ b/src/pl/plpython/expected/plpython_trigger.out @@ -567,3 +567,40 @@ SELECT * FROM composite_trigger_test; (3,f) | (7,t) (1 row) +-- triggers with composite type columns (bug #6559) +CREATE TABLE composite_trigger_noop_test (f1 comp1, f2 comp2); +CREATE FUNCTION composite_trigger_noop_f() RETURNS trigger AS $$ + return 'MODIFY' +$$ LANGUAGE plpythonu; +CREATE TRIGGER composite_trigger_noop BEFORE INSERT ON composite_trigger_noop_test + FOR EACH ROW EXECUTE PROCEDURE composite_trigger_noop_f(); +INSERT INTO composite_trigger_noop_test VALUES (NULL, NULL); +INSERT INTO composite_trigger_noop_test VALUES (ROW(1, 'f'), NULL); +INSERT INTO composite_trigger_noop_test VALUES (ROW(NULL, 't'), ROW(1, 'f')); +SELECT * FROM composite_trigger_noop_test; + f1 | f2 +-------+------- + | + (1,f) | + (,t) | (1,f) +(3 rows) + +-- nested composite types +CREATE TYPE comp3 AS (c1 comp1, c2 comp2, m integer); +CREATE TABLE composite_trigger_nested_test(c comp3); +CREATE FUNCTION composite_trigger_nested_f() RETURNS trigger AS $$ + return 'MODIFY' +$$ LANGUAGE plpythonu; +CREATE TRIGGER composite_trigger_nested BEFORE INSERT ON composite_trigger_nested_test + FOR EACH ROW EXECUTE PROCEDURE composite_trigger_nested_f(); +INSERT INTO composite_trigger_nested_test VALUES (NULL); +INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(1, 'f'), NULL, 3)); +INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(NULL, 't'), ROW(1, 'f'), NULL)); +SELECT * FROM composite_trigger_nested_test; + c +------------------- + + ("(1,f)",,3) + ("(,t)","(1,f)",) +(3 rows) + diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c index 280d3ed1aca..ad30fc0065b 100644 --- a/src/pl/plpython/plpy_exec.c +++ b/src/pl/plpython/plpy_exec.c @@ -180,8 +180,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) } else if (proc->result.is_rowtype >= 1) { - TupleDesc desc; - HeapTuple tuple = NULL; + TupleDesc desc; /* make sure it's not an unnamed record */ Assert((proc->result.out.d.typoid == RECORDOID && @@ -192,18 +191,8 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) desc = lookup_rowtype_tupdesc(proc->result.out.d.typoid, proc->result.out.d.typmod); - tuple = PLyObject_ToTuple(&proc->result, desc, plrv); - - if (tuple != NULL) - { - fcinfo->isnull = false; - rv = HeapTupleGetDatum(tuple); - } - else - { - fcinfo->isnull = true; - rv = (Datum) NULL; - } + rv = PLyObject_ToCompositeDatum(&proc->result, desc, plrv); + fcinfo->isnull = (rv == (Datum) NULL); } else { diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c index 9d6af053761..c503d2b2dd1 100644 --- a/src/pl/plpython/plpy_typeio.c +++ b/src/pl/plpython/plpy_typeio.c @@ -49,10 +49,11 @@ static Datum PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *pl static Datum PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv); static Datum PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv); -/* conversion from Python objects to heap tuples (used by triggers and SRFs) */ -static HeapTuple PLyMapping_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping); -static HeapTuple PLySequence_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence); -static HeapTuple PLyGenericObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *object); +/* conversion from Python objects to composite Datums (used by triggers and SRFs) */ +static Datum PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string); +static Datum PLyMapping_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping); +static Datum PLySequence_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence); +static Datum PLyGenericObject_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *object); /* make allocations in the TopMemoryContext */ static void perm_fmgr_info(Oid functionId, FmgrInfo *finfo); @@ -333,26 +334,28 @@ PLyDict_FromTuple(PLyTypeInfo *info, HeapTuple tuple, TupleDesc desc) } /* - * Convert a Python object to a PostgreSQL tuple, using all supported - * conversion methods: tuple as a sequence, as a mapping or as an object that - * has __getattr__ support. + * Convert a Python object to a composite Datum, using all supported + * conversion methods: composite as a string, as a sequence, as a mapping or + * as an object that has __getattr__ support. */ -HeapTuple -PLyObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv) +Datum +PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv) { - HeapTuple tuple; + Datum datum; - if (PySequence_Check(plrv)) + if (PyString_Check(plrv) || PyUnicode_Check(plrv)) + datum = PLyString_ToComposite(info, desc, plrv); + else if (PySequence_Check(plrv)) /* composite type as sequence (tuple, list etc) */ - tuple = PLySequence_ToTuple(info, desc, plrv); + datum = PLySequence_ToComposite(info, desc, plrv); else if (PyMapping_Check(plrv)) /* composite type as mapping (currently only dict) */ - tuple = PLyMapping_ToTuple(info, desc, plrv); + datum = PLyMapping_ToComposite(info, desc, plrv); else /* returned as smth, must provide method __getattr__(name) */ - tuple = PLyGenericObject_ToTuple(info, desc, plrv); + datum = PLyGenericObject_ToComposite(info, desc, plrv); - return tuple; + return datum; } static void @@ -681,7 +684,6 @@ PLyObject_ToBytea(PLyObToDatum *arg, int32 typmod, PyObject *plrv) static Datum PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv) { - HeapTuple tuple = NULL; Datum rv; PLyTypeInfo info; TupleDesc desc; @@ -703,15 +705,10 @@ PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv) * that info instead of looking it up every time a tuple is returned from * the function. */ - tuple = PLyObject_ToTuple(&info, desc, plrv); + rv = PLyObject_ToCompositeDatum(&info, desc, plrv); PLy_typeinfo_dealloc(&info); - if (tuple != NULL) - rv = HeapTupleGetDatum(tuple); - else - rv = (Datum) NULL; - return rv; } @@ -818,8 +815,27 @@ PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv) return PointerGetDatum(array); } -static HeapTuple -PLyMapping_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping) + +static Datum +PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string) +{ + HeapTuple typeTup; + + typeTup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(desc->tdtypeid)); + if (!HeapTupleIsValid(typeTup)) + elog(ERROR, "cache lookup failed for type %u", desc->tdtypeid); + + PLy_output_datum_func2(&info->out.d, typeTup); + + ReleaseSysCache(typeTup); + ReleaseTupleDesc(desc); + + return PLyObject_ToDatum(&info->out.d, info->out.d.typmod, string); +} + + +static Datum +PLyMapping_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping) { HeapTuple tuple; Datum *values; @@ -887,12 +903,12 @@ PLyMapping_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping) pfree(values); pfree(nulls); - return tuple; + return HeapTupleGetDatum(tuple); } -static HeapTuple -PLySequence_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence) +static Datum +PLySequence_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence) { HeapTuple tuple; Datum *values; @@ -973,12 +989,12 @@ PLySequence_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence) pfree(values); pfree(nulls); - return tuple; + return HeapTupleGetDatum(tuple); } -static HeapTuple -PLyGenericObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *object) +static Datum +PLyGenericObject_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *object) { HeapTuple tuple; Datum *values; @@ -1045,7 +1061,7 @@ PLyGenericObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *object) pfree(values); pfree(nulls); - return tuple; + return HeapTupleGetDatum(tuple); } /* diff --git a/src/pl/plpython/plpy_typeio.h b/src/pl/plpython/plpy_typeio.h index e52c5d50479..11532b8c201 100644 --- a/src/pl/plpython/plpy_typeio.h +++ b/src/pl/plpython/plpy_typeio.h @@ -98,8 +98,8 @@ extern void PLy_output_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc); extern void PLy_output_record_funcs(PLyTypeInfo *arg, TupleDesc desc); -/* conversion from Python objects to heap tuples */ -extern HeapTuple PLyObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv); +/* conversion from Python objects to composite Datums */ +extern Datum PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv); /* conversion from heap tuples to Python dictionaries */ extern PyObject *PLyDict_FromTuple(PLyTypeInfo *info, HeapTuple tuple, TupleDesc desc); diff --git a/src/pl/plpython/sql/plpython_record.sql b/src/pl/plpython/sql/plpython_record.sql index 8df65fbfe1f..9bab4c9e82d 100644 --- a/src/pl/plpython/sql/plpython_record.sql +++ b/src/pl/plpython/sql/plpython_record.sql @@ -43,6 +43,8 @@ elif typ == 'obj': type_record.first = first type_record.second = second return type_record +elif typ == 'str': + return "('%s',%r)" % (first, second) $$ LANGUAGE plpythonu; CREATE FUNCTION test_in_out_params(first in text, second out text) AS $$ @@ -108,6 +110,8 @@ SELECT * FROM test_type_record_as('obj', null, 2, false); SELECT * FROM test_type_record_as('obj', 'three', 3, false); SELECT * FROM test_type_record_as('obj', null, null, true); +SELECT * FROM test_type_record_as('str', 'one', 1, false); + SELECT * FROM test_in_out_params('test_in'); SELECT * FROM test_in_out_params_multi('test_in'); SELECT * FROM test_inout_params('test_in'); @@ -151,3 +155,9 @@ CREATE FUNCTION test_type_record_error3() RETURNS type_record AS $$ $$ LANGUAGE plpythonu; SELECT * FROM test_type_record_error3(); + +CREATE FUNCTION test_type_record_error4() RETURNS type_record AS $$ + return 'foo' +$$ LANGUAGE plpythonu; + +SELECT * FROM test_type_record_error4(); diff --git a/src/pl/plpython/sql/plpython_trigger.sql b/src/pl/plpython/sql/plpython_trigger.sql index 2afdf511275..f60565cde6b 100644 --- a/src/pl/plpython/sql/plpython_trigger.sql +++ b/src/pl/plpython/sql/plpython_trigger.sql @@ -346,3 +346,39 @@ CREATE TRIGGER composite_trigger BEFORE INSERT ON composite_trigger_test INSERT INTO composite_trigger_test VALUES (NULL, NULL); SELECT * FROM composite_trigger_test; + + +-- triggers with composite type columns (bug #6559) + +CREATE TABLE composite_trigger_noop_test (f1 comp1, f2 comp2); + +CREATE FUNCTION composite_trigger_noop_f() RETURNS trigger AS $$ + return 'MODIFY' +$$ LANGUAGE plpythonu; + +CREATE TRIGGER composite_trigger_noop BEFORE INSERT ON composite_trigger_noop_test + FOR EACH ROW EXECUTE PROCEDURE composite_trigger_noop_f(); + +INSERT INTO composite_trigger_noop_test VALUES (NULL, NULL); +INSERT INTO composite_trigger_noop_test VALUES (ROW(1, 'f'), NULL); +INSERT INTO composite_trigger_noop_test VALUES (ROW(NULL, 't'), ROW(1, 'f')); +SELECT * FROM composite_trigger_noop_test; + + +-- nested composite types + +CREATE TYPE comp3 AS (c1 comp1, c2 comp2, m integer); + +CREATE TABLE composite_trigger_nested_test(c comp3); + +CREATE FUNCTION composite_trigger_nested_f() RETURNS trigger AS $$ + return 'MODIFY' +$$ LANGUAGE plpythonu; + +CREATE TRIGGER composite_trigger_nested BEFORE INSERT ON composite_trigger_nested_test + FOR EACH ROW EXECUTE PROCEDURE composite_trigger_nested_f(); + +INSERT INTO composite_trigger_nested_test VALUES (NULL); +INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(1, 'f'), NULL, 3)); +INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(NULL, 't'), ROW(1, 'f'), NULL)); +SELECT * FROM composite_trigger_nested_test; -- GitLab