Re: JSON Function Bike Shedding - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: JSON Function Bike Shedding
Date
Msg-id 51476AFD.5070706@dunslane.net
Whole thread Raw
In response to Re: JSON Function Bike Shedding  (Merlin Moncure <mmoncure@gmail.com>)
Responses Re: JSON Function Bike Shedding
Re: JSON Function Bike Shedding
List pgsql-hackers
On 03/01/2013 11:09 AM, Merlin Moncure wrote:
> On Fri, Feb 22, 2013 at 11:50 AM, David E. Wheeler
> <david@justatheory.com> wrote:
>> On Feb 22, 2013, at 9:37 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>
>>> What I think is NOT tolerable is choosing a set of short but arbitrary
>>> names which are different from anything that we have now and
>>> pretending that we'll want to use those again for the next data type
>>> that comes along.  That's just wishful thinking.  Programmers who
>>> believe that their decisions will act as precedent for all future code
>>> are almost inevitably disappointed.  Precedent grows organically out
>>> of what happens; it's very hard to create it ex nihilo, especially
>>> since we have no clear idea what future data types we'll likely want
>>> to add.  Sure, if we add something that's just like JSON but with a
>>> few extra features, we'll be able to reuse the names no problem.  But
>>> that's unlikely, because we typically resist the urge to add things
>>> that are too much like what we already have.  The main reason we're
>>> adding JSON when we already have hstore is because JSON has become
>>> something of a standard.  We probably WILL add more "container" types
>>> in the future, but I'd guess that they are likely to be as different
>>> from JSON as JSON is from XML, or from arrays.  I'm not convinced we
>>> can define a set of semantics that are going to sweep that broadly.
>> Maybe. I would argue, however, that a key/value-oriented data type will always call those things "keys" and
"values".So keys() and vals() (or get_keys() and get_vals()) seems pretty reasonable to me. 
>>
>> Anyway, back to practicalities, Andrew last posted:
>>
>>> I am going to go the way that involves the least amount of explicit casting or array construction. So get_path()
stays,but becomes non-variadic. get() can take an int or variadic text[], so you can do: 
>>>
>>>     get(myjson,0)
>>>     get(myjson,'f1')
>>>     get(myjson,'f1','2','f3')
>>>     get_path(myjson,'{f1,2,f3}')
>> I would change these to mention the return types:
>>
>>     get_json(myjson,0)
>>     get_json(myjson,'f1')
>>     get_json(myjson,'f1','2','f3')
>>     get_path_json(myjson,'{f1,2,f3}')
>>
>> And then the complementary text-returning versions:
>>
>>     get_text(myjson,0)
>>     get_text(myjson,'f1')
>>     get_text(myjson,'f1','2','f3')
>>     get_path_text(myjson,'{f1,2,f3}')
>>
>> I do think that something like length() has pretty good semantics across data types, though. So to update the
proposednames, taking in the discussion, I now propose: 
>>
>> Existing Name                  Proposed Name
>> --------------------------     -------------------
>> json_array_length()             length()
>> json_each()                     each_json()
>> json_each_as_text()             each_text()
>> json_get()                      get_json()
>> json_get_as_text()              get_text()
>> json_get_path()                 get_path_json()
>> json_get_path_as_text()         get_path_text()
>> json_object_keys()              get_keys()
>> json_populate_record()          to_record()
>> json_populate_recordset()       to_records()
>> json_unnest()                   get_values()
>> json_agg()                      json_agg()
>>
>> I still prefer to_record() and to_records() to populate_record(). It just feels more like a cast to me. I dislike
json_agg(),but assume we're stuck with it. 
>>
>> But at this point, I’m happy to leave Andrew to it. The functionality is awesome.
>
> Agreed: +1 to your thoughts here.  But also +1 to the originals and +1
> to Robert's point of view also.   This feature is of huge strategic
> importance to the project and we need to lock this down and commit it.
>    There is a huge difference between "i slightly prefer some different
> names" and "the feature has issues".
>
> So, i think the various positions are clear: this is one argument i'd
> be happy to lose (or win).
>




I've been sitting here for a while mulling none too happily over the
debate on the names for the proposed JSON extraction functions. I
haven't really been happy with any of the suggestions, much, not least
my own original function names which were really only intended as
placeholders. Last night in the still watches I decided I just couldn't
go with a function name as almost totally content-free as get(), or even
get_text(). And I don't think prepending "json_'" to the name helps much
either.

Just concentrating to start with on those get() functions, in the simple
case we really don't need them at all. hstore has the "->" operator
without documenting the underlying function ("fetchval"). So maybe we
should just do that. We could have documented, simply:
    myjson ->  'fname'    myjson ->  1    myjson ->> 'fname'    myjson ->> 1    myjson #>  '{fname,1}'    myjson #>>
'{fname,1}'

and leave the underlying functions undocumented.

One wrinkle in this picture is the variadic forms of extraction which
don't lend themselves nicely to use with an operator. We could decide to
do away with those altogether, or come up with a better name. I'm loath
to use "json_path" since it's a name used for something similar but
different elsewhere. I do think it's valuable to have the variadic form,
though, and I'd be sad to see it go.

Regarding the remaining functions,
 * I'd be inclined to stick with json_array_length() and   json_object_keys() - I think they describe pretty well what
theydo.   hstore's skeys() does more or less the same as json_object_keys(),   so we could use that if we want to be
consistent.I don't think it's   a terribly good name though. * json_unnest() should certainly be renamed. Alternatives
thatcome to   mind are json_unfold() or json_elements() or json_array_elements(). * json_each(), json_each_as_text(),
json_populate_record()and   json_populate_recordset() - to be consistent with hstore we could   remove the "json_". We
probablyshould remove the "_as_ from   json_each_as_text(). 


cheers

andrew



pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: Enabling Checksums
Next
From: Tom Lane
Date:
Subject: Re: Materialized view assertion failure in HEAD