Weird return-value from pg_get_function_identity_arguments() on certain aggregate functions?

On Mon, Mar 12, 2018 at 2:19 PM, P O'Toole <P.OToole@uwyo.edu> wrote:

According to the System Information Functions docs,
pg_get_function_identity_arguments(OID) should simply "get argument list
to identify a function (without default values)", but one example of how it
behaves strangely is that:

SELECT pg_get_function_identity_arguments('pg_catalog.percentile_disc(DOUBLE
PRECISION[], ANYELEMENT)'::REGPROCEDURE)

​FWIW a simple \dfS percentile* will elicit the same description.

I suppose it depends on what you are using the output for - the
(percentile*) functions that are decorated with ORDER BY are exclusively
aggregate, as opposed to standard, functions.

An ORDER BY is not a "default value" so we aren't really contradicting the
docs - though seeing ORDER BY in a function signature is a surprise to me
too...but reading the doc for CREATE AGGREGATE the SQL command syntax is:

CREATE AGGREGATE name ( [ [ argmode ] [ argname ] arg_data_type [ , ... ] ]
ORDER BY [ argmode ] [ argname ] arg_data_type [ ,
... ] )

So I'd have to say this is working correctly.

I seem to recall some recent (last few weeks) discussion regarding
aggregates vs functions in the information schema. That may prove to be
enlightening but at the moment I don't have time to go look for and review
it.

David J.
​

On Mon, Mar 12, 2018 at 2:47 PM, David G. Johnston <
david.g.johnston@gmail.com> wrote:

On Mon, Mar 12, 2018 at 2:19 PM, P O'Toole <P.OToole@uwyo.edu> wrote:

According to the System Information Functions docs,
pg_get_function_identity_arguments(OID) should simply "get argument list
to identify a function (without default values)", but one example of how it
behaves strangely is that:

SELECT pg_get_function_identity_arguments('pg_catalog.percentile_disc(DOUBLE
PRECISION[], ANYELEMENT)'::REGPROCEDURE)

​FWIW a simple \dfS percentile* will elicit the same description.

I suppose it depends on what you are using the output for - the
(percentile*) functions that are decorated with ORDER BY are exclusively
aggregate, as opposed to standard, functions.

In this case I'd say the supposed bug is that GRANT ON FUNCTION doesn't
accept the signature of a valid CREATE AGGREGATE​ even though our existing
implementation uses it as an implementation mechanism for both (i.e., we
don't have a separate GRANT ON AGGREGATE).

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Mon, Mar 12, 2018 at 2:19 PM, P O'Toole <P.OToole@uwyo.edu> wrote:

According to the System Information Functions docs,
pg_get_function_identity_arguments(OID) should simply "get argument list
to identify a function (without default values)", but one example of how it
behaves strangely is that:

In this case I'd say the supposed bug is that GRANT ON FUNCTION doesn't
accept the signature of a valid CREATE AGGREGATE​ even though our existing
implementation uses it as an implementation mechanism for both (i.e., we
don't have a separate GRANT ON AGGREGATE).

The entire point of pg_get_function_identity_arguments is that it's
supposed to print the arguments in the form that would be accepted by
GRANT, as well as DROP FUNCTION and some other commands, so I'd tend
to blame pg_get_function_identity_arguments. Probably the reason
nobody has noticed up to now is a dearth of user-defined ordered-set
functions. Still, we're supposed to support such things, so we
oughta fix it.

[ pokes at it ... ] Hmm, DROP AGGREGATE will let you spell it
either way, so maybe it is indeed only GRANT that's got an issue.
If so, perhaps allowing this syntax there is an attractive fix.
In theory there might be pg_dump files out there using this syntax.

regards, tom lane

I wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

In this case I'd say the supposed bug is that GRANT ON FUNCTION doesn't
accept the signature of a valid CREATE AGGREGATE​ even though our existing
implementation uses it as an implementation mechanism for both (i.e., we
don't have a separate GRANT ON AGGREGATE).

The entire point of pg_get_function_identity_arguments is that it's
supposed to print the arguments in the form that would be accepted by
GRANT, as well as DROP FUNCTION and some other commands, so I'd tend
to blame pg_get_function_identity_arguments. Probably the reason
nobody has noticed up to now is a dearth of user-defined ordered-set
functions. Still, we're supposed to support such things, so we
oughta fix it.

[ pokes at it ... ] Hmm, DROP AGGREGATE will let you spell it
either way, so maybe it is indeed only GRANT that's got an issue.
If so, perhaps allowing this syntax there is an attractive fix.
In theory there might be pg_dump files out there using this syntax.

I spent some time digging around in gram.y to see what we could do
about this. I do not think we can get it to accept

privilege_target: FUNCTION aggregate_with_argtypes

because there would be shift-reduce conflicts too soon. So my
thought above doesn't work. But we definitely could add "AGGREGATE
aggregate_with_argtypes" to the list of possible GRANT targets, and it
seems like overall that would be by far the cleanest, least confusing fix.

So far as I can find in a quick scan, GRANT/REVOKE is the only
SQL command type that accepts FUNCTION but not AGGREGATE, so
getting rid of the inconsistency seems like a good thing.
It would simplify life for pg_dump, too, which currently needs an
ugly hack to emit the GRANT ON FUNCTION syntax for an aggregate.

An objection to this is that if pg_dump starts emitting "GRANT
ON AGGREGATE", that will create a hazard for loading the output
into old server versions. We could reduce the hazard to the
minimum set of necessary cases if we used that syntax only when
dealing with an actual ordered-set aggregate, and otherwise
continued to print GRANT ON FUNCTION. But I think that's more
confusion and complication than the situation is worth. In general
we don't promise that output from pg_dump version X will load
into server versions before X.

A bigger objection is that this doesn't seem like a back-patchable
fix. But given the narrowness of the problem (i.e, user-defined
ordered-set aggregates) I think that's acceptable. What we can't
do is make GRANT/REVOKE reject naming aggregates with FUNCTION
syntax, because that'd break forward compatibility of existing
dump files.

regards, tom lane