Re: additional json functionality

On 11/13/2013 09:45 AM, Merlin Moncure wrote:
> On Wed, Nov 13, 2013 at 1:33 AM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>> On 11/13/2013 02:34 AM, Andrew Dunstan wrote:
>>> If there's agreement on taking these, I will prepare patches and submit
>>> them by the 15th.
>> With JSON enhancement, my only concern is that there's work ongoing to
>> integrate the v2 development version of hstore with json, providing
>> typed hstore and an efficient binary storage format for json.
>>
>> It might be worth seeing how that work is going and what functionality
>> needs to be added to it, rather than enhancing the existing json support
>> that may soon change dramatically.

I'm going to be fairly upset if I'm told I have to wait for the new
format work, and then I'm later told it's too late to bring this into 9.4.

I think these are really orthogonal issues. Adding a new serialization
format (which I have been discussing with Oleg and Teodor, and which I
hope to help in bringing to JSON) will make some things lots faster than
they now are, and might make some things easier or possible where now
they are hard or impossible, but it won't remove any functionality
requirement.

In particular, json_build, which lets you build up arbitrarily complex
and irregular json in a way that's just not possible without using a PL
right now, is quite an important requirement. I've given talks about it
and JSON users have been quite excited by the possibilities it opens up.

The patch for json_to_record is quite small (two functions), and it has
the advantage that unlike the json_populate_record functions you don't
need to have or create a named type to use it. I think that makes it
worth having in itself.

> I'm not so sure we should require hstore to do things like build
> arbitrary json objects even though I agree that hstore will probably
> displace json for must cases where you want to store nested data (as
> opposed to (de-)serialize).

I have no idea what this means.

The plan with the work that Oleg and Teodor are doing is to provide a
set of common code that can be used by either a binary json
representation (which will be able to be distinguished from a text
representation, so there would be no pg_upgrade problems) or nested
hstore. In effect, nested hstore and json would have pretty much
identical capabilities, so using one ovber another should be largely a
matter of preference than a forced choice. Frankly, I believe the
audience for JSON is vastly larger, and I expect it to be the treeish
data format of choice for almost all users.

> Andrew's patches just fill out a couple
> of missing cases that are handled in the existing API. Putting all
> the patches together, ISTM there might be a function or two too many.
> I'm not sure why the json_ prefix was abandoned for build_json_object
> and build_json_array.

I'm quite happy to change it.

>
> Also, json_object is pretty weird to me, I'm not sure I see the
> advantage of a new serialization format,

What? there is no new serialization format. This is a way to generate a
json object in the existing format from a one or two dimensional array
of text. c.f. |existing function hstore(text[]) => hstore|

> and I don't agree with the
> statement "but it is the caller's reponsibility to ensure that keys
> are not repeated.". I think the caller should have no such
> responsibility. Keys should be able to repeated.

They can be repeated, as they can in the current json text format.
However, the function makes no attempt to deal with repeated keys. If a
key is repeated in the inout it will be repeated in the output. In this
respect it differs from the hstore function.

Note too, that one effect of moving to a non-text representation of json
will be that duplicated keys will be resolved (last value will win). But
that's a much wider issue that this function.

> Also, I'm not sure
> how the {k,v,k,v,k,v}...convention serialized into a string is very
> useful in general practice. I greatly prefer the aggregation and the
> variadic methods in json_build.

The extension was built before json_build. But it met a requirement that
existed at the time. It probably wouldn't be a tragedy to leave it out,
but there is probably a place for it just as there is for the hstore
function.

>
> Putting it all together, I'd consider:
> *) dropping json_object (although maybe there is a case I'm not thinking about)
> *) changing json_build function names to get the json prefix
> *) adding a json object constructor that takes two parallel arrays as
> arguments.

The third point seems to conflict with the first. I'd only consider that
if we *do* add the one-array version of json_object.

cheers

andrew