Thread: Weird return-value from pg_get_function_identity_arguments() oncertain aggregate functions?

Hello, all.

 

I'm writing because I may have found a bug which emerged somewhere after version 9.3 and at or before 9.6.

 

While experimenting with some automation for a DBA, I found that expressions created in PLPgSQL using:

 

SELECT INTO execstring

    format(

      'GRANT EXECUTE ON FUNCTION %I.%I(%s) TO PUBLIC',

      nspname,

      proname,

      pg_get_function_identity_arguments(pg_proc.oid)

    )

    FROM

      pg_proc

      JOIN

      pg_namespace ON pronamespace = pg_namespace.oid

-- WHERE

--   more stuff

 

were failing in some cases due to syntax error in the generated SQL.

 

 

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)

 

yields

 

'double precision[] ORDER BY anyelement'

 

which leaves you with a bad expression like:

 

GRANT EXECUTE ON FUNCTION pg_catalog.percentile_disc(double precision[] ORDER BY anyelement) TO public;

 

Obviously, the above leads to a syntax error.

Version: PostgreSQL 9.6.6 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-11), 64-bit

 

 

Presumably, the approach I showed is old and no longer in use. I've found a way around needing to use pg_get_function_identity_arguments() myself. However, the behavior described above still represents a little pitfall. Is this actually a bug or do the docs perhaps need to be more clear on what pg_get_function_identity_arguments() is meant for (and possibly recommend some alternate way to generate function-signatures)?

 

Regards,

- Patrick O'Toole

 

Application Developer

Wyoming Natural Diversity Database

UW Berry Biodiversity Conservation Center

Department 3381, 1000 E. University Av.

Laramie, WY 82071

P: 307-766-3018

 

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