Give a hint, when [] is incorrectly used for a composite type in array.

That used to be accepted, so let's try to give a hint to users on why
their PL/python functions no longer work.

Reviewed by Pavel Stehule.

Discussion: <CAH38_tmbqwaUyKs9yagyRra=SMaT45FPBxk1pmTYcM0TyXGG7Q@mail.gmail.com>

diff --git a/src/pl/plpython/expected/plpython_composite.out b/src/pl/plpython/expected/plpython_composite.out
index 1ab3b31e6841ef64e3e80d41eb4596be9e45400c..c6964841fbaa96ce72311552de35926ebcbcf2f6 100644 (file)
--- a/src/pl/plpython/expected/plpython_composite.out
+++ b/src/pl/plpython/expected/plpython_composite.out
@@ -579,3 +579,16 @@ SELECT * FROM composite_type_as_list();
  {{"(first,1)","(second,1)"},{"(first,2)","(second,2)"},{"(first,3)","(second,3)"}}
 (1 row)
 
+-- Starting with PostgreSQL 10, a composite type in an array cannot be
+-- represented as a Python list, because it's ambiguous with multi-dimensional
+-- arrays. So this throws an error now. The error should contain a useful hint
+-- on the issue.
+CREATE FUNCTION composite_type_as_list_broken()  RETURNS type_record[] AS $$
+  return [['first', 1]];
+$$ LANGUAGE plpythonu;
+SELECT * FROM composite_type_as_list_broken();
+ERROR:  malformed record literal: "first"
+DETAIL:  Missing left parenthesis.
+HINT:  To return a composite type in an array, return the composite type as a Python tuple, e.g. "[('foo')]"
+CONTEXT:  while creating return value
+PL/Python function "composite_type_as_list_broken"

diff --git a/src/pl/plpython/plpy_cursorobject.c b/src/pl/plpython/plpy_cursorobject.c
index 0e17a03ce7b196e1a4f03eec2f33014606a95351..e682bfe566a9e8589cd7dbf219e590936b3b9536 100644 (file)
--- a/src/pl/plpython/plpy_cursorobject.c
+++ b/src/pl/plpython/plpy_cursorobject.c
@@ -240,7 +240,8 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
                                        plan->values[j] =
                                                plan->args[j].out.d.func(&(plan->args[j].out.d),
                                                                                                 -1,
-                                                                                                elem);
+                                                                                                elem,
+                                                                                                false);
                                }
                                PG_CATCH();
                                {

diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c
index 25e4744c7d8c09758836f65c8b559f94515606ef..fa583fab164d1c3fec4d2b17a16300a39d360ba8 100644 (file)
--- a/src/pl/plpython/plpy_exec.c
+++ b/src/pl/plpython/plpy_exec.c
@@ -245,7 +245,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
                        desc = lookup_rowtype_tupdesc(proc->result.out.d.typoid,
                                                                                  proc->result.out.d.typmod);
 
-                       rv = PLyObject_ToCompositeDatum(&proc->result, desc, plrv);
+                       rv = PLyObject_ToCompositeDatum(&proc->result, desc, plrv, false);
                        fcinfo->isnull = (rv == (Datum) NULL);
 
                        ReleaseTupleDesc(desc);
@@ -253,7 +253,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
                else
                {
                        fcinfo->isnull = false;
-                       rv = (proc->result.out.d.func) (&proc->result.out.d, -1, plrv);
+                       rv = (proc->result.out.d.func) (&proc->result.out.d, -1, plrv, false);
                }
        }
        PG_CATCH();
@@ -984,7 +984,8 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
 
                                modvalues[i] = (att->func) (att,
                                                                                        tupdesc->attrs[atti]->atttypmod,
-                                                                                       plval);
+                                                                                       plval,
+                                                                                       false);
                                modnulls[i] = ' ';
                        }
                        else

diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c
index 09ee06d9e861e32741f371c69aaf2261eb2beeeb..0d556a2ec28081b1c0c61f9940c35c763b2b2645 100644 (file)
--- a/src/pl/plpython/plpy_spi.c
+++ b/src/pl/plpython/plpy_spi.c
@@ -264,7 +264,8 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
                                        plan->values[j] =
                                                plan->args[j].out.d.func(&(plan->args[j].out.d),
                                                                                                 -1,
-                                                                                                elem);
+                                                                                                elem,
+                                                                                                false);
                                }
                                PG_CATCH();
                                {

diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 85d50c2b0c322fea842466036de905df1e292494..d346e22591972ab7cfc64c30416fd1531a01a898 100644 (file)
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -14,6 +14,7 @@
 #include "parser/parse_type.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
+#include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/numeric.h"
@@ -49,21 +50,21 @@ static PyObject *PLyList_FromArray_recurse(PLyDatumToOb *elm, int *dims, int ndi
                                                  char **dataptr_p, bits8 **bitmap_p, int *bitmask_p);
 
 /* conversion from Python objects to Datums */
-static Datum PLyObject_ToBool(PLyObToDatum *arg, int32 typmod, PyObject *plrv);
-static Datum PLyObject_ToBytea(PLyObToDatum *arg, int32 typmod, PyObject *plrv);
-static Datum PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv);
-static Datum PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv);
-static Datum PLyObject_ToTransform(PLyObToDatum *arg, int32 typmod, PyObject *plrv);
-static Datum PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv);
+static Datum PLyObject_ToBool(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray);
+static Datum PLyObject_ToBytea(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray);
+static Datum PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray);
+static Datum PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray);
+static Datum PLyObject_ToTransform(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray);
+static Datum PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray);
 static void PLySequence_ToArray_recurse(PLyObToDatum *elm, PyObject *list,
                                                        int *dims, int ndim, int dim,
                                                        Datum *elems, bool *nulls, int *currelem);
 
 /* conversion from Python objects to composite Datums (used by triggers and SRFs) */
-static Datum PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string);
+static Datum PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string, bool inarray);
 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);
+static Datum PLyGenericObject_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *object, bool inarray);
 
 void
 PLy_typeinfo_init(PLyTypeInfo *arg, MemoryContext mcxt)
@@ -341,12 +342,12 @@ PLyDict_FromTuple(PLyTypeInfo *info, HeapTuple tuple, TupleDesc desc)
  *     as an object that has __getattr__ support.
  */
 Datum
-PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv)
+PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv, bool inarray)
 {
        Datum           datum;
 
        if (PyString_Check(plrv) || PyUnicode_Check(plrv))
-               datum = PLyString_ToComposite(info, desc, plrv);
+               datum = PLyString_ToComposite(info, desc, plrv, inarray);
        else if (PySequence_Check(plrv))
                /* composite type as sequence (tuple, list etc) */
                datum = PLySequence_ToComposite(info, desc, plrv);
@@ -355,7 +356,7 @@ PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv)
                datum = PLyMapping_ToComposite(info, desc, plrv);
        else
                /* returned as smth, must provide method __getattr__(name) */
-               datum = PLyGenericObject_ToComposite(info, desc, plrv);
+               datum = PLyGenericObject_ToComposite(info, desc, plrv, inarray);
 
        return datum;
 }
@@ -746,7 +747,7 @@ PLyList_FromArray_recurse(PLyDatumToOb *elm, int *dims, int ndim, int dim,
  * type can parse.
  */
 static Datum
-PLyObject_ToBool(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
+PLyObject_ToBool(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray)
 {
        Datum           rv;
 
@@ -765,7 +766,7 @@ PLyObject_ToBool(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
  * with embedded nulls.  And it's faster this way.
  */
 static Datum
-PLyObject_ToBytea(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
+PLyObject_ToBytea(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray)
 {
        PyObject   *volatile plrv_so = NULL;
        Datum           rv;
@@ -809,7 +810,7 @@ PLyObject_ToBytea(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
  * for obtaining PostgreSQL tuples.
  */
 static Datum
-PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
+PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray)
 {
        Datum           rv;
        PLyTypeInfo info;
@@ -836,7 +837,7 @@ PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
         * that info instead of looking it up every time a tuple is returned from
         * the function.
         */
-       rv = PLyObject_ToCompositeDatum(&info, desc, plrv);
+       rv = PLyObject_ToCompositeDatum(&info, desc, plrv, inarray);
 
        ReleaseTupleDesc(desc);
 
@@ -908,26 +909,70 @@ PLyObject_AsString(PyObject *plrv)
  * cstring into PostgreSQL type.
  */
 static Datum
-PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
+PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray)
 {
+       char       *str;
+
        Assert(plrv != Py_None);
 
+       str = PLyObject_AsString(plrv);
+
+       /*
+        * If we are parsing a composite type within an array, and the string
+        * isn't a valid record literal, there's a high chance that the function
+        * did something like:
+        *
+        * CREATE FUNCTION .. RETURNS comptype[] AS $$ return [['foo', 'bar']] $$
+        * LANGUAGE plpython;
+        *
+        * Before PostgreSQL 10, that was interpreted as a single-dimensional
+        * array, containing record ('foo', 'bar'). PostgreSQL 10 added support
+        * for multi-dimensional arrays, and it is now interpreted as a
+        * two-dimensional array, containing two records, 'foo', and 'bar'.
+        * record_in() will throw an error, because "foo" is not a valid record
+        * literal.
+        *
+        * To make that less confusing to users who are upgrading from older
+        * versions, try to give a hint in the typical instances of that. If we are
+        * parsing an array of composite types, and we see a string literal that
+        * is not a valid record literal, give a hint. We only want to give the
+        * hint in the narrow case of a malformed string literal, not any error
+        * from record_in(), so check for that case here specifically.
+        *
+        * This check better match the one in record_in(), so that we don't forbid
+        * literals that are actually valid!
+        */
+       if (inarray && arg->typfunc.fn_oid == F_RECORD_IN)
+       {
+               char       *ptr = str;
+
+               /* Allow leading whitespace */
+               while (*ptr && isspace((unsigned char) *ptr))
+                       ptr++;
+               if (*ptr++ != '(')
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                                        errmsg("malformed record literal: \"%s\"", str),
+                                        errdetail("Missing left parenthesis."),
+                                        errhint("To return a composite type in an array, return the composite type as a Python tuple, e.g. \"[('foo')]\"")));
+       }
+
        return InputFunctionCall(&arg->typfunc,
-                                                        PLyObject_AsString(plrv),
+                                                        str,
                                                         arg->typioparam,
                                                         typmod);
 }
 
 
 static Datum
-PLyObject_ToTransform(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
+PLyObject_ToTransform(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray)
 {
        return FunctionCall1(&arg->typtransform, PointerGetDatum(plrv));
 }
 
 
 static Datum
-PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv)
+PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv, bool inarray)
 {
        ArrayType  *array;
        int                     i;
@@ -1085,7 +1130,7 @@ PLySequence_ToArray_recurse(PLyObToDatum *elm, PyObject *list,
                        else
                        {
                                nulls[*currelem] = false;
-                               elems[*currelem] = elm->func(elm, -1, obj);
+                               elems[*currelem] = elm->func(elm, -1, obj, true);
                        }
                        Py_XDECREF(obj);
                        (*currelem)++;
@@ -1095,7 +1140,7 @@ PLySequence_ToArray_recurse(PLyObToDatum *elm, PyObject *list,
 
 
 static Datum
-PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string)
+PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string, bool inarray)
 {
        Datum           result;
        HeapTuple       typeTup;
@@ -1120,7 +1165,7 @@ PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string)
 
        ReleaseSysCache(typeTup);
 
-       result = PLyObject_ToDatum(&locinfo.out.d, desc->tdtypmod, string);
+       result = PLyObject_ToDatum(&locinfo.out.d, desc->tdtypmod, string, inarray);
 
        MemoryContextDelete(cxt);
 
@@ -1172,7 +1217,7 @@ PLyMapping_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *mapping)
                        }
                        else if (value)
                        {
-                               values[i] = (att->func) (att, -1, value);
+                               values[i] = (att->func) (att, -1, value, false);
                                nulls[i] = false;
                        }
                        else
@@ -1265,7 +1310,7 @@ PLySequence_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence)
                        }
                        else if (value)
                        {
-                               values[i] = (att->func) (att, -1, value);
+                               values[i] = (att->func) (att, -1, value, false);
                                nulls[i] = false;
                        }
 
@@ -1294,7 +1339,7 @@ PLySequence_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *sequence)
 
 
 static Datum
-PLyGenericObject_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *object)
+PLyGenericObject_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *object, bool inarray)
 {
        Datum           result;
        HeapTuple       tuple;
@@ -1335,16 +1380,29 @@ PLyGenericObject_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *object
                        }
                        else if (value)
                        {
-                               values[i] = (att->func) (att, -1, value);
+                               values[i] = (att->func) (att, -1, value, false);
                                nulls[i] = false;
                        }
                        else
+                       {
+                               /*
+                                * No attribute for this column in the object.
+                                *
+                                * If we are parsing a composite type in an array, a likely
+                                * cause is that the function contained something like "[[123,
+                                * 'foo']]". Before PostgreSQL 10, that was interpreted as an
+                                * array, with a composite type (123, 'foo') in it. But now
+                                * it's interpreted as a two-dimensional array, and we try to
+                                * interpret "123" as the composite type. See also similar
+                                * heuristic in PLyObject_ToDatum().
+                                */
                                ereport(ERROR,
                                                (errcode(ERRCODE_UNDEFINED_COLUMN),
                                                 errmsg("attribute \"%s\" does not exist in Python object", key),
-                                                errhint("To return null in a column, "
-                                                  "let the returned object have an attribute named "
-                                                                "after column with value None.")));
+                                                inarray ?
+                                                errhint("To return a composite type in an array, return the composite type as a Python tuple, e.g. \"[('foo')]\"") :
+                                                errhint("To return null in a column, let the returned object have an attribute named after column with value None.")));
+                       }
 
                        Py_XDECREF(value);
                        value = NULL;

diff --git a/src/pl/plpython/plpy_typeio.h b/src/pl/plpython/plpy_typeio.h
index 29fff61dc56b7df151de38a6aedc2a827350d903..5f5c1ad5c6babfb1e7c5485decd50eeec361693b 100644 (file)
--- a/src/pl/plpython/plpy_typeio.h
+++ b/src/pl/plpython/plpy_typeio.h
@@ -10,8 +10,11 @@
 #include "fmgr.h"
 #include "storage/itemptr.h"
 
+/*
+ * Conversion from PostgreSQL Datum to a Python object.
+ */
 struct PLyDatumToOb;
-typedef PyObject *(*PLyDatumToObFunc) (struct PLyDatumToOb *, Datum);
+typedef PyObject *(*PLyDatumToObFunc) (struct PLyDatumToOb *arg, Datum val);
 
 typedef struct PLyDatumToOb
 {
@@ -39,11 +42,15 @@ typedef union PLyTypeInput
        PLyTupleToOb r;
 } PLyTypeInput;
 
-/* convert PyObject to a Postgresql Datum or tuple.
- * output from Python
+/*
+ * Conversion from Python object to a Postgresql Datum.
+ *
+ * The 'inarray' argument to the conversion function is true, if the
+ * converted value was in an array (Python list). It is used to give a
+ * better error message in some cases.
  */
 struct PLyObToDatum;
-typedef Datum (*PLyObToDatumFunc) (struct PLyObToDatum *, int32, PyObject *);
+typedef Datum (*PLyObToDatumFunc) (struct PLyObToDatum *arg, int32 typmod, PyObject *val, bool inarray);
 
 typedef struct PLyObToDatum
 {
@@ -104,7 +111,7 @@ 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 composite Datums */
-extern Datum PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv);
+extern Datum PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObject *plrv, bool isarray);
 
 /* 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_composite.sql b/src/pl/plpython/sql/plpython_composite.sql
index 7a60b1f6b791513f15791966df0a15484bdbf5e6..0fd2f5d5e3bc40648f96b412017c086f0a27fac1 100644 (file)
--- a/src/pl/plpython/sql/plpython_composite.sql
+++ b/src/pl/plpython/sql/plpython_composite.sql
@@ -213,3 +213,12 @@ CREATE FUNCTION composite_type_as_list()  RETURNS type_record[] AS $$
   return [[('first', 1), ('second', 1)], [('first', 2), ('second', 2)], [('first', 3), ('second', 3)]];
 $$ LANGUAGE plpythonu;
 SELECT * FROM composite_type_as_list();
+
+-- Starting with PostgreSQL 10, a composite type in an array cannot be
+-- represented as a Python list, because it's ambiguous with multi-dimensional
+-- arrays. So this throws an error now. The error should contain a useful hint
+-- on the issue.
+CREATE FUNCTION composite_type_as_list_broken()  RETURNS type_record[] AS $$
+  return [['first', 1]];
+$$ LANGUAGE plpythonu;
+SELECT * FROM composite_type_as_list_broken();