On 02/24/2013 02:09 PM, Steve Singer wrote: > Here is a review of this patch., > > Overview > --------------------- > This patch adds a set of functions to convert types to json. Specifically > > * An aggregate that take the elements and builds up a json array. > * Conversions from an hstore to json > > For non-builtin types the text conversion is used unless a cast has > specifically been defined from the type to json. > > There was some discussion last year on this patch that raised three > issues > > a) Robert was concerned that if someone added a cast from a non-core > type like citext to json that it would change the behaviour of this > function. No one else offered an opinion on this at the time. I don't > see this as a problem, if I add a cast between citext (or any other > non-core datatype) to json I would expect it to effect how that > datatype is generated as a json object in any functions that generate > json. I don't see why this would be surprising behaviour. If I add > a cast between my datatype and json to generate a json representation > that differs from the textout representation then I would expect that > representation to be used in the json_agg function as well. I'm not thrilled about that, because we're likely to want to add more JSON-specific casts to built-in or extension types in the future. If doing so changes behaviour, causing something that used to work to continue to work but produce a different result, that'll result in considerable arguments about backward compatibility.
I'd be happier to require explicit casts to text or require the user to explicitly CREATE CAST where no JSON-aware cast is already defined. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
