>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>> pg_get_function_arguments()'s interface assumes that the caller is>> providing the enclosing parens. Changing it
wouldhave meant>> returning a result like 'float8) WITHIN GROUP (float8' which I>> reckoned would have too much chance
ofbreaking existing callers.
Tom> Well, as it stands, existing callers are broken a fortiori;
Not in most cases, because using the output of
pg_get_function_arguments works in all contexts except for
constructing the CREATE AGGREGATE statement (for example, a function
that uses the pg_get_function_arguments output to construct GRANTs
or ALTER OWNERs would still work).
And since constructing a correct CREATE AGGREGATE statement for an
ordered set function requires that the caller know about the
additional options to supply in the body, this does not seem like a
restriction.
Tom> Of course, if we define the right output as being just theTom> arguments according to pg_proc, it's fine.
The patch accepts the 'just the arguments according to pg_proc' as
input everywhere except in CREATE AGGREGATE, in addition to the
syntax with explicit WITHIN GROUP.
(With the exception of GRANT, as it happens, where since there is no
existing GRANT ON AGGREGATE variant and the patch doesn't add one,
only the pg_proc arguments form is accepted.)
Tom> But your point about the parens is a good one anyway, becauseTom> now that I think about it, what \da has
traditionallyprinted is
Tom> pg_catalog | sum | bigint | integer | sum as bigint acroTom> ss all integer input values
Tom> and I see the patch makes it do this
Tom> pg_catalog | sum | bigint | (integer) | sum as bigint acro
Tom> which I cannot find to be an improvement, especially since \dfTom> does not look like that. (As patched, the
outputof "\df ran*"Tom> is positively schizophrenic.)
Since I don't particularly trust my own judgement on aesthetics, I used
the feedback I got from others when deciding what to do. Frankly I think
this one needs wider input than just you and me arguing over it.
--
Andrew (irc:RhodiumToad)
