Re: [HACKERS] Bad behavior from plpython 'return []'

[Jim Nasby]

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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Bad behavior from plpython 'return []'

[Tom Lane]

Robert Haas  writes:
> On Thu, Jun 30, 2016 at 9:25 PM, Jim Nasby  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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Bad behavior from plpython 'return []'

[Robert Haas]

On Thu, Jun 30, 2016 at 9:25 PM, Jim Nasby  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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Bad behavior from plpython 'return []'

[Jim Nasby]

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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers