Re: json api WIP patch

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: json api WIP patch
Date: 2013-01-14 16:32:49
Message-ID: CA+TgmoaSvdKOt=uysy8bAWjigsh4iLp+bgJZLcorcFZTdimSbQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 10, 2013 at 6:42 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> Udated patch that contains most of the functionality I'm after. One piece
>> left is populate_recordset (populate a set of records from a single json
>> datum which is an array of objects, in one pass). That requires a bit of
>> thought.
>>
>> I hope most of the whitespace issues are fixed.
>
>
> This updated patch contains all the intended functionality, including
> operators for the json_get_path functions, so you can say things like
>
> select jsonval->array['f1','0','f2] ...
>
> It also removes any requirement to copy the json value before setting up the
> lexer by removing the lexer's requirement to have a nul terminated string.
> Instead the lexer is told the input length and relies on that. For this
> reason, json_in() now calls cstring_get_text() before rather than after
> calling the validation routine, but that's really not something worth
> bothering about.
>
> A couple of points worth noting: it's a pity that we have to run CREATE OR
> REPLACE FUNCTION in system_views.sql in order to set up default values for
> builtin functions. That feels very kludgy. Also, making operators for
> variadic functions is a bit of a pain. I had to set up non-variadic version
> of the same functions (see json_get_path_op and json_get_path_as_text_op)
> just so I could set up the operators. Neither of these are exactly
> showstopper items, just mild annoyances.
>
> I will continue hunting memory leaks, but when Merlin gets done with docco I
> think we'll be far enough advanced to add this to the commitfest.

So, how much performance does this lose on json_in() on a large
cstring, as compared with master?

I can't shake the feeling that this is adding a LOT of unnecessary
data copying. For one thing, instead of copying every single lexeme
(including the single-character ones?) out of the original object, we
could just store a pointer to the offset where the object starts and a
length, instead of copying it.

This is also remarkably thin on comments.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-01-14 16:45:01 Re: [PERFORM] Slow query: bitmap scan troubles
Previous Message Stephen Frost 2013-01-14 16:28:30 Re: [PATCH] COPY .. COMPRESSED