Le dimanche 29 juin 2014 16:54:03 Tom Lane a écrit : > Abhijit Menon-Sen <a...@2ndquadrant.com> writes: > >> 3) This is such a simple change with no new infrastructure code > >> (PLyObject_ToComposite already exists). Can you think of a reason > >> why this wasn't done until now? Was it a simple miss or purposefully > >> excluded? > > > > This is not an authoritative answer: I think the infrastructure was > > originally missing, but was later added in #bc411f25 for OUT parameters. > > Perhaps it was overlooked at the time that the same code would suffice > > for this earlier-missing case. (I've Cc:ed Peter E. in case he has any > > comments.) > > > > I think the patch is ready for committer.
Sorry for being this late. I've tested the patch, everything seems to work as expected, including complex nesting of Composite and array types. No documentation changes are needed, since the limitation wasn't even mentioned before. Regression tests are ok, and the patch seems simple enough. Formatting looks OK too. > > I took a quick look at this; not really a review either, but I have > a couple comments. > > 1. While I think the patch does what it intends to, it's a bit distressing > that it will invoke the information lookups in PLyObject_ToComposite over > again for *each element* of the array. We probably ought to quantify that > overhead to see if it's bad enough that we need to do something about > improving caching, as speculated in the comment in PLyObject_ToComposite. I don't know how to do that without implementing the cache itself. > > 2. I wonder whether the no-composites restriction in plpy.prepare > (see lines 133ff in plpy_spi.c) could be removed as easily. Hum, I tried that, but its not that easy: lifting the restriction results in a SEGFAULT when trying to pfree the parameters given to SPI_ExecutePlan (line 320 in plpy_spi.c). Correct me if I'm wrong, but I think the problem is that HeapTupleGetDatum returns the t_data field, whereas heap_form_tuple allocation returns the address of the HeapTuple itself. Then, the datum itself has not been palloced. Changing the HeapTupleGetDatum call for an heap_copy_tuple_as_datum fixes this issue, but I'm not sure this the best way to do that. The attached patch implements this. > > regards, tom lane -- Ronan Dunklau http://dalibo.com - http://dalibo.org
diff --git a/src/pl/plpython/expected/plpython_composite.out b/src/pl/plpython/expected/plpython_composite.out
index 61b3046..e7a7edb 100644
--- a/src/pl/plpython/expected/plpython_composite.out
+++ b/src/pl/plpython/expected/plpython_composite.out
@@ -270,9 +270,13 @@ yield {'tab': [['first', 1], ['second', 2]],
{'first': 'fourth', 'second': 4}]}
$$ LANGUAGE plpythonu;
SELECT * FROM composite_types_table();
-ERROR: PL/Python functions cannot return type table_record[]
-DETAIL: PL/Python does not support conversion to arrays of row types.
-CONTEXT: PL/Python function "composite_types_table"
+ tab | typ
+----------------------------+----------------------------
+ {"(first,1)","(second,2)"} | {"(third,3)","(fourth,4)"}
+ {"(first,1)","(second,2)"} | {"(third,3)","(fourth,4)"}
+ {"(first,1)","(second,2)"} | {"(third,3)","(fourth,4)"}
+(3 rows)
+
-- check what happens if the output record descriptor changes
CREATE FUNCTION return_record(t text) RETURNS record AS $$
return {'t': t, 'val': 10}
diff --git a/src/pl/plpython/expected/plpython_spi.out b/src/pl/plpython/expected/plpython_spi.out
index 5175794..de547d2 100644
--- a/src/pl/plpython/expected/plpython_spi.out
+++ b/src/pl/plpython/expected/plpython_spi.out
@@ -376,6 +376,15 @@ plan = plpy.prepare("select fname, lname from users where fname like $1 || '%'",
["text"])
c = plpy.cursor(plan, ["a", "b"])
$$ LANGUAGE plpythonu;
+CREATE TYPE test_composite_type AS (
+ a1 int,
+ a2 varchar
+);
+CREATE OR REPLACE FUNCTION plan_composite_args() RETURNS test_composite_type AS $$
+plan = plpy.prepare("select $1::test_composite_type as c1", ["test_composite_type"])
+res = plpy.execute(plan, [{"a1": 3, "a2": "label"}])
+return res[0]["c1"]
+$$ LANGUAGE plpythonu;
SELECT simple_cursor_test();
simple_cursor_test
--------------------
@@ -432,3 +441,9 @@ CONTEXT: Traceback (most recent call last):
PL/Python function "cursor_plan_wrong_args", line 4, in <module>
c = plpy.cursor(plan, ["a", "b"])
PL/Python function "cursor_plan_wrong_args"
+SELECT plan_composite_args();
+ plan_composite_args
+---------------------
+ (3,label)
+(1 row)
+
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index b98318c..9576905 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -632,13 +632,12 @@ PL/Python function "test_type_conversion_array_mixed2"
CREATE FUNCTION test_type_conversion_array_record() RETURNS type_record[] AS $$
return [None]
$$ LANGUAGE plpythonu;
-ERROR: PL/Python functions cannot return type type_record[]
-DETAIL: PL/Python does not support conversion to arrays of row types.
SELECT * FROM test_type_conversion_array_record();
-ERROR: function test_type_conversion_array_record() does not exist
-LINE 1: SELECT * FROM test_type_conversion_array_record();
- ^
-HINT: No function matches the given name and argument types. You might need to add explicit type casts.
+ test_type_conversion_array_record
+-----------------------------------
+ {NULL}
+(1 row)
+
CREATE FUNCTION test_type_conversion_array_string() RETURNS text[] AS $$
return 'abc'
$$ LANGUAGE plpythonu;
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index e104356..f38e2a7 100644
--- a/src/pl/plpython/expected/plpython_types_3.out
+++ b/src/pl/plpython/expected/plpython_types_3.out
@@ -632,13 +632,12 @@ PL/Python function "test_type_conversion_array_mixed2"
CREATE FUNCTION test_type_conversion_array_record() RETURNS type_record[] AS $$
return [None]
$$ LANGUAGE plpython3u;
-ERROR: PL/Python functions cannot return type type_record[]
-DETAIL: PL/Python does not support conversion to arrays of row types.
SELECT * FROM test_type_conversion_array_record();
-ERROR: function test_type_conversion_array_record() does not exist
-LINE 1: SELECT * FROM test_type_conversion_array_record();
- ^
-HINT: No function matches the given name and argument types. You might need to add explicit type casts.
+ test_type_conversion_array_record
+-----------------------------------
+ {NULL}
+(1 row)
+
CREATE FUNCTION test_type_conversion_array_string() RETURNS text[] AS $$
return 'abc'
$$ LANGUAGE plpython3u;
diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c
index 060d514..6c3eeff 100644
--- a/src/pl/plpython/plpy_spi.c
+++ b/src/pl/plpython/plpy_spi.c
@@ -130,12 +130,7 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
plan->types[i] = typeId;
typeStruct = (Form_pg_type) GETSTRUCT(typeTup);
- if (typeStruct->typtype != TYPTYPE_COMPOSITE)
- PLy_output_datum_func(&plan->args[i], typeTup);
- else
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("plpy.prepare does not support composite types")));
+ PLy_output_datum_func(&plan->args[i], typeTup);
ReleaseSysCache(typeTup);
}
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 566cf6c..a8d1b3e 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -404,11 +404,7 @@ PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup)
Oid funcid;
if (type_is_rowtype(element_type))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("PL/Python functions cannot return type %s",
- format_type_be(arg->typoid)),
- errdetail("PL/Python does not support conversion to arrays of row types.")));
+ arg->func = PLyObject_ToComposite;
arg->elm = PLy_malloc0(sizeof(*arg->elm));
arg->elm->func = arg->func;
@@ -835,11 +831,6 @@ PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
else
{
nulls[i] = false;
-
- /*
- * We don't support arrays of row types yet, so the first argument
- * can be NULL.
- */
elems[i] = arg->elm->func(arg->elm, -1, obj);
}
Py_XDECREF(obj);
@@ -947,7 +938,7 @@ PLyMapping_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping)
pfree(values);
pfree(nulls);
- return HeapTupleGetDatum(tuple);
+ return heap_copy_tuple_as_datum(tuple, desc);
}
@@ -1033,7 +1024,7 @@ PLySequence_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence)
pfree(values);
pfree(nulls);
- return HeapTupleGetDatum(tuple);
+ return heap_copy_tuple_as_datum(tuple, desc);
}
@@ -1105,7 +1096,7 @@ PLyGenericObject_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *object
pfree(values);
pfree(nulls);
- return HeapTupleGetDatum(tuple);
+ return heap_copy_tuple_as_datum(tuple, desc);
}
/*
diff --git a/src/pl/plpython/sql/plpython_spi.sql b/src/pl/plpython/sql/plpython_spi.sql
index 09f0d7f..4675199 100644
--- a/src/pl/plpython/sql/plpython_spi.sql
+++ b/src/pl/plpython/sql/plpython_spi.sql
@@ -284,6 +284,17 @@ plan = plpy.prepare("select fname, lname from users where fname like $1 || '%'",
c = plpy.cursor(plan, ["a", "b"])
$$ LANGUAGE plpythonu;
+CREATE TYPE test_composite_type AS (
+ a1 int,
+ a2 varchar
+);
+
+CREATE OR REPLACE FUNCTION plan_composite_args() RETURNS test_composite_type AS $$
+plan = plpy.prepare("select $1::test_composite_type as c1", ["test_composite_type"])
+res = plpy.execute(plan, [{"a1": 3, "a2": "label"}])
+return res[0]["c1"]
+$$ LANGUAGE plpythonu;
+
SELECT simple_cursor_test();
SELECT double_cursor_close();
SELECT cursor_fetch();
@@ -293,3 +304,4 @@ SELECT next_after_close();
SELECT cursor_fetch_next_empty();
SELECT cursor_plan();
SELECT cursor_plan_wrong_args();
+SELECT plan_composite_args();
signature.asc
Description: This is a digitally signed message part.
