On 10/04/12 21:47, Jan Urbański wrote:
On 10/04/12 21:27, Tom Lane wrote:
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?=<wulc...@wulczer.org> writes:
Yes, that would be ideal, even though not backwards-compatible.
Back-patching is out of the question, but do we want to change trigger
functions to receive dictionaries in NEW?
Hm, I was not thinking of this as being trigger-specific, but more a
general principle that composite columns of tuples ought to be handled
in a recursive fashion.
Sure, that would be the way.
If so, should this be 9.2 material, or just a TODO?
If it can be done quickly and with not much risk, I'd vote for
squeezing it into 9.2, because it seems to me to be a clear bug that the
two directions are not handled consistently. If you don't have time for
it now or you don't think it would be a small/safe patch, we'd better
just put it on TODO.
It turned out not to be as straightforward as I though :(
The I/O code in PL/Python is a bit of a mess and that's something that
I'd like to address somewhere in the 9.3 development cycle. Right now
making the conversion function recursive is not possible without some
deep surgery (or kludgery...) so I limited myself to producing
regression-fixing patches for 9.2 and 9.1 (attached).
Cheers,
Jan
>From 513e8c484f32599a8753035fad638c8712339480 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Mon, 23 Apr 2012 02:07:46 +0200
Subject: [PATCH] Accept strings in PL/Python 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 Postgres tuples was
being used.
The reason why it's important is that trigger functions on tables with
composite columns get the composite row passed in as a string (from
record_out). This makes it impossible to implement passthrough
behaviour 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 objecst to produce
dictionaries that would then be correctly interpreted by the
Python->Postgres counterpart code. This would be too invasive to
backpatch in 9.1 and it is too late in the 9.2 cycle to do it in
HEAD. It should get revisited in 9.3, though.
Reported as bug #6559 by Kirill Simonov.
---
src/pl/plpython/expected/plpython_record.out | 16 ++++
src/pl/plpython/expected/plpython_trigger.out | 37 +++++++++
src/pl/plpython/plpython.c | 94 ++++++++++++-----------
src/pl/plpython/regression.diffs | 99 +++++++++++++++++++++++++
src/pl/plpython/regression.out | 20 +++++
src/pl/plpython/sql/plpython_record.sql | 10 +++
src/pl/plpython/sql/plpython_trigger.sql | 36 +++++++++
7 files changed, 268 insertions(+), 44 deletions(-)
create mode 100644 src/pl/plpython/regression.diffs
create mode 100644 src/pl/plpython/regression.out
diff --git a/src/pl/plpython/expected/plpython_record.out b/src/pl/plpython/expected/plpython_record.out
index 0bcc46c..4583307 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 2ef66a8..ea4d500 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)
+-- 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 530b5f0..7307e51 100644
--- 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
{
@@ -2417,26 +2407,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;
}
/*
@@ -2511,7 +2503,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;
@@ -2533,15 +2524,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;
}
@@ -2648,8 +2634,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;
@@ -2717,12 +2723,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;
@@ -2803,12 +2809,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;
@@ -2875,7 +2881,7 @@ PLyGenericObject_ToTuple(PLyTypeInfo *info, TupleDesc desc, PyObject *object)
pfree(values);
pfree(nulls);
- return tuple;
+ return HeapTupleGetDatum(tuple);
}
diff --git a/src/pl/plpython/regression.diffs b/src/pl/plpython/regression.diffs
new file mode 100644
index 0000000..5972e9a
--- /dev/null
+++ b/src/pl/plpython/regression.diffs
@@ -0,0 +1,99 @@
+*** /home/wulczer/src/pg/src/pl/plpython/expected/plpython_record.out 2012-04-23 02:06:56.706278888 +0200
+--- /home/wulczer/src/pg/src/pl/plpython/results/plpython_record.out 2012-04-23 02:07:14.890488658 +0200
+***************
+*** 293,303 ****
+ (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
+ -------------------
+--- 293,301 ----
+ (1 row)
+
+ SELECT * FROM test_type_record_as('str', 'one', 1, false);
+! ERROR: length of returned sequence did not match number of columns in row
+! CONTEXT: while creating return value
+! PL/Python function "test_type_record_as"
+ SELECT * FROM test_in_out_params('test_in');
+ second
+ -------------------
+
+======================================================================
+
+*** /home/wulczer/src/pg/src/pl/plpython/expected/plpython_trigger.out 2012-04-23 02:06:56.706278888 +0200
+--- /home/wulczer/src/pg/src/pl/plpython/results/plpython_trigger.out 2012-04-23 02:07:14.998489907 +0200
+***************
+*** 576,589 ****
+ 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);
+--- 576,593 ----
+ 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);
++ ERROR: length of returned sequence did not match number of columns in row
++ CONTEXT: while modifying trigger row
++ PL/Python function "composite_trigger_noop_f"
+ INSERT INTO composite_trigger_noop_test VALUES (ROW(NULL, 't'), ROW(1, 'f'));
++ ERROR: length of returned sequence did not match number of columns in row
++ CONTEXT: while modifying trigger row
++ PL/Python function "composite_trigger_noop_f"
+ SELECT * FROM composite_trigger_noop_test;
+! f1 | f2
+! ----+----
+! |
+! (1 row)
+
+ -- nested composite types
+ CREATE TYPE comp3 AS (c1 comp1, c2 comp2, m integer);
+***************
+*** 595,606 ****
+ 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)
+
+--- 599,614 ----
+ 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));
++ ERROR: length of returned sequence did not match number of columns in row
++ CONTEXT: while modifying trigger row
++ PL/Python function "composite_trigger_nested_f"
+ INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(NULL, 't'), ROW(1, 'f'), NULL));
++ ERROR: length of returned sequence did not match number of columns in row
++ CONTEXT: while modifying trigger row
++ PL/Python function "composite_trigger_nested_f"
+ SELECT * FROM composite_trigger_nested_test;
+! c
+! ---
+
+! (1 row)
+
+
+======================================================================
+
diff --git a/src/pl/plpython/regression.out b/src/pl/plpython/regression.out
new file mode 100644
index 0000000..593953e
--- /dev/null
+++ b/src/pl/plpython/regression.out
@@ -0,0 +1,20 @@
+test plpython_schema ... ok
+test plpython_populate ... ok
+test plpython_test ... ok
+test plpython_do ... ok
+test plpython_global ... ok
+test plpython_import ... ok
+test plpython_spi ... ok
+test plpython_newline ... ok
+test plpython_void ... ok
+test plpython_params ... ok
+test plpython_setof ... ok
+test plpython_record ... FAILED
+test plpython_trigger ... FAILED
+test plpython_types ... ok
+test plpython_error ... ok
+test plpython_unicode ... ok
+test plpython_quote ... ok
+test plpython_composite ... ok
+test plpython_subtransaction ... ok
+test plpython_drop ... ok
diff --git a/src/pl/plpython/sql/plpython_record.sql b/src/pl/plpython/sql/plpython_record.sql
index 8df65fb..9bab4c9 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 2afdf51..1263d53 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;
+
+
+-- 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;
--
1.7.9.5
>From 21bb3e09555834ca3951c9f1193514beeae97d9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Mon, 23 Apr 2012 01:51:39 +0200
Subject: [PATCH] Accept strings in PL/Python 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 Postgres tuples was
being used.
The reason why it's important is that trigger functions on tables with
composite columns get the composite row passed in as a string (from
record_out). This makes it impossible to implement passthrough
behaviour 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 objecst to produce
dictionaries that would then be correctly interpreted by the
Python->Postgres counterpart code. This would be too invasive to
backpatch in 9.1 and it is too late in the 9.2 cycle to do it in
HEAD. It should get revisited in 9.3, though.
Reported as bug #6559 by Kirill Simonov.
---
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 0bcc46c..4583307 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 2ef66a8..ea4d500 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)
+-- 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 280d3ed..ad30fc0 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 9d6af05..c503d2b 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 e52c5d5..11532b8 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 8df65fb..9bab4c9 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 2afdf51..1263d53 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;
+
+
+-- 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;
--
1.7.9.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
