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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: "P O'Toole" <P(dot)OToole(at)uwyo(dot)edu>, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: Weird return-value from pg_get_function_identity_arguments() on certain aggregate functions?
Date: 2018-03-18 20:07:26
Message-ID: 1041.1521403646@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

I wrote:
> "David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)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

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Tomas Vondra 2018-03-19 00:34:05 Re: BUG #15121: Multiple UBSAN errors
Previous Message PG Bug reporting form 2018-03-18 19:59:45 BUG #15121: Multiple UBSAN errors