Re: Jsonb transform for pl/python

Поиск
Список
Период
Сортировка
От Aleksander Alekseev
Тема Re: Jsonb transform for pl/python
Дата
Msg-id 20171120134835.GA9855@e733.localdomain
обсуждение исходный текст
Ответ на Re: Jsonb transform for pl/python  (Anthony Bykov <a.bykov@postgrespro.ru>)
Ответы Re: Jsonb transform for pl/python  (Michael Paquier <michael.paquier@gmail.com>)
Re: Jsonb transform for pl/python  (Anthony Bykov <a.bykov@postgrespro.ru>)
Список pgsql-hackers
Hi Anthony,

> thank you for your review. I took your comments into account in the
> third version of the patch. In the new version, I've added all the
> tests you asked for. The interesting thing is that:
> 1. set or any other non-jsonb-transformable object is transformed into
> string and added to jsonb as a string.

Well frankly I very much doubt that this:

```
+-- set -> jsonb
+CREATE FUNCTION test1set() RETURNS jsonb
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+x = set()
+x.add(1)
+x.add("string")
+x.add(None)
+return x
+$$;
+SELECT test1set();
+          test1set
+----------------------------
+ "set([1, 'string', None])"
+(1 row)
```

... is an expected and valid behavior. If user tries to transform a
set() to JSONB this is most likely a mistake since there is no standard
representation of a set() in JSONB. I believe we should rise an error in
this case instead of generating a string. Besides user can expect that
such string can be transformed back to set() which doesn't sound like a
good idea either.

If necessary, user can just transform a set() to a list():

```
>>> x = set([1,2,3,4])
>>> x
{1, 2, 3, 4}
>>> list(x)
[1, 2, 3, 4]
```

BTW I just recalled that Python supports complex numbers out-of-the box
and that range and xrange are a separate types too:

```
>>> 1 + 2j
(1+2j)
>>> range(3)
range(0, 3)
>>> type(range(3))
<class 'range'>
```

I think we should add all this to the tests as well. Naturally complex
numbers can't be represented in JSON so we should rise an error if user
tries to transform a complex number to JSON. I'm not that sure regarding
ranges though. Probably the best solution will be to rise and error in
this case as well just to keep things consistent.

> +ERROR:  jsonb doesn't support inf type yet

I would say this error message is too informal. How about something more
like "Infinity can't be represented in JSONB"?

> 2. couldn't find a solution of working with "inf": this extension
> troughs exception if python generates a number called "inf" and returns
> it, but if we pass a very large integer into a function, it works fine
> with the whole number. This situation can be seen in tests.
>
> I've added tests of large numerics which weights quite heavy. So,
> please find it in compressed format in attachments.

I'm afraid that tests fail on Python 3:

``` SELECT test1set();        test1set         -----------------------
!  "{None, 1, 'string'}" (1 row)  DROP EXTENSION plpython3u CASCADE;
--- 296,302 ---- SELECT test1set();        test1set         -----------------------
!  "{1, None, 'string'}" (1 row)  DROP EXTENSION plpython3u CASCADE
```

--
Best regards,
Aleksander Alekseev

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Kapila
Дата:
Сообщение: Re: [HACKERS] A GUC to prevent leader processes from running subplans?
Следующее
От: Alik Khilazhev
Дата:
Сообщение: Re: [HACKERS] [WIP] Zipfian distribution in pgbench