Andrew Dunstan <andrew@dunslane.net> writes:
> On 09/27/2014 08:00 AM, Stephen Frost wrote:
>> Yeah, I don't see adding this option to all json generator functions as
>> making a lot of sense but rather just to the select few things which it
>> really makes sense for and then having a function which can be used by
>> users to do the same for results from other operations.
> I guess I'm questioning the wisdom of keeping it for row_to_json given
> that it doesn't operate recursively anyway (and making it do so would be
> difficult and ugly).
IMO, adding it to row_to_json was really ugly to start with, independently
of whether it's difficult or not. That function had one too many optional
arguments already, and in its current form it's nothing but a loaded gun
pointed at users' feet. (Quick, which bool argument is which?)
> If we're going to keep this, I think we also need to provide it
> (non-recursive) for json_agg via an optional second argument. This
> should be a fairly simple change: just steer the result via
> composite_to_json if it's a record, rather than to datum_to_json.
Unfortunately, any such thing will fall foul of an established project
policy. I quote the opr_sanity regression test that will complain:
-- Check that there are not aggregates with the same name and different
-- numbers of arguments. While not technically wrong, we have a project policy
-- to avoid this because it opens the door for confusion in connection with
-- ORDER BY: novices frequently put the ORDER BY in the wrong place.
-- See the fate of the single-argument form of string_agg() for history.
So my vote is for a separate function and no optional arguments.
regards, tom lane
