Re: [PATCH] Add missing type conversion functions for PL/Python

On Wed, 31 Jan 2018 11:57:12 +0800
Haozhou Wang <hawang@pivotal.io> wrote:

> Hi All,
>
> PL/Python already has different type conversion functions to
> convert PostgreSQL datum to Python object. However, the conversion
> functions from Python object to PostgreSQL datum only has Boolean,
> Bytea and String functions.
>
> In this patch, we add rest of Integer and Float related datatype
> conversion functions
> and can increase the performance of data conversion greatly especially
> when returning a large array.
>
> We did a quick test about the performance of returning array in
> PL/Python:
>
> The following UDF is used for test:
>
> ```
> CREATE OR REPLACE FUNCTION pyoutfloat8(num int) RETURNS float8[] AS $$
> return [x/3.0 for x in range(num)]
> $$ LANGUAGE plpythonu;
> ```
>
> The test command is
>
> ```
> select count(pyoutfloat8(n));
> ```
>
> The count is used for avoid large output, where n is the number of
> element in returned array, and the size is from 1.5k to15m.
>
> Size of Array      1.5k           |            15k         |
>  150k        |           1.5m        |         15m       |
>
> Origin                 2.324ms     |     19.834ms      |     194.991ms
> |     1927.28ms    |   19982.1ms  |
>
> With this patch   1.168ms      |      3.052ms       |
> 21.888ms     | 213.39ms      |    2138.5ms   |
>
> All test for both PG and PL/Python are passed.
>
> Thanks very much.
>
>

Hello,
sounds like a really nice patch. I've started looking
through the code and noticed a sort of a typos (or I just couldn't
understand what did you mean) in comments.

file "src/pl/plpython/plpy_typeio.c"
the comment is
* If can not convert if directly, fallback to PLyObject_ToDatum
* to convert it

Maybe it should be something like ("it" instead of second "if")
* If can not convert it directly, fallback to PLyObject_ToDatum
* to convert it

And the same typo is repeated several times in comments.

--
Anthony Bykov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company