PL/Python: Accept strings in functions returning composite types

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

diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out
index 0bcc46c55d1181a512ed1a38d7532f75de851505..458330713a8c45e8db78e97f5e1a35640d3aa8a1 100644 (file)
--- 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 2ef66a8f06d4005e728d3944938345f63981a088..58aa24b62cb6ae675258dba604ddcef6dd2c266a 100644 (file)
--- 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/plpython.c b/src/pl/plpython/plpython.c
index c7b9b6003edd8c9ea736c4f0943c6d8f2925273a..95efd0b42e6d2d9ab341abe3847b142ea6061c5a 100644 (file)
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -410,10 +410,11 @@ static Datum PLyObject_ToComposite(PLyObToDatum *, int32, PyObject *);
 static Datum PLyObject_ToDatum(PLyObToDatum *, int32, PyObject *);
 static Datum PLySequence_ToArray(PLyObToDatum *, int32, PyObject *);
 
-static HeapTuple PLyObject_ToTuple(PLyTypeInfo *, TupleDesc, PyObject *);
-static HeapTuple PLyMapping_ToTuple(PLyTypeInfo *, TupleDesc, PyObject *);
-static HeapTuple PLySequence_ToTuple(PLyTypeInfo *, TupleDesc, PyObject *);
-static HeapTuple PLyGenericObject_ToTuple(PLyTypeInfo *, TupleDesc, PyObject *);
+static Datum PLyObject_ToCompositeDatum(PLyTypeInfo *, TupleDesc, PyObject *);
+static Datum PLyString_ToComposite(PLyTypeInfo *, TupleDesc, PyObject *);
+static Datum PLyMapping_ToComposite(PLyTypeInfo *, TupleDesc, PyObject *);
+static Datum PLySequence_ToComposite(PLyTypeInfo *, TupleDesc, PyObject *);
+static Datum PLyGenericObject_ToComposite(PLyTypeInfo *, TupleDesc, PyObject *);
 
 /*
  * Currently active plpython function
@@ -1213,7 +1214,6 @@ PLy_function_handler(FunctionCallInfo fcinfo, PLyProcedure *proc)
                else if (proc->result.is_rowtype >= 1)
                {
                        TupleDesc       desc;
-                       HeapTuple       tuple = NULL;
 
                        /* make sure it's not an unnamed record */
                        Assert((proc->result.out.d.typoid == RECORDOID &&
@@ -1224,18 +1224,8 @@ PLy_function_handler(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
                {
@@ -2419,26 +2409,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.
  */
-static HeapTuple
-PLyObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv)
+static 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;
 }
 
 /*
@@ -2513,7 +2505,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;
@@ -2535,15 +2526,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;
 }
 
@@ -2650,8 +2636,28 @@ 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;
@@ -2719,12 +2725,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;
@@ -2805,12 +2811,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;
@@ -2877,7 +2883,7 @@ PLyGenericObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *object)
        pfree(values);
        pfree(nulls);
 
-       return tuple;
+       return HeapTupleGetDatum(tuple);
 }
 
 

diff --git a/src/pl/plpython/sql/plpython_record.sql b/src/pl/plpython/sql/plpython_record.sql
index 8df65fbfe1f3af380a1f4fee97e99fad04436ab2..9bab4c9e82d96fa3b353805baa4343f74d3c6752 100644 (file)
--- 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 2afdf511275541e74b971ec3d78f3d0c74ae6a81..f60565cde6bdaaf72b7fb74dea92acec9807e332 100644 (file)
--- 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;