On 03/01/2013 11:09 AM, Merlin Moncure wrote:
On Fri, Feb 22, 2013 at 11:50 AM, David E. Wheeler
<da...@justatheory.com> wrote:
On Feb 22, 2013, at 9:37 AM, Robert Haas <robertmh...@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 proposed names, 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 they do.
   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 that come 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 probably should remove the "_as_ from
   json_each_as_text().


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to