Thread: Bad behavior from plpython 'return []'

CREATE FUNCTION pg_temp.bad() RETURNS text[] LANGUAGE plpythonu AS 
$$return []$$;
SELECT pg_temp.bad(); bad
----- {}
(1 row)

SELECT pg_temp.bad() = '{}'::text[]; ?column?
---------- f
(1 row)

Erm?? Turns out this is because

SELECT array_dims(pg_temp.bad()), array_dims('{}'::text[]); array_dims | array_dims
------------+------------ [1:0]      |
(1 row)

and array_eq does this right off the bat:

>     /* fast path if the arrays do not have the same dimensionality */
>     if (ndims1 != ndims2 ||
>         memcmp(dims1, dims2, ndims1 * sizeof(int)) != 0 ||
>         memcmp(lbs1, lbs2, ndims1 * sizeof(int)) != 0)
>         result = false;

plpython is calling construct_md_array() with ndims set to 1, *lbs=1 and 
(I'm pretty sure) *dims=0. array_in throws that combination out as 
bogus; I think that construct_md_array should at least assert() that as 
well. It's only used in a few places outside of arrayfuncs.c, but I find 
it rather disturbing that an included PL has been broken in this fashion 
for quite some time (PLySequence_ToArray() is the same in 9.0). There's 
at least one plpython unit test that would have thrown an assert.

plperl appears to be immune from this because it calls 
accumArrayResult() inside a loop that shouldn't execute for a 0 length 
array. Would that be the preferred method of building arrays in 
plpython? ISTM that'd be wasteful since it would incur a useless copy 
for everything that's varlena (AFAICT plperl already suffers from this).
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461

On Thu, Jun 30, 2016 at 9:25 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
> CREATE FUNCTION pg_temp.bad() RETURNS text[] LANGUAGE plpythonu AS $$return
> []$$;
> SELECT pg_temp.bad();
>  bad
> -----
>  {}
> (1 row)
>
> SELECT pg_temp.bad() = '{}'::text[];
>  ?column?
> ----------
>  f
> (1 row)
>
> Erm?? Turns out this is because
>
> SELECT array_dims(pg_temp.bad()), array_dims('{}'::text[]);
>  array_dims | array_dims
> ------------+------------
>  [1:0]      |
> (1 row)

Yeah, that's a bug.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Jun 30, 2016 at 9:25 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
>> SELECT array_dims(pg_temp.bad()), array_dims('{}'::text[]);
>> array_dims | array_dims
>> ------------+------------
>> [1:0]      |
>> (1 row)

> Yeah, that's a bug.

It looks like this is because PLySequence_ToArray neglects to special-case
zero-element arrays.  We could fix it there, but this is not the first
such bug.  I wonder if we should change construct_md_array to force
zero-element arrays to be converted to empty arrays, rather than assuming
callers will have short-circuited the case earlier.  Something like
/* fast track for empty array */if (ndims == 0)    return construct_empty_array(elmtype);
nelems = ArrayGetNItems(ndims, dims);

+    /* if caller tries to specify zero-length array, make it empty */
+    if (nelems <= 0)
+        return construct_empty_array(elmtype);
+/* compute required space */nbytes = 0;hasnulls = false;

But that might introduce new problems too, if any callers expect the
array dimensions to be exactly what they asked for.
        regards, tom lane

On 7/1/16 2:52 PM, Tom Lane wrote:
> +    /* if caller tries to specify zero-length array, make it empty */
> +    if (nelems <= 0)
> +        return construct_empty_array(elmtype);
> +
>     /* compute required space */
>     nbytes = 0;
>     hasnulls = false;
>
> But that might introduce new problems too, if any callers expect the
> array dimensions to be exactly what they asked for.

You mean ndims? What if instead of an empty array it returned an array 
where *dims was just all zeros (and correctly set *lbs)? array_eq would 
still need to account for that, but I think we don't have a choice about 
that unless we expressly forbid arrays where any of the elements of 
*dims were 0 (which I suspect we should probably do anyway... I don't 
see how you can do anything with a 2x0x3 array...)
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461