Обсуждение: Error code missing for "wrong length of inner sequence" error

In PLySequence_ToArray_recurse(), there's this check:

>     if (PySequence_Length(list) != dims[dim])
>         ereport(ERROR,
>                 (errmsg("wrong length of inner sequence: has length %d, but %d was expected",
>                         (int) PySequence_Length(list), dims[dim]),
>                  (errdetail("To construct a multidimensional array, the inner sequences must all have the same
length."))));

It's missing an error code, making it implicitly ERRCODE_INTERNAL_ERROR, 
which is surely not right. That's simple to fix, but what error code 
should we use?

I'm inclined to use ERRCODE_ARRAY_SUBSCRIPT_ERROR, because that's used 
in the similar error message with SQL ARRAY expression, like "ARRAY[[1], 
[2, 3]]". Most checks when converting between SQL and Python types use 
the PLy_elog() function, which uses the genericc 
ERRCODE_EXTERNAL_ROUTINE_EXCEPTION error code, but I think 
ERRCODE_ARRAY_SUBSCRIPT_ERROR is better.

You get this error e.g. if you try to return a python lists of lists 
from a PL/python function as a multi-dimensional array, but the sublists 
have different lengths:

create function return_array() returns int[] as $$
return [[1], [1,2]]
$$ language plpythonu;

postgres=# select return_array();
ERROR:  wrong length of inner sequence: has length 2, but 1 was expected
DETAIL:  To construct a multidimensional array, the inner sequences must 
all have the same length.
CONTEXT:  while creating return value
PL/Python function "return_array"


Thoughts? If not, I'm going to add 
errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR) to that.

- Heikki

> On 1 Oct 2020, at 12:54, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> Most checks when converting between SQL and Python types use the PLy_elog() function, which uses the genericc
ERRCODE_EXTERNAL_ROUTINE_EXCEPTIONerror code, but I think ERRCODE_ARRAY_SUBSCRIPT_ERROR is better. 

On that note, wouldn't the dimension check errors in PLySequence_ToArray be
just as well off using normal ereport()'s?  Only one of them seem to error out
in a way that could propagate an error from Python to postgres.

> Thoughts? If not, I'm going to add errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR) to that.

+1 on using ERRCODE_ARRAY_SUBSCRIPT_ERROR as it better conveys meaning.

cheers ./daniel

On 01/10/2020 14:21, Daniel Gustafsson wrote:
>> On 1 Oct 2020, at 12:54, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> 
>> Most checks when converting between SQL and Python types use the PLy_elog() function, which uses the genericc
ERRCODE_EXTERNAL_ROUTINE_EXCEPTIONerror code, but I think ERRCODE_ARRAY_SUBSCRIPT_ERROR is better.
 
> 
> On that note, wouldn't the dimension check errors in PLySequence_ToArray be
> just as well off using normal ereport()'s?  Only one of them seem to error out
> in a way that could propagate an error from Python to postgres.

Yes, you are right. I was going to say that we are not very consistent 
on when to use ereport() and when PLy_elog() in general, but looking 
closer, we are. Outside the array-conversion functions, PLy_elog() is 
only used when a Python C API call fails, and ereport() is used otherwise.

Patch attached.

- Heikki

> On 2 Oct 2020, at 13:44, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 01/10/2020 14:21, Daniel Gustafsson wrote:
>>> On 1 Oct 2020, at 12:54, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>> Most checks when converting between SQL and Python types use the PLy_elog() function, which uses the genericc
ERRCODE_EXTERNAL_ROUTINE_EXCEPTIONerror code, but I think ERRCODE_ARRAY_SUBSCRIPT_ERROR is better. 
>> On that note, wouldn't the dimension check errors in PLySequence_ToArray be
>> just as well off using normal ereport()'s?  Only one of them seem to error out
>> in a way that could propagate an error from Python to postgres.
>
> Yes, you are right. I was going to say that we are not very consistent on when to use ereport() and when PLy_elog()
ingeneral, but looking closer, we are. Outside the array-conversion functions, PLy_elog() is only used when a Python C
APIcall fails, and ereport() is used otherwise. 

I did another scan as well and couldn't find any other offenders.

> Patch attached.

LGTM.

cheers ./daniel

On 02/10/2020 15:06, Daniel Gustafsson wrote:
>> On 2 Oct 2020, at 13:44, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>> On 01/10/2020 14:21, Daniel Gustafsson wrote:
>>> On that note, wouldn't the dimension check errors in PLySequence_ToArray be
>>> just as well off using normal ereport()'s?  Only one of them seem to error out
>>> in a way that could propagate an error from Python to postgres.
>>
>> Yes, you are right. I was going to say that we are not very
>> consistent on when to use ereport() and when PLy_elog() in general,
>> but looking closer, we are. Outside the array-conversion functions,
>> PLy_elog() is only used when a Python C API call fails, and
>> ereport() is used otherwise.
> 
> I did another scan as well and couldn't find any other offenders.
Pushed. Thanks for checking!

- Heikki