Thread: Variadic aggregates vs. project policy
So I was hacking away at supporting variadic aggregates (per an internal request at Salesforce), and had it pretty much working, when I came across this old comment in opr_sanity.sql: -- 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. -- The only aggregates that should show up here are count(x) and count(*). While a variadic-using aggregate doesn't actually trip the associated test query, it surely violates the spirit of this policy: if you put ORDER BY in the wrong place the parser will be unable to detect that that wasn't what you meant. For context see the thread starting here: http://www.postgresql.org/message-id/AANLkTikV5ok2tS8t6V+gsAPtE3N6TJq1JpPhMZhG2XL0@mail.gmail.com In that thread we agreed that this "policy" might be rather squishy, but we should at least think hard about whether it would be wise to create built-in aggregates with the same name and different numbers of arguments. So the question I'm now wondering about is whether this consideration makes variadic aggregates a bad idea all around, even if we don't have any built-in ones. Is the risk of user confusion (in the use of their own aggregate) sufficient reason to reject such a feature? regards, tom lane
2013/8/29 Tom Lane <tgl@sss.pgh.pa.us>
So I was hacking away at supporting variadic aggregates (per an internal
request at Salesforce), and had it pretty much working, when I came across
this old comment in opr_sanity.sql:
-- 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.
-- The only aggregates that should show up here are count(x) and count(*).
While a variadic-using aggregate doesn't actually trip the associated test
query, it surely violates the spirit of this policy: if you put ORDER BY
in the wrong place the parser will be unable to detect that that wasn't
what you meant.
For context see the thread starting here:
http://www.postgresql.org/message-id/AANLkTikV5ok2tS8t6V+gsAPtE3N6TJq1JpPhMZhG2XL0@mail.gmail.com
In that thread we agreed that this "policy" might be rather squishy,
but we should at least think hard about whether it would be wise to create
built-in aggregates with the same name and different numbers of arguments.
So the question I'm now wondering about is whether this consideration
makes variadic aggregates a bad idea all around, even if we don't have
any built-in ones. Is the risk of user confusion (in the use of their
own aggregate) sufficient reason to reject such a feature?
can be this issue solved by syntax?
In September commitfest is patch for "WITHIN GROUP" where ORDER BY clause is safety separated from parameters.
Regards
Pavel
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2013/8/29 Tom Lane <tgl@sss.pgh.pa.us>
>> So the question I'm now wondering about is whether this consideration
>> makes variadic aggregates a bad idea all around, even if we don't have
>> any built-in ones. Is the risk of user confusion (in the use of their
>> own aggregate) sufficient reason to reject such a feature?
> can be this issue solved by syntax?
> In September commitfest is patch for "WITHIN GROUP" where ORDER BY clause
> is safety separated from parameters.
That might not be the ugliest syntax the SQL committee ever invented, but
it's right up there. I don't want to go that way, especially not when the
existing precedent for the same feature with regular functions doesn't use
any weird special syntax.
On further reflection, what the "policy" was actually about was not that
we should forbid users from creating potentially-confusing aggregates
themselves, but only that we'd avoid having any *built in* aggregates with
this hazard. So maybe I'm overthinking this, and the correct reading is
just that we should have a policy against built-in variadic aggregates.
regards, tom lane
2013/8/29 Tom Lane <tgl@sss.pgh.pa.us>
>> So the question I'm now wondering about is whether this considerationThat might not be the ugliest syntax the SQL committee ever invented, but
>> makes variadic aggregates a bad idea all around, even if we don't have
>> any built-in ones. Is the risk of user confusion (in the use of their
>> own aggregate) sufficient reason to reject such a feature?
> can be this issue solved by syntax?
> In September commitfest is patch for "WITHIN GROUP" where ORDER BY clause
> is safety separated from parameters.
it's right up there. I don't want to go that way, especially not when the
existing precedent for the same feature with regular functions doesn't use
any weird special syntax.
It is maybe not nice, but it is long years supported by almost all SQL servers.
When I talked with Atri, he mentioned, so variadic aggregates are supported there too.
Regards
Pavel
Pavel
On further reflection, what the "policy" was actually about was not that
we should forbid users from creating potentially-confusing aggregates
themselves, but only that we'd avoid having any *built in* aggregates with
this hazard. So maybe I'm overthinking this, and the correct reading is
just that we should have a policy against built-in variadic aggregates.
can be this potentially strange situation identified? - and some warning can be raised.
regards, tom lane
Tom, > On further reflection, what the "policy" was actually about was not that > we should forbid users from creating potentially-confusing aggregates > themselves, but only that we'd avoid having any *built in* aggregates with > this hazard. So maybe I'm overthinking this, and the correct reading is > just that we should have a policy against built-in variadic aggregates. Yes. I think we can assume that anyone smart enough to create a variadic aggregate is smart enough to put ORDER BY in the right place. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus wrote:
> Tom,
>
> > On further reflection, what the "policy" was actually about was not that
> > we should forbid users from creating potentially-confusing aggregates
> > themselves, but only that we'd avoid having any *built in* aggregates with
> > this hazard. So maybe I'm overthinking this, and the correct reading is
> > just that we should have a policy against built-in variadic aggregates.
>
> Yes. I think we can assume that anyone smart enough to create a
> variadic aggregate is smart enough to put ORDER BY in the right place.
I don't think it's a question of "smart enough", but rather how easy it
is to make a mistake. In the referenced thread email, I'm sure many of
us looked at the failing query without realizing what the problem was.
That said, if the question is whether or not to offer variadic
aggregates if they give you the chance to misuse them in this way, the
decision seems quite clear to me. Presumably, in the problematic case
the arguments to the aggregate are going to be constructed
programatically in client code; how would an ORDER BY be accidentally
inserted by such code? If you end up having
someagg('foo','bar','baz' ORDER BY 'qux', 'blarg')
instead of someagg('foo','bar','baz', 'blarg' ORDER BY 'qux')
I think you're going to realize the problem quickly enough.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 2013-08-29 15:55:13 -0400, Tom Lane wrote: > So I was hacking away at supporting variadic aggregates (per an internal > request at Salesforce), and had it pretty much working, when I came across > this old comment in opr_sanity.sql: > > -- 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. > -- The only aggregates that should show up here are count(x) and count(*). > > While a variadic-using aggregate doesn't actually trip the associated test > query, it surely violates the spirit of this policy: if you put ORDER BY > in the wrong place the parser will be unable to detect that that wasn't > what you meant. > > For context see the thread starting here: > http://www.postgresql.org/message-id/AANLkTikV5ok2tS8t6V+gsAPtE3N6TJq1JpPhMZhG2XL0@mail.gmail.com > In that thread we agreed that this "policy" might be rather squishy, > but we should at least think hard about whether it would be wise to create > built-in aggregates with the same name and different numbers of arguments. > > So the question I'm now wondering about is whether this consideration > makes variadic aggregates a bad idea all around, even if we don't have > any built-in ones. Is the risk of user confusion (in the use of their > own aggregate) sufficient reason to reject such a feature? I vote for abolishing that policy or maybe weakinging it. As you comment somewhere downthread the policy just prohibits core functions, but even for those it looks too strong for me. There are some useful variadic aggregates I'd like to see and I don't think that the kind of errors prevented by the policy are frequent enough to warrant a blanket prohibition. I'd say we let the check in there but have a list of exceptions in it so that one has to explicitly think about the issue before adding the function. Some functions are worth the risk, others are not. E.g. the 1 argument form of string_agg() doesn't have enough benefits, but other functions might. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-08-29 15:55:13 -0400, Tom Lane wrote: >> For context see the thread starting here: >> http://www.postgresql.org/message-id/AANLkTikV5ok2tS8t6V+gsAPtE3N6TJq1JpPhMZhG2XL0@mail.gmail.com >> In that thread we agreed that this "policy" might be rather squishy, >> but we should at least think hard about whether it would be wise to create >> built-in aggregates with the same name and different numbers of arguments. > I vote for abolishing that policy or maybe weakinging it. As you comment > somewhere downthread the policy just prohibits core functions, but even > for those it looks too strong for me. There are some useful variadic > aggregates I'd like to see and I don't think that the kind of errors > prevented by the policy are frequent enough to warrant a blanket > prohibition. Well, I dunno. We had two different "bug reports" caused by this type of confusion before string_agg even got out of beta, both from intelligent people. So I'm not about to discount the potential for confusion. As we said originally, this is a policy that might be broken for sufficiently strong cause --- but I don't want to just forget about the risks. > I'd say we let the check in there but have a list of exceptions in it so > that one has to explicitly think about the issue before adding the > function. That's pretty much how the tests in opr_sanity work now. regards, tom lane
On 2013-08-29 18:29:34 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-08-29 15:55:13 -0400, Tom Lane wrote: > >> For context see the thread starting here: > >> http://www.postgresql.org/message-id/AANLkTikV5ok2tS8t6V+gsAPtE3N6TJq1JpPhMZhG2XL0@mail.gmail.com > >> In that thread we agreed that this "policy" might be rather squishy, > >> but we should at least think hard about whether it would be wise to create > >> built-in aggregates with the same name and different numbers of arguments. > > > I vote for abolishing that policy or maybe weakinging it. As you comment > > somewhere downthread the policy just prohibits core functions, but even > > for those it looks too strong for me. There are some useful variadic > > aggregates I'd like to see and I don't think that the kind of errors > > prevented by the policy are frequent enough to warrant a blanket > > prohibition. > > Well, I dunno. We had two different "bug reports" caused by this type of > confusion before string_agg even got out of beta, both from intelligent > people. So I'm not about to discount the potential for confusion. > > As we said originally, this is a policy that might be broken for > sufficiently strong cause --- but I don't want to just forget about > the risks. > > I'd say we let the check in there but have a list of exceptions in it so > > that one has to explicitly think about the issue before adding the > > function. > > That's pretty much how the tests in opr_sanity work now. I basically mean that we should adapt the paragraph you quoted upthread to roughly say something like: -- Check that there are not aggregates with the same name and different -- numbers of arguments. For many aggregates - look for string_agg in -- the archives for an example - the risk of confusing novices, which -- place ORDER BY in the wrong place, seems too big. If an aggregate is -- deemed not to be likely to cause such a problem or provides a feature -- which doesn't seem possibly to provide in another way that, add an -- excception for it. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 08/29/2013 05:37 PM, Josh Berkus wrote: > Tom, > >> On further reflection, what the "policy" was actually about was not that >> we should forbid users from creating potentially-confusing aggregates >> themselves, but only that we'd avoid having any *built in* aggregates with >> this hazard. So maybe I'm overthinking this, and the correct reading is >> just that we should have a policy against built-in variadic aggregates. > Yes. I think we can assume that anyone smart enough to create a > variadic aggregate is smart enough to put ORDER BY in the right place. > > It's not the creator who is in danger, though, it's the user of the aggregate function, AIUI. So unless you're saying that anyone smart enough to use a variadic aggregate can be assumed to be smart enough to put ORDER BY in the right place, I don't think this argument holds. cheers andrew
2013/8/30 Andrew Dunstan <andrew@dunslane.net>
It's not the creator who is in danger, though, it's the user of the aggregate function, AIUI. So unless you're saying that anyone smart enough to use a variadic aggregate can be assumed to be smart enough to put ORDER BY in the right place, I don't think this argument holds.
On 08/29/2013 05:37 PM, Josh Berkus wrote:Tom,On further reflection, what the "policy" was actually about was not thatYes. I think we can assume that anyone smart enough to create a
we should forbid users from creating potentially-confusing aggregates
themselves, but only that we'd avoid having any *built in* aggregates with
this hazard. So maybe I'm overthinking this, and the correct reading is
just that we should have a policy against built-in variadic aggregates.
variadic aggregate is smart enough to put ORDER BY in the right place.
I was one who sent a bug report - this error is not too dangerous, but it is hidden, and difficult to find, if you don't know what can be happen. Same as bug with plpgsql and SQL identifier collisions. If you understand, then you can protect self well and simply. If not, then it is a magic error. So still I am thing so best solution is
a) a warning when detect ORDER BY in variadic aggregates
b) disallow ORDER BY in variadic aggregates in classic syntax, and enable it only in WITHIN GROUP syntax where is safe , btw a aggregates that needs a ORDER BY is better to evaluate different for minimise risk of OOP killer. This is a good solution without any risks.
Regards
Pavel
Pavel
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Pavel Stehule <pavel.stehule@gmail.com> writes:
> I was one who sent a bug report - this error is not too dangerous, but it
> is hidden, and difficult to find, if you don't know what can be happen.
> Same as bug with plpgsql and SQL identifier collisions. If you understand,
> then you can protect self well and simply. If not, then it is a magic
> error. So still I am thing so best solution is
> a) a warning when detect ORDER BY in variadic aggregates
Such a warning would never be tolerated by users, because it would appear
even when the query is perfectly correct.
> b) disallow ORDER BY in variadic aggregates in classic syntax, and enable
> it only in WITHIN GROUP syntax where is safe ,
And we're *not* inventing randomly different syntax for variadic
aggregates. That ship sailed when we did it this way for regular
functions.
regards, tom lane
Tom Lane-2 wrote > Pavel Stehule < > pavel.stehule@ > > writes: >> I was one who sent a bug report - this error is not too dangerous, but it >> is hidden, and difficult to find, if you don't know what can be happen. >> Same as bug with plpgsql and SQL identifier collisions. If you >> understand, >> then you can protect self well and simply. If not, then it is a magic >> error. So still I am thing so best solution is > >> a) a warning when detect ORDER BY in variadic aggregates > > Such a warning would never be tolerated by users, because it would appear > even when the query is perfectly correct. > >> b) disallow ORDER BY in variadic aggregates in classic syntax, and enable >> it only in WITHIN GROUP syntax where is safe , > > And we're *not* inventing randomly different syntax for variadic > aggregates. That ship sailed when we did it this way for regular > functions. In the example case the problem is that ORDER BY constant is a valid, if not-very-useful, construct. Can we warn on this specific usage and thus mitigate many of the potential avenues of mis-use? If we alter syntax for mitigation purposes I'd want to consider requiring parentheses around the columns that belong to the ORDER BY instead of using the full extended syntax of WITHIN GROUP. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Variadic-aggregates-vs-project-policy-tp5768980p5769106.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
On 2013-08-30 06:34:47 -0700, David Johnston wrote: > Tom Lane-2 wrote > >> I was one who sent a bug report - this error is not too dangerous, but it > >> is hidden, and difficult to find, if you don't know what can be happen. > >> Same as bug with plpgsql and SQL identifier collisions. If you > >> understand, > >> then you can protect self well and simply. If not, then it is a magic > >> error. So still I am thing so best solution is > > > >> a) a warning when detect ORDER BY in variadic aggregates > > > > Such a warning would never be tolerated by users, because it would appear > > even when the query is perfectly correct. > > > >> b) disallow ORDER BY in variadic aggregates in classic syntax, and enable > >> it only in WITHIN GROUP syntax where is safe , > > > > And we're *not* inventing randomly different syntax for variadic > > aggregates. That ship sailed when we did it this way for regular > > functions. > > In the example case the problem is that ORDER BY constant is a valid, if > not-very-useful, construct. Can we warn on this specific usage and thus > mitigate many of the potential avenues of mis-use? That doesn't help against something like »SELECT string_agg(somecol ORDER BY bar, separator)« where separator is a column. > If we alter syntax for mitigation purposes I'd want to consider requiring > parentheses around the columns that belong to the ORDER BY instead of using > the full extended syntax of WITHIN GROUP. I think that ship has sailed. The syntax is there and it's not going away. Requiring different syntaxes for variadic/nonvariadic usages is going to be a way much bigger pitfall for users. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund-3 wrote > On 2013-08-30 06:34:47 -0700, David Johnston wrote: >> Tom Lane-2 wrote >> >> I was one who sent a bug report - this error is not too dangerous, but >> it >> >> is hidden, and difficult to find, if you don't know what can be >> happen. >> >> Same as bug with plpgsql and SQL identifier collisions. If you >> >> understand, >> >> then you can protect self well and simply. If not, then it is a magic >> >> error. So still I am thing so best solution is >> > >> >> a) a warning when detect ORDER BY in variadic aggregates >> > >> > Such a warning would never be tolerated by users, because it would >> appear >> > even when the query is perfectly correct. >> > >> >> b) disallow ORDER BY in variadic aggregates in classic syntax, and >> enable >> >> it only in WITHIN GROUP syntax where is safe , >> > >> > And we're *not* inventing randomly different syntax for variadic >> > aggregates. That ship sailed when we did it this way for regular >> > functions. >> >> In the example case the problem is that ORDER BY constant is a valid, if >> not-very-useful, construct. Can we warn on this specific usage and thus >> mitigate many of the potential avenues of mis-use? > > That doesn't help against something like »SELECT string_agg(somecol > ORDER BY bar, separator)« where separator is a column. > >> If we alter syntax for mitigation purposes I'd want to consider requiring >> parentheses around the columns that belong to the ORDER BY instead of >> using >> the full extended syntax of WITHIN GROUP. > > I think that ship has sailed. The syntax is there and it's not going > away. Requiring different syntaxes for variadic/nonvariadic usages is > going to be a way much bigger pitfall for users. Neither suggestion (nor any suggestion I would imagine) is going to "solve" the problem. The goal is to minimize the size of the exposure. For the second ORDER BY (col1, col2) suggestion it would be added and recommended so those using that syntax would have less to worry about. This would apply to ALL invocations, not just variadic. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Variadic-aggregates-vs-project-policy-tp5768980p5769119.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
2013/8/30 David Johnston <polobo@yahoo.com>
Andres Freund-3 wroteNeither suggestion (nor any suggestion I would imagine) is going to "solve"> On 2013-08-30 06:34:47 -0700, David Johnston wrote:
>> Tom Lane-2 wrote
>> >> I was one who sent a bug report - this error is not too dangerous, but
>> it
>> >> is hidden, and difficult to find, if you don't know what can be
>> happen.
>> >> Same as bug with plpgsql and SQL identifier collisions. If you
>> >> understand,
>> >> then you can protect self well and simply. If not, then it is a magic
>> >> error. So still I am thing so best solution is
>> >
>> >> a) a warning when detect ORDER BY in variadic aggregates
>> >
>> > Such a warning would never be tolerated by users, because it would
>> appear
>> > even when the query is perfectly correct.
>> >
>> >> b) disallow ORDER BY in variadic aggregates in classic syntax, and
>> enable
>> >> it only in WITHIN GROUP syntax where is safe ,
>> >
>> > And we're *not* inventing randomly different syntax for variadic
>> > aggregates. That ship sailed when we did it this way for regular
>> > functions.
>>
>> In the example case the problem is that ORDER BY constant is a valid, if
>> not-very-useful, construct. Can we warn on this specific usage and thus
>> mitigate many of the potential avenues of mis-use?
>
> That doesn't help against something like »SELECT string_agg(somecol
> ORDER BY bar, separator)« where separator is a column.
>
>> If we alter syntax for mitigation purposes I'd want to consider requiring
>> parentheses around the columns that belong to the ORDER BY instead of
>> using
>> the full extended syntax of WITHIN GROUP.
>
> I think that ship has sailed. The syntax is there and it's not going
> away. Requiring different syntaxes for variadic/nonvariadic usages is
> going to be a way much bigger pitfall for users.
the problem. The goal is to minimize the size of the exposure.
For the second ORDER BY (col1, col2) suggestion it would be added and
recommended so those using that syntax would have less to worry about. This
would apply to ALL invocations, not just variadic.
you cannot use this syntax - probably, because (col1, col2) is same as ROW(col1, col2) and this syntax is supported now. So there is a collision. I had a same idea, but I don't think so it is possible
Regards
Pavel
Pavel
David J.
--
View this message in context: http://postgresql.1045698.n5.nabble.com/Variadic-aggregates-vs-project-policy-tp5768980p5769119.htmlSent from the PostgreSQL - hackers mailing list archive at Nabble.com.--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
David Johnston <polobo@yahoo.com> writes:
>>> If we alter syntax for mitigation purposes I'd want to consider requiring
>>> parentheses around the columns that belong to the ORDER BY instead of
>>> using the full extended syntax of WITHIN GROUP.
Unfortunately, that ORDER BY syntax is specified by the SQL standard,
and they didn't say anything about parentheses. We don't get to
require parens there.
The particular case that's standardized is only array_agg():
<array aggregate function> ::= ARRAY_AGG <left paren> <value expression> [ ORDER BY <sort specification
list>] <right paren>
but, as we customarily do, we didn't restrict the feature to be used only
with that aggregate.
regards, tom lane
Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-08-29 18:29:34 -0400, Tom Lane wrote:
>> Andres Freund <andres@2ndquadrant.com> writes:
>>> I'd say we let the check in there but have a list of exceptions in it so
>>> that one has to explicitly think about the issue before adding the
>>> function.
>> That's pretty much how the tests in opr_sanity work now.
> I basically mean that we should adapt the paragraph you quoted upthread
> to roughly say something like:
Attached is a complete draft patch for this. I modified the text about
the opr_sanity test a bit (though not exactly as you suggest) and also
added some text in xaggr.sgml pointing out the hazard, so that users can
make informed decisions about whether they want to use the feature.
My basic approach was to share the existing grammar and catalog support
for declaring variadic function arguments. That made it practically free
to add support for storing argument names for aggregates, so I did that
too. However, it was and remains true that aggregates don't support
default values for arguments, nor do we support writing calls in named or
mixed parameter style for them. The same is true of window functions.
I felt that fixing those things was out of scope for this patch, but it
might be something somebody else wants to work on. (I think that fixing
this might be relatively easy for window functions --- just need to get
the planner to apply its existing code for fixing up regular FuncExprs.
But it's less than trivial for aggregates because of our use of
TargetEntry list representation for aggregate parameter lists.)
Addition of default values for aggregates would run up against the same
number-of-arguments ambiguity we were discussing for VARIADIC, but I don't
see why we shouldn't adopt the same we-won't-do-this-but-users-can stance
as for VARIADIC.
Another point is that variadic window functions could stand some love.
I think it partially works, but ruleutils.c at least doesn't know how to
reverse-list such calls correctly. Again, that seemed out of scope for
this patch.
Comments?
regards, tom lane
diff --git a/doc/src/sgml/ref/alter_aggregate.sgml b/doc/src/sgml/ref/alter_aggregate.sgml
index 571a50a..aab5b2b 100644
*** a/doc/src/sgml/ref/alter_aggregate.sgml
--- b/doc/src/sgml/ref/alter_aggregate.sgml
*************** PostgreSQL documentation
*** 21,29 ****
<refsynopsisdiv>
<synopsis>
! ALTER AGGREGATE <replaceable>name</replaceable> ( <replaceable>argtype</replaceable> [ , ... ] ) RENAME TO
<replaceable>new_name</replaceable>
! ALTER AGGREGATE <replaceable>name</replaceable> ( <replaceable>argtype</replaceable> [ , ... ] ) OWNER TO
<replaceable>new_owner</replaceable>
! ALTER AGGREGATE <replaceable>name</replaceable> ( <replaceable>argtype</replaceable> [ , ... ] ) SET SCHEMA
<replaceable>new_schema</replaceable>
</synopsis>
</refsynopsisdiv>
--- 21,32 ----
<refsynopsisdiv>
<synopsis>
! ALTER AGGREGATE <replaceable>name</replaceable> ( [ <replaceable>argmode</replaceable> ] [
<replaceable>arg_name</replaceable>] <replaceable>arg_data_type</replaceable> [ , ... ] )
! RENAME TO <replaceable>new_name</replaceable>
! ALTER AGGREGATE <replaceable>name</replaceable> ( [ <replaceable>argmode</replaceable> ] [
<replaceable>arg_name</replaceable>] <replaceable>arg_data_type</replaceable> [ , ... ] )
! OWNER TO <replaceable>new_owner</replaceable>
! ALTER AGGREGATE <replaceable>name</replaceable> ( [ <replaceable>argmode</replaceable> ] [
<replaceable>arg_name</replaceable>] <replaceable>arg_data_type</replaceable> [ , ... ] )
! SET SCHEMA <replaceable>new_schema</replaceable>
</synopsis>
</refsynopsisdiv>
*************** ALTER AGGREGATE <replaceable>name</repla
*** 62,73 ****
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">argtype</replaceable></term>
<listitem>
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of input data types.
</para>
</listitem>
</varlistentry>
--- 65,100 ----
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">argmode</replaceable></term>
!
! <listitem>
! <para>
! The mode of an argument: <literal>IN</> or <literal>VARIADIC</>.
! If omitted, the default is <literal>IN</>.
! </para>
! </listitem>
! </varlistentry>
!
! <varlistentry>
! <term><replaceable class="parameter">arg_name</replaceable></term>
!
! <listitem>
! <para>
! The name of an argument.
! Note that <command>ALTER AGGREGATE</command> does not actually pay
! any attention to argument names, since only the argument data
! types are needed to determine the aggregate function's identity.
! </para>
! </listitem>
! </varlistentry>
!
! <varlistentry>
! <term><replaceable class="parameter">arg_data_type</replaceable></term>
<listitem>
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of argument specifications.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/alter_extension.sgml b/doc/src/sgml/ref/alter_extension.sgml
index 2dbba0c..a14fcb4 100644
*** a/doc/src/sgml/ref/alter_extension.sgml
--- b/doc/src/sgml/ref/alter_extension.sgml
*************** ALTER EXTENSION <replaceable class="PARA
*** 30,36 ****
<phrase>where <replaceable class="PARAMETER">member_object</replaceable> is:</phrase>
! AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> (<replaceable
class="PARAMETER">agg_type</replaceable>[, ...] ) |
CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>) |
COLLATION <replaceable class="PARAMETER">object_name</replaceable> |
CONVERSION <replaceable class="PARAMETER">object_name</replaceable> |
--- 30,36 ----
<phrase>where <replaceable class="PARAMETER">member_object</replaceable> is:</phrase>
! AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> ( [ <replaceable
class="parameter">argmode</replaceable>] [ <replaceable class="parameter">argname</replaceable> ] <replaceable
class="parameter">agg_type</replaceable>[, ...] ) |
CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>) |
COLLATION <replaceable class="PARAMETER">object_name</replaceable> |
CONVERSION <replaceable class="PARAMETER">object_name</replaceable> |
*************** ALTER EXTENSION <replaceable class="PARA
*** 179,185 ****
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of input data types.
</para>
</listitem>
</varlistentry>
--- 179,185 ----
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of argument specifications.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index e94dd4b..e550500 100644
*** a/doc/src/sgml/ref/comment.sgml
--- b/doc/src/sgml/ref/comment.sgml
*************** PostgreSQL documentation
*** 23,29 ****
<synopsis>
COMMENT ON
{
! AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> (<replaceable
class="PARAMETER">agg_type</replaceable>[, ...] ) |
CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>) |
COLLATION <replaceable class="PARAMETER">object_name</replaceable> |
COLUMN <replaceable class="PARAMETER">relation_name</replaceable>.<replaceable
class="PARAMETER">column_name</replaceable>|
--- 23,29 ----
<synopsis>
COMMENT ON
{
! AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> ( [ <replaceable
class="parameter">argmode</replaceable>] [ <replaceable class="parameter">argname</replaceable> ] <replaceable
class="parameter">agg_type</replaceable>[, ...] ) |
CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>) |
COLLATION <replaceable class="PARAMETER">object_name</replaceable> |
COLUMN <replaceable class="PARAMETER">relation_name</replaceable>.<replaceable
class="PARAMETER">column_name</replaceable>|
*************** COMMENT ON
*** 126,132 ****
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of input data types.
</para>
</listitem>
</varlistentry>
--- 126,132 ----
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of argument specifications.
</para>
</listitem>
</varlistentry>
*************** COMMENT ON
*** 156,162 ****
The mode of a function argument: <literal>IN</>, <literal>OUT</>,
<literal>INOUT</>, or <literal>VARIADIC</>.
If omitted, the default is <literal>IN</>.
! Note that <command>COMMENT ON FUNCTION</command> does not actually pay
any attention to <literal>OUT</> arguments, since only the input
arguments are needed to determine the function's identity.
So it is sufficient to list the <literal>IN</>, <literal>INOUT</>,
--- 156,162 ----
The mode of a function argument: <literal>IN</>, <literal>OUT</>,
<literal>INOUT</>, or <literal>VARIADIC</>.
If omitted, the default is <literal>IN</>.
! Note that <command>COMMENT</command> does not actually pay
any attention to <literal>OUT</> arguments, since only the input
arguments are needed to determine the function's identity.
So it is sufficient to list the <literal>IN</>, <literal>INOUT</>,
*************** COMMENT ON
*** 170,176 ****
<listitem>
<para>
The name of a function argument.
! Note that <command>COMMENT ON FUNCTION</command> does not actually pay
any attention to argument names, since only the argument data
types are needed to determine the function's identity.
</para>
--- 170,176 ----
<listitem>
<para>
The name of a function argument.
! Note that <command>COMMENT</command> does not actually pay
any attention to argument names, since only the argument data
types are needed to determine the function's identity.
</para>
diff --git a/doc/src/sgml/ref/create_aggregate.sgml b/doc/src/sgml/ref/create_aggregate.sgml
index d5e4e27..2b35fa4 100644
*** a/doc/src/sgml/ref/create_aggregate.sgml
--- b/doc/src/sgml/ref/create_aggregate.sgml
*************** PostgreSQL documentation
*** 21,27 ****
<refsynopsisdiv>
<synopsis>
! CREATE AGGREGATE <replaceable class="PARAMETER">name</replaceable> ( <replaceable
class="PARAMETER">input_data_type</replaceable>[ , ... ] ) (
SFUNC = <replaceable class="PARAMETER">sfunc</replaceable>,
STYPE = <replaceable class="PARAMETER">state_data_type</replaceable>
[ , FINALFUNC = <replaceable class="PARAMETER">ffunc</replaceable> ]
--- 21,27 ----
<refsynopsisdiv>
<synopsis>
! CREATE AGGREGATE <replaceable class="parameter">name</replaceable> ( [ <replaceable
class="parameter">argmode</replaceable>] [ <replaceable class="parameter">arg_name</replaceable> ] <replaceable
class="parameter">arg_data_type</replaceable>[ , ... ] ) (
SFUNC = <replaceable class="PARAMETER">sfunc</replaceable>,
STYPE = <replaceable class="PARAMETER">state_data_type</replaceable>
[ , FINALFUNC = <replaceable class="PARAMETER">ffunc</replaceable> ]
*************** CREATE AGGREGATE <replaceable class="PAR
*** 118,124 ****
Note that this behavior is only available when
<replaceable class="PARAMETER">state_data_type</replaceable>
is the same as the first
! <replaceable class="PARAMETER">input_data_type</replaceable>.
When these types are different, you must supply a nonnull initial
condition or use a nonstrict transition function.
</para>
--- 118,124 ----
Note that this behavior is only available when
<replaceable class="PARAMETER">state_data_type</replaceable>
is the same as the first
! <replaceable class="PARAMETER">arg_data_type</replaceable>.
When these types are different, you must supply a nonnull initial
condition or use a nonstrict transition function.
</para>
*************** SELECT col FROM tab ORDER BY col USING s
*** 187,198 ****
</varlistentry>
<varlistentry>
! <term><replaceable class="PARAMETER">input_data_type</replaceable></term>
<listitem>
<para>
An input data type on which this aggregate function operates.
To create a zero-argument aggregate function, write <literal>*</>
! in place of the list of input data types. (An example of such an
aggregate is <function>count(*)</function>.)
</para>
</listitem>
--- 187,222 ----
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">argmode</replaceable></term>
!
! <listitem>
! <para>
! The mode of an argument: <literal>IN</> or <literal>VARIADIC</>.
! (Aggregate functions do not support <literal>OUT</> arguments.)
! If omitted, the default is <literal>IN</>. Only the last argument
! can be marked <literal>VARIADIC</>.
! </para>
! </listitem>
! </varlistentry>
!
! <varlistentry>
! <term><replaceable class="parameter">arg_name</replaceable></term>
!
! <listitem>
! <para>
! The name of an argument. This is currently only useful for
! documentation purposes. If omitted, the argument has no name.
! </para>
! </listitem>
! </varlistentry>
!
! <varlistentry>
! <term><replaceable class="PARAMETER">arg_data_type</replaceable></term>
<listitem>
<para>
An input data type on which this aggregate function operates.
To create a zero-argument aggregate function, write <literal>*</>
! in place of the list of argument specifications. (An example of such an
aggregate is <function>count(*)</function>.)
</para>
</listitem>
*************** SELECT col FROM tab ORDER BY col USING s
*** 205,212 ****
In the old syntax for <command>CREATE AGGREGATE</>, the input data type
is specified by a <literal>basetype</> parameter rather than being
written next to the aggregate name. Note that this syntax allows
! only one input parameter. To define a zero-argument aggregate function,
! specify the <literal>basetype</> as
<literal>"ANY"</> (not <literal>*</>).
</para>
</listitem>
--- 229,236 ----
In the old syntax for <command>CREATE AGGREGATE</>, the input data type
is specified by a <literal>basetype</> parameter rather than being
written next to the aggregate name. Note that this syntax allows
! only one input parameter. To define a zero-argument aggregate function
! with this syntax, specify the <literal>basetype</> as
<literal>"ANY"</> (not <literal>*</>).
</para>
</listitem>
diff --git a/doc/src/sgml/ref/drop_aggregate.sgml b/doc/src/sgml/ref/drop_aggregate.sgml
index 1ed152f..06060fb 100644
*** a/doc/src/sgml/ref/drop_aggregate.sgml
--- b/doc/src/sgml/ref/drop_aggregate.sgml
*************** PostgreSQL documentation
*** 21,27 ****
<refsynopsisdiv>
<synopsis>
! DROP AGGREGATE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> ( <replaceable
class="PARAMETER">argtype</replaceable>[ , ... ] ) [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
--- 21,29 ----
<refsynopsisdiv>
<synopsis>
! DROP AGGREGATE [ IF EXISTS ]
! <replaceable class="parameter">name</replaceable> ( [ <replaceable class="parameter">argmode</replaceable> ] [
<replaceableclass="parameter">arg_name</replaceable> ] <replaceable class="parameter">arg_data_type</replaceable> [ ,
...] )
! [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
*************** DROP AGGREGATE [ IF EXISTS ] <replaceabl
*** 60,71 ****
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">argtype</replaceable></term>
<listitem>
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of input data types.
</para>
</listitem>
</varlistentry>
--- 62,97 ----
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">argmode</replaceable></term>
!
! <listitem>
! <para>
! The mode of an argument: <literal>IN</> or <literal>VARIADIC</>.
! If omitted, the default is <literal>IN</>.
! </para>
! </listitem>
! </varlistentry>
!
! <varlistentry>
! <term><replaceable class="parameter">arg_name</replaceable></term>
!
! <listitem>
! <para>
! The name of an argument.
! Note that <command>DROP AGGREGATE</command> does not actually pay
! any attention to argument names, since only the argument data
! types are needed to determine the aggregate function's identity.
! </para>
! </listitem>
! </varlistentry>
!
! <varlistentry>
! <term><replaceable class="parameter">arg_data_type</replaceable></term>
<listitem>
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of argument specifications.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/security_label.sgml b/doc/src/sgml/ref/security_label.sgml
index 52cb1d1..76c131f 100644
*** a/doc/src/sgml/ref/security_label.sgml
--- b/doc/src/sgml/ref/security_label.sgml
*************** SECURITY LABEL [ FOR <replaceable class=
*** 25,31 ****
{
TABLE <replaceable class="PARAMETER">object_name</replaceable> |
COLUMN <replaceable class="PARAMETER">table_name</replaceable>.<replaceable
class="PARAMETER">column_name</replaceable>|
! AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> (<replaceable
class="PARAMETER">agg_type</replaceable>[, ...] ) |
DATABASE <replaceable class="PARAMETER">object_name</replaceable> |
DOMAIN <replaceable class="PARAMETER">object_name</replaceable> |
EVENT TRIGGER <replaceable class="PARAMETER">object_name</replaceable> |
--- 25,31 ----
{
TABLE <replaceable class="PARAMETER">object_name</replaceable> |
COLUMN <replaceable class="PARAMETER">table_name</replaceable>.<replaceable
class="PARAMETER">column_name</replaceable>|
! AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> ( [ <replaceable
class="parameter">argmode</replaceable>] [ <replaceable class="parameter">argname</replaceable> ] <replaceable
class="parameter">agg_type</replaceable>[, ...] ) |
DATABASE <replaceable class="PARAMETER">object_name</replaceable> |
DOMAIN <replaceable class="PARAMETER">object_name</replaceable> |
EVENT TRIGGER <replaceable class="PARAMETER">object_name</replaceable> |
*************** SECURITY LABEL [ FOR <replaceable class=
*** 107,118 ****
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">arg_type</replaceable></term>
<listitem>
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of input data types.
</para>
</listitem>
</varlistentry>
--- 107,118 ----
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">agg_type</replaceable></term>
<listitem>
<para>
An input data type on which the aggregate function operates.
To reference a zero-argument aggregate function, write <literal>*</>
! in place of the list of argument specifications.
</para>
</listitem>
</varlistentry>
*************** SECURITY LABEL [ FOR <replaceable class=
*** 125,131 ****
The mode of a function argument: <literal>IN</>, <literal>OUT</>,
<literal>INOUT</>, or <literal>VARIADIC</>.
If omitted, the default is <literal>IN</>.
! Note that <command>SECURITY LABEL ON FUNCTION</command> does not actually
pay any attention to <literal>OUT</> arguments, since only the input
arguments are needed to determine the function's identity.
So it is sufficient to list the <literal>IN</>, <literal>INOUT</>,
--- 125,131 ----
The mode of a function argument: <literal>IN</>, <literal>OUT</>,
<literal>INOUT</>, or <literal>VARIADIC</>.
If omitted, the default is <literal>IN</>.
! Note that <command>SECURITY LABEL</command> does not actually
pay any attention to <literal>OUT</> arguments, since only the input
arguments are needed to determine the function's identity.
So it is sufficient to list the <literal>IN</>, <literal>INOUT</>,
*************** SECURITY LABEL [ FOR <replaceable class=
*** 140,146 ****
<listitem>
<para>
The name of a function argument.
! Note that <command>SECURITY LABEL ON FUNCTION</command> does not actually
pay any attention to argument names, since only the argument data
types are needed to determine the function's identity.
</para>
--- 140,146 ----
<listitem>
<para>
The name of a function argument.
! Note that <command>SECURITY LABEL</command> does not actually
pay any attention to argument names, since only the argument data
types are needed to determine the function's identity.
</para>
diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml
index 803ed85..e3dbc4b 100644
*** a/doc/src/sgml/syntax.sgml
--- b/doc/src/sgml/syntax.sgml
*************** SELECT concat_lower_or_upper('Hello', 'W
*** 2524,2529 ****
--- 2524,2536 ----
having numerous parameters that have default values, named or mixed
notation can save a great deal of writing and reduce chances for error.
</para>
+
+ <note>
+ <para>
+ Named and mixed call notations can currently be used only with regular
+ functions, not with aggregate functions or window functions.
+ </para>
+ </note>
</sect2>
</sect1>
diff --git a/doc/src/sgml/xaggr.sgml b/doc/src/sgml/xaggr.sgml
index 1822f6d..9ed7d99 100644
*** a/doc/src/sgml/xaggr.sgml
--- b/doc/src/sgml/xaggr.sgml
*************** SELECT attrelid::regclass, array_accum(a
*** 170,175 ****
--- 170,211 ----
</para>
<para>
+ An aggregate function can be made to accept a varying number of arguments
+ by declaring its last argument as a <literal>VARIADIC</> array, in much
+ the same fashion as for regular functions; see
+ <xref linkend="xfunc-sql-variadic-functions">. The aggregate's transition
+ function must have the same array type as its last argument. The
+ transition function typically would also be marked <literal>VARIADIC</>,
+ but this is not strictly required.
+ </para>
+
+ <note>
+ <para>
+ Variadic aggregates are easily misused in connection with
+ the <literal>ORDER BY</> option (see <xref linkend="syntax-aggregates">),
+ since the parser cannot tell whether the wrong number of actual arguments
+ have been given in such a combination. Keep in mind that everything to
+ the right of <literal>ORDER BY</> is a sort key, not an argument to the
+ aggregate. For example, in
+ <programlisting>
+ SELECT myaggregate(a ORDER BY a, b, c) FROM ...
+ </programlisting>
+ the parser will see this as a single aggregate function argument and
+ three sort keys. However, the user might have intended
+ <programlisting>
+ SELECT myaggregate(a, b, c ORDER BY a) FROM ...
+ </programlisting>
+ If <literal>myaggregate</> is variadic, both these calls could be
+ perfectly valid.
+ </para>
+
+ <para>
+ For the same reason, it's wise to think twice before creating aggregate
+ functions with the same names and different numbers of regular arguments.
+ </para>
+ </note>
+
+ <para>
A function written in C can detect that it is being called as an
aggregate transition or final function by calling
<function>AggCheckCallContext</>, for example:
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index 480c17c..d9e961e 100644
*** a/src/backend/catalog/pg_aggregate.c
--- b/src/backend/catalog/pg_aggregate.c
*************** static Oid lookup_agg_function(List *fnN
*** 45,52 ****
Oid
AggregateCreate(const char *aggName,
Oid aggNamespace,
- Oid *aggArgTypes,
int numArgs,
List *aggtransfnName,
List *aggfinalfnName,
List *aggsortopName,
--- 45,56 ----
Oid
AggregateCreate(const char *aggName,
Oid aggNamespace,
int numArgs,
+ oidvector *parameterTypes,
+ Datum allParameterTypes,
+ Datum parameterModes,
+ Datum parameterNames,
+ List *parameterDefaults,
List *aggtransfnName,
List *aggfinalfnName,
List *aggsortopName,
*************** AggregateCreate(const char *aggName,
*** 61,66 ****
--- 65,71 ----
Oid transfn;
Oid finalfn = InvalidOid; /* can be omitted */
Oid sortop = InvalidOid; /* can be omitted */
+ Oid *aggArgTypes = parameterTypes->values;
bool hasPolyArg;
bool hasInternalArg;
Oid rettype;
*************** AggregateCreate(const char *aggName,
*** 244,255 ****
false, /* isStrict (not needed for agg) */
PROVOLATILE_IMMUTABLE, /* volatility (not
* needed for agg) */
! buildoidvector(aggArgTypes,
! numArgs), /* paramTypes */
! PointerGetDatum(NULL), /* allParamTypes */
! PointerGetDatum(NULL), /* parameterModes */
! PointerGetDatum(NULL), /* parameterNames */
! NIL, /* parameterDefaults */
PointerGetDatum(NULL), /* proconfig */
1, /* procost */
0); /* prorows */
--- 249,259 ----
false, /* isStrict (not needed for agg) */
PROVOLATILE_IMMUTABLE, /* volatility (not
* needed for agg) */
! parameterTypes, /* paramTypes */
! allParameterTypes, /* allParamTypes */
! parameterModes, /* parameterModes */
! parameterNames, /* parameterNames */
! parameterDefaults, /* parameterDefaults */
PointerGetDatum(NULL), /* proconfig */
1, /* procost */
0); /* prorows */
diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c
index 4a03786..78af092 100644
*** a/src/backend/commands/aggregatecmds.c
--- b/src/backend/commands/aggregatecmds.c
***************
*** 45,54 ****
*
* "oldstyle" signals the old (pre-8.2) style where the aggregate input type
* is specified by a BASETYPE element in the parameters. Otherwise,
! * "args" defines the input type(s).
*/
Oid
! DefineAggregate(List *name, List *args, bool oldstyle, List *parameters)
{
char *aggName;
Oid aggNamespace;
--- 45,56 ----
*
* "oldstyle" signals the old (pre-8.2) style where the aggregate input type
* is specified by a BASETYPE element in the parameters. Otherwise,
! * "args" is a list of FunctionParameter structs defining the agg's arguments.
! * "parameters" is a list of DefElem representing the agg's definition clauses.
*/
Oid
! DefineAggregate(List *name, List *args, bool oldstyle, List *parameters,
! const char *queryString)
{
char *aggName;
Oid aggNamespace;
*************** DefineAggregate(List *name, List *args,
*** 59,66 ****
TypeName *baseType = NULL;
TypeName *transType = NULL;
char *initval = NULL;
- Oid *aggArgTypes;
int numArgs;
Oid transTypeId;
char transTypeType;
ListCell *pl;
--- 61,72 ----
TypeName *baseType = NULL;
TypeName *transType = NULL;
char *initval = NULL;
int numArgs;
+ oidvector *parameterTypes;
+ ArrayType *allParameterTypes;
+ ArrayType *parameterModes;
+ ArrayType *parameterNames;
+ List *parameterDefaults;
Oid transTypeId;
char transTypeType;
ListCell *pl;
*************** DefineAggregate(List *name, List *args,
*** 131,136 ****
--- 137,144 ----
* Historically we allowed the command to look like basetype = 'ANY'
* so we must do a case-insensitive comparison for the name ANY. Ugh.
*/
+ Oid aggArgTypes[1];
+
if (baseType == NULL)
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
*************** DefineAggregate(List *name, List *args,
*** 139,160 ****
if (pg_strcasecmp(TypeNameToString(baseType), "ANY") == 0)
{
numArgs = 0;
! aggArgTypes = NULL;
}
else
{
numArgs = 1;
- aggArgTypes = (Oid *) palloc(sizeof(Oid));
aggArgTypes[0] = typenameTypeId(NULL, baseType);
}
}
else
{
/*
! * New style: args is a list of TypeNames (possibly zero of 'em).
*/
! ListCell *lc;
! int i = 0;
if (baseType != NULL)
ereport(ERROR,
--- 147,172 ----
if (pg_strcasecmp(TypeNameToString(baseType), "ANY") == 0)
{
numArgs = 0;
! aggArgTypes[0] = InvalidOid;
}
else
{
numArgs = 1;
aggArgTypes[0] = typenameTypeId(NULL, baseType);
}
+ parameterTypes = buildoidvector(aggArgTypes, numArgs);
+ allParameterTypes = NULL;
+ parameterModes = NULL;
+ parameterNames = NULL;
+ parameterDefaults = NIL;
}
else
{
/*
! * New style: args is a list of FunctionParameters (possibly zero of
! * 'em). We share functioncmds.c's code for processing them.
*/
! Oid requiredResultType;
if (baseType != NULL)
ereport(ERROR,
*************** DefineAggregate(List *name, List *args,
*** 162,174 ****
errmsg("basetype is redundant with aggregate input type specification")));
numArgs = list_length(args);
! aggArgTypes = (Oid *) palloc(sizeof(Oid) * numArgs);
! foreach(lc, args)
! {
! TypeName *curTypeName = (TypeName *) lfirst(lc);
!
! aggArgTypes[i++] = typenameTypeId(NULL, curTypeName);
! }
}
/*
--- 174,193 ----
errmsg("basetype is redundant with aggregate input type specification")));
numArgs = list_length(args);
! interpret_function_parameter_list(args,
! InvalidOid,
! true, /* is an aggregate */
! queryString,
! ¶meterTypes,
! &allParameterTypes,
! ¶meterModes,
! ¶meterNames,
! ¶meterDefaults,
! &requiredResultType);
! /* Parameter defaults are not currently allowed by the grammar */
! Assert(parameterDefaults == NIL);
! /* There shouldn't have been any OUT parameters, either */
! Assert(requiredResultType == InvalidOid);
}
/*
*************** DefineAggregate(List *name, List *args,
*** 219,226 ****
*/
return AggregateCreate(aggName, /* aggregate name */
aggNamespace, /* namespace */
- aggArgTypes, /* input data type(s) */
numArgs,
transfuncName, /* step function name */
finalfuncName, /* final function name */
sortoperatorName, /* sort operator name */
--- 238,249 ----
*/
return AggregateCreate(aggName, /* aggregate name */
aggNamespace, /* namespace */
numArgs,
+ parameterTypes,
+ PointerGetDatum(allParameterTypes),
+ PointerGetDatum(parameterModes),
+ PointerGetDatum(parameterNames),
+ parameterDefaults,
transfuncName, /* step function name */
finalfuncName, /* final function name */
sortoperatorName, /* sort operator name */
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0a9facf..ca754b4 100644
*** a/src/backend/commands/functioncmds.c
--- b/src/backend/commands/functioncmds.c
*************** compute_return_type(TypeName *returnType
*** 88,94 ****
typtup = LookupTypeName(NULL, returnType, NULL);
-
if (typtup)
{
if (!((Form_pg_type) GETSTRUCT(typtup))->typisdefined)
--- 88,93 ----
*************** compute_return_type(TypeName *returnType
*** 158,179 ****
}
/*
! * Interpret the parameter list of the CREATE FUNCTION statement.
*
* Results are stored into output parameters. parameterTypes must always
* be created, but the other arrays are set to NULL if not needed.
* requiredResultType is set to InvalidOid if there are no OUT parameters,
* else it is set to the OID of the implied result type.
*/
! static void
! examine_parameter_list(List *parameters, Oid languageOid,
! const char *queryString,
! oidvector **parameterTypes,
! ArrayType **allParameterTypes,
! ArrayType **parameterModes,
! ArrayType **parameterNames,
! List **parameterDefaults,
! Oid *requiredResultType)
{
int parameterCount = list_length(parameters);
Oid *inTypes;
--- 157,187 ----
}
/*
! * Interpret the function parameter list of a CREATE FUNCTION or
! * CREATE AGGREGATE statement.
! *
! * Input parameters:
! * parameters: list of FunctionParameter structs
! * languageOid: OID of function language (InvalidOid if it's CREATE AGGREGATE)
! * is_aggregate: needed only to determine error handling
! * queryString: likewise, needed only for error handling
*
* Results are stored into output parameters. parameterTypes must always
* be created, but the other arrays are set to NULL if not needed.
* requiredResultType is set to InvalidOid if there are no OUT parameters,
* else it is set to the OID of the implied result type.
*/
! void
! interpret_function_parameter_list(List *parameters,
! Oid languageOid,
! bool is_aggregate,
! const char *queryString,
! oidvector **parameterTypes,
! ArrayType **allParameterTypes,
! ArrayType **parameterModes,
! ArrayType **parameterNames,
! List **parameterDefaults,
! Oid *requiredResultType)
{
int parameterCount = list_length(parameters);
Oid *inTypes;
*************** examine_parameter_list(List *parameters,
*** 223,228 ****
--- 231,242 ----
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("SQL function cannot accept shell type %s",
TypeNameToString(t))));
+ /* We don't allow creating aggregates on shell types either */
+ else if (is_aggregate)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+ errmsg("aggregate cannot accept shell type %s",
+ TypeNameToString(t))));
else
ereport(NOTICE,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
*************** examine_parameter_list(List *parameters,
*** 246,254 ****
aclcheck_error_type(aclresult, toid);
if (t->setof)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
! errmsg("functions cannot accept set arguments")));
/* handle input parameters */
if (fp->mode != FUNC_PARAM_OUT && fp->mode != FUNC_PARAM_TABLE)
--- 260,275 ----
aclcheck_error_type(aclresult, toid);
if (t->setof)
! {
! if (is_aggregate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
! errmsg("aggregates cannot accept set arguments")));
! else
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
! errmsg("functions cannot accept set arguments")));
! }
/* handle input parameters */
if (fp->mode != FUNC_PARAM_OUT && fp->mode != FUNC_PARAM_TABLE)
*************** CreateFunction(CreateFunctionStmt *stmt,
*** 890,902 ****
* Convert remaining parameters of CREATE to form wanted by
* ProcedureCreate.
*/
! examine_parameter_list(stmt->parameters, languageOid, queryString,
! ¶meterTypes,
! &allParameterTypes,
! ¶meterModes,
! ¶meterNames,
! ¶meterDefaults,
! &requiredResultType);
if (stmt->returnType)
{
--- 911,926 ----
* Convert remaining parameters of CREATE to form wanted by
* ProcedureCreate.
*/
! interpret_function_parameter_list(stmt->parameters,
! languageOid,
! false, /* not an aggregate */
! queryString,
! ¶meterTypes,
! &allParameterTypes,
! ¶meterModes,
! ¶meterNames,
! ¶meterDefaults,
! &requiredResultType);
if (stmt->returnType)
{
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 7a0c254..e02a6ff 100644
*** a/src/backend/executor/nodeAgg.c
--- b/src/backend/executor/nodeAgg.c
*************** ExecInitAgg(Agg *node, EState *estate, i
*** 1696,1701 ****
--- 1696,1702 ----
/* build expression trees using actual argument & result types */
build_aggregate_fnexprs(inputTypes,
numArguments,
+ aggref->aggvariadic,
aggtranstype,
aggref->aggtype,
aggref->inputcollid,
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index bbc5336..544ba98 100644
*** a/src/backend/executor/nodeWindowAgg.c
--- b/src/backend/executor/nodeWindowAgg.c
*************** initialize_peragg(WindowAggState *winsta
*** 1817,1822 ****
--- 1817,1823 ----
/* build expression trees using actual argument & result types */
build_aggregate_fnexprs(inputTypes,
numArguments,
+ false, /* no variadic window functions yet */
aggtranstype,
wfunc->wintype,
wfunc->inputcollid,
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 788907e..65f3b98 100644
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyAggref(const Aggref *from)
*** 1141,1146 ****
--- 1141,1147 ----
COPY_NODE_FIELD(aggdistinct);
COPY_NODE_FIELD(aggfilter);
COPY_SCALAR_FIELD(aggstar);
+ COPY_SCALAR_FIELD(aggvariadic);
COPY_SCALAR_FIELD(agglevelsup);
COPY_LOCATION_FIELD(location);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 496e31d..4c9b05e 100644
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalAggref(const Aggref *a, const Aggr
*** 198,203 ****
--- 198,204 ----
COMPARE_NODE_FIELD(aggdistinct);
COMPARE_NODE_FIELD(aggfilter);
COMPARE_SCALAR_FIELD(aggstar);
+ COMPARE_SCALAR_FIELD(aggvariadic);
COMPARE_SCALAR_FIELD(agglevelsup);
COMPARE_LOCATION_FIELD(location);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index cff4734..a2903f9 100644
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
*************** _outAggref(StringInfo str, const Aggref
*** 962,967 ****
--- 962,968 ----
WRITE_NODE_FIELD(aggdistinct);
WRITE_NODE_FIELD(aggfilter);
WRITE_BOOL_FIELD(aggstar);
+ WRITE_BOOL_FIELD(aggvariadic);
WRITE_UINT_FIELD(agglevelsup);
WRITE_LOCATION_FIELD(location);
}
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index aad63e5..d325bb3 100644
*** a/src/backend/nodes/readfuncs.c
--- b/src/backend/nodes/readfuncs.c
*************** _readAggref(void)
*** 497,502 ****
--- 497,503 ----
READ_NODE_FIELD(aggdistinct);
READ_NODE_FIELD(aggfilter);
READ_BOOL_FIELD(aggstar);
+ READ_BOOL_FIELD(aggvariadic);
READ_UINT_FIELD(agglevelsup);
READ_LOCATION_FIELD(location);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 22e82ba..a9812af 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** static Node *makeRecursiveViewSelect(cha
*** 324,331 ****
reloptions opt_reloptions
OptWith opt_distinct opt_definition func_args func_args_list
func_args_with_defaults func_args_with_defaults_list
func_as createfunc_opt_list alterfunc_opt_list
! aggr_args old_aggr_definition old_aggr_list
oper_argtypes RuleActionList RuleActionMulti
opt_column_list columnList opt_name_list
sort_clause opt_sort_clause sortby_list index_params
--- 324,332 ----
reloptions opt_reloptions
OptWith opt_distinct opt_definition func_args func_args_list
func_args_with_defaults func_args_with_defaults_list
+ aggr_args aggr_args_list
func_as createfunc_opt_list alterfunc_opt_list
! old_aggr_definition old_aggr_list
oper_argtypes RuleActionList RuleActionMulti
opt_column_list columnList opt_name_list
sort_clause opt_sort_clause sortby_list index_params
*************** static Node *makeRecursiveViewSelect(cha
*** 352,358 ****
%type <into> into_clause create_as_target create_mv_target
%type <defelt> createfunc_opt_item common_func_opt_item dostmt_opt_item
! %type <fun_param> func_arg func_arg_with_default table_func_column
%type <fun_param_mode> arg_class
%type <typnam> func_return func_type
--- 353,359 ----
%type <into> into_clause create_as_target create_mv_target
%type <defelt> createfunc_opt_item common_func_opt_item dostmt_opt_item
! %type <fun_param> func_arg func_arg_with_default table_func_column aggr_arg
%type <fun_param_mode> arg_class
%type <typnam> func_return func_type
*************** AlterExtensionContentsStmt:
*** 3659,3665 ****
n->action = $4;
n->objtype = OBJECT_AGGREGATE;
n->objname = $6;
! n->objargs = $7;
$$ = (Node *)n;
}
| ALTER EXTENSION name add_drop CAST '(' Typename AS Typename ')'
--- 3660,3666 ----
n->action = $4;
n->objtype = OBJECT_AGGREGATE;
n->objname = $6;
! n->objargs = extractArgTypes($7);
$$ = (Node *)n;
}
| ALTER EXTENSION name add_drop CAST '(' Typename AS Typename ')'
*************** def_arg: func_type { $$ = (Node *)$
*** 4760,4769 ****
| Sconst { $$ = (Node *)makeString($1); }
;
- aggr_args: '(' type_list ')' { $$ = $2; }
- | '(' '*' ')' { $$ = NIL; }
- ;
-
old_aggr_definition: '(' old_aggr_list ')' { $$ = $2; }
;
--- 4761,4766 ----
*************** CommentStmt:
*** 5242,5248 ****
CommentStmt *n = makeNode(CommentStmt);
n->objtype = OBJECT_AGGREGATE;
n->objname = $4;
! n->objargs = $5;
n->comment = $7;
$$ = (Node *) n;
}
--- 5239,5245 ----
CommentStmt *n = makeNode(CommentStmt);
n->objtype = OBJECT_AGGREGATE;
n->objname = $4;
! n->objargs = extractArgTypes($5);
n->comment = $7;
$$ = (Node *) n;
}
*************** SecLabelStmt:
*** 5408,5414 ****
n->provider = $3;
n->objtype = OBJECT_AGGREGATE;
n->objname = $6;
! n->objargs = $7;
n->label = $9;
$$ = (Node *) n;
}
--- 5405,5411 ----
n->provider = $3;
n->objtype = OBJECT_AGGREGATE;
n->objname = $6;
! n->objargs = extractArgTypes($7);
n->label = $9;
$$ = (Node *) n;
}
*************** func_arg_with_default:
*** 6395,6400 ****
--- 6392,6419 ----
}
;
+ /* Aggregate args can be most things that function args can be */
+ aggr_arg: func_arg
+ {
+ if (!($1->mode == FUNC_PARAM_IN ||
+ $1->mode == FUNC_PARAM_VARIADIC))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("aggregates cannot have output arguments"),
+ parser_errposition(@1)));
+ $$ = $1;
+ }
+ ;
+
+ /* Zero-argument aggregates are named with * for consistency with COUNT(*) */
+ aggr_args: '(' aggr_args_list ')' { $$ = $2; }
+ | '(' '*' ')' { $$ = NIL; }
+ ;
+
+ aggr_args_list:
+ aggr_arg { $$ = list_make1($1); }
+ | aggr_args_list ',' aggr_arg { $$ = lappend($1, $3); }
+ ;
createfunc_opt_list:
/* Must be at least one to prevent conflict */
*************** RemoveAggrStmt:
*** 6594,6600 ****
DropStmt *n = makeNode(DropStmt);
n->removeType = OBJECT_AGGREGATE;
n->objects = list_make1($3);
! n->arguments = list_make1($4);
n->behavior = $5;
n->missing_ok = false;
n->concurrent = false;
--- 6613,6619 ----
DropStmt *n = makeNode(DropStmt);
n->removeType = OBJECT_AGGREGATE;
n->objects = list_make1($3);
! n->arguments = list_make1(extractArgTypes($4));
n->behavior = $5;
n->missing_ok = false;
n->concurrent = false;
*************** RemoveAggrStmt:
*** 6605,6611 ****
DropStmt *n = makeNode(DropStmt);
n->removeType = OBJECT_AGGREGATE;
n->objects = list_make1($5);
! n->arguments = list_make1($6);
n->behavior = $7;
n->missing_ok = true;
n->concurrent = false;
--- 6624,6630 ----
DropStmt *n = makeNode(DropStmt);
n->removeType = OBJECT_AGGREGATE;
n->objects = list_make1($5);
! n->arguments = list_make1(extractArgTypes($6));
n->behavior = $7;
n->missing_ok = true;
n->concurrent = false;
*************** RenameStmt: ALTER AGGREGATE func_name ag
*** 6821,6827 ****
RenameStmt *n = makeNode(RenameStmt);
n->renameType = OBJECT_AGGREGATE;
n->object = $3;
! n->objarg = $4;
n->newname = $7;
n->missing_ok = false;
$$ = (Node *)n;
--- 6840,6846 ----
RenameStmt *n = makeNode(RenameStmt);
n->renameType = OBJECT_AGGREGATE;
n->object = $3;
! n->objarg = extractArgTypes($4);
n->newname = $7;
n->missing_ok = false;
$$ = (Node *)n;
*************** AlterObjectSchemaStmt:
*** 7295,7301 ****
AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt);
n->objectType = OBJECT_AGGREGATE;
n->object = $3;
! n->objarg = $4;
n->newschema = $7;
n->missing_ok = false;
$$ = (Node *)n;
--- 7314,7320 ----
AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt);
n->objectType = OBJECT_AGGREGATE;
n->object = $3;
! n->objarg = extractArgTypes($4);
n->newschema = $7;
n->missing_ok = false;
$$ = (Node *)n;
*************** AlterOwnerStmt: ALTER AGGREGATE func_nam
*** 7524,7530 ****
AlterOwnerStmt *n = makeNode(AlterOwnerStmt);
n->objectType = OBJECT_AGGREGATE;
n->object = $3;
! n->objarg = $4;
n->newowner = $7;
$$ = (Node *)n;
}
--- 7543,7549 ----
AlterOwnerStmt *n = makeNode(AlterOwnerStmt);
n->objectType = OBJECT_AGGREGATE;
n->object = $3;
! n->objarg = extractArgTypes($4);
n->newowner = $7;
$$ = (Node *)n;
}
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 4e4e1cd..98cb58a 100644
*** a/src/backend/parser/parse_agg.c
--- b/src/backend/parser/parse_agg.c
*************** check_ungrouped_columns_walker(Node *nod
*** 965,970 ****
--- 965,971 ----
void
build_aggregate_fnexprs(Oid *agg_input_types,
int agg_num_inputs,
+ bool agg_variadic,
Oid agg_state_type,
Oid agg_result_type,
Oid agg_input_collation,
*************** build_aggregate_fnexprs(Oid *agg_input_t
*** 975,980 ****
--- 976,982 ----
{
Param *argp;
List *args;
+ FuncExpr *fexpr;
int i;
/*
*************** build_aggregate_fnexprs(Oid *agg_input_t
*** 1005,1016 ****
args = lappend(args, argp);
}
! *transfnexpr = (Expr *) makeFuncExpr(transfn_oid,
! agg_state_type,
! args,
! InvalidOid,
! agg_input_collation,
! COERCE_EXPLICIT_CALL);
/* see if we have a final function */
if (!OidIsValid(finalfn_oid))
--- 1007,1020 ----
args = lappend(args, argp);
}
! fexpr = makeFuncExpr(transfn_oid,
! agg_state_type,
! args,
! InvalidOid,
! agg_input_collation,
! COERCE_EXPLICIT_CALL);
! fexpr->funcvariadic = agg_variadic;
! *transfnexpr = (Expr *) fexpr;
/* see if we have a final function */
if (!OidIsValid(finalfn_oid))
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 1f02c9a..2bd24c8 100644
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 385,391 ****
}
/*
! * When function is called an explicit VARIADIC labeled parameter,
* and the declared_arg_type is "any", then sanity check the actual
* parameter type now - it must be an array.
*/
--- 385,391 ----
}
/*
! * When function is called with an explicit VARIADIC labeled parameter,
* and the declared_arg_type is "any", then sanity check the actual
* parameter type now - it must be an array.
*/
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 425,432 ****
aggref->aggtype = rettype;
/* aggcollid and inputcollid will be set by parse_collate.c */
/* args, aggorder, aggdistinct will be set by transformAggregateCall */
- aggref->aggstar = agg_star;
aggref->aggfilter = agg_filter;
/* agglevelsup will be set by transformAggregateCall */
aggref->location = location;
--- 425,433 ----
aggref->aggtype = rettype;
/* aggcollid and inputcollid will be set by parse_collate.c */
/* args, aggorder, aggdistinct will be set by transformAggregateCall */
aggref->aggfilter = agg_filter;
+ aggref->aggstar = agg_star;
+ aggref->aggvariadic = func_variadic;
/* agglevelsup will be set by transformAggregateCall */
aggref->location = location;
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 448,457 ****
parser_errposition(pstate, location)));
/*
! * Currently it's not possible to define an aggregate with named
! * arguments, so this case should be impossible. Check anyway because
! * the planner and executor wouldn't cope with NamedArgExprs in an
! * Aggref node.
*/
if (argnames != NIL)
ereport(ERROR,
--- 449,461 ----
parser_errposition(pstate, location)));
/*
! * We might want to support named arguments later, but disallow it for
! * now. We'd need to figure out the parsed representation (should the
! * NamedArgExprs go above or below the TargetEntry nodes?) and then
! * teach the planner to reorder the list properly. Or maybe we could
! * make transformAggregateCall do that? However, if you'd also like
! * to allow default arguments for aggregates, we'd need to do it in
! * planning to avoid semantic problems.
*/
if (argnames != NIL)
ereport(ERROR,
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c940897..b1023c4 100644
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
*************** ProcessUtilitySlow(Node *parsetree,
*** 1103,1109 ****
{
case OBJECT_AGGREGATE:
DefineAggregate(stmt->defnames, stmt->args,
! stmt->oldstyle, stmt->definition);
break;
case OBJECT_OPERATOR:
Assert(stmt->args == NIL);
--- 1103,1110 ----
{
case OBJECT_AGGREGATE:
DefineAggregate(stmt->defnames, stmt->args,
! stmt->oldstyle, stmt->definition,
! queryString);
break;
case OBJECT_OPERATOR:
Assert(stmt->args == NIL);
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 2b005d6..37d66e1 100644
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
*************** get_agg_expr(Aggref *aggref, deparse_con
*** 7405,7410 ****
--- 7405,7411 ----
Oid argtypes[FUNC_MAX_ARGS];
List *arglist;
int nargs;
+ bool use_variadic;
ListCell *l;
/* Extract the regular arguments, ignoring resjunk stuff for the moment */
*************** get_agg_expr(Aggref *aggref, deparse_con
*** 7430,7442 ****
appendStringInfo(buf, "%s(%s",
generate_function_name(aggref->aggfnoid, nargs,
NIL, argtypes,
! false, NULL),
(aggref->aggdistinct != NIL) ? "DISTINCT " : "");
/* aggstar can be set only in zero-argument aggregates */
if (aggref->aggstar)
appendStringInfoChar(buf, '*');
else
! get_rule_expr((Node *) arglist, context, true);
if (aggref->aggorder != NIL)
{
appendStringInfoString(buf, " ORDER BY ");
--- 7431,7456 ----
appendStringInfo(buf, "%s(%s",
generate_function_name(aggref->aggfnoid, nargs,
NIL, argtypes,
! aggref->aggvariadic,
! &use_variadic),
(aggref->aggdistinct != NIL) ? "DISTINCT " : "");
+
/* aggstar can be set only in zero-argument aggregates */
if (aggref->aggstar)
appendStringInfoChar(buf, '*');
else
! {
! nargs = 0;
! foreach(l, arglist)
! {
! if (nargs++ > 0)
! appendStringInfoString(buf, ", ");
! if (use_variadic && lnext(l) == NULL)
! appendStringInfoString(buf, "VARIADIC ");
! get_rule_expr((Node *) lfirst(l), context, true);
! }
! }
!
if (aggref->aggorder != NIL)
{
appendStringInfoString(buf, " ORDER BY ");
*************** generate_relation_name(Oid relid, List *
*** 8581,8587 ****
* types. (Those matter because of ambiguous-function resolution rules.)
*
* If we're dealing with a potentially variadic function (in practice, this
! * means a FuncExpr and not some other way of calling the function), then
* was_variadic must specify whether VARIADIC appeared in the original call,
* and *use_variadic_p will be set to indicate whether to print VARIADIC in
* the output. For non-FuncExpr cases, was_variadic should be FALSE and
--- 8595,8601 ----
* types. (Those matter because of ambiguous-function resolution rules.)
*
* If we're dealing with a potentially variadic function (in practice, this
! * means a FuncExpr or Aggref, not some other way of calling a function), then
* was_variadic must specify whether VARIADIC appeared in the original call,
* and *use_variadic_p will be set to indicate whether to print VARIADIC in
* the output. For non-FuncExpr cases, was_variadic should be FALSE and
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e1ef55f..57320cc 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** static void getTableData(TableInfo *tbli
*** 229,235 ****
static void makeTableDataInfo(TableInfo *tbinfo, bool oids);
static void buildMatViewRefreshDependencies(Archive *fout);
static void getTableDataFKConstraints(void);
! static char *format_function_arguments(FuncInfo *finfo, char *funcargs);
static char *format_function_arguments_old(Archive *fout,
FuncInfo *finfo, int nallargs,
char **allargtypes,
--- 229,236 ----
static void makeTableDataInfo(TableInfo *tbinfo, bool oids);
static void buildMatViewRefreshDependencies(Archive *fout);
static void getTableDataFKConstraints(void);
! static char *format_function_arguments(FuncInfo *finfo, char *funcargs,
! bool is_agg);
static char *format_function_arguments_old(Archive *fout,
FuncInfo *finfo, int nallargs,
char **allargtypes,
*************** dumpProcLang(Archive *fout, ProcLangInfo
*** 9365,9379 ****
* format_function_arguments: generate function name and argument list
*
* This is used when we can rely on pg_get_function_arguments to format
! * the argument list.
*/
static char *
! format_function_arguments(FuncInfo *finfo, char *funcargs)
{
PQExpBufferData fn;
initPQExpBuffer(&fn);
! appendPQExpBuffer(&fn, "%s(%s)", fmtId(finfo->dobj.name), funcargs);
return fn.data;
}
--- 9366,9385 ----
* format_function_arguments: generate function name and argument list
*
* This is used when we can rely on pg_get_function_arguments to format
! * the argument list. Note, however, that pg_get_function_arguments
! * does not special-case zero-argument aggregates.
*/
static char *
! format_function_arguments(FuncInfo *finfo, char *funcargs, bool is_agg)
{
PQExpBufferData fn;
initPQExpBuffer(&fn);
! appendPQExpBuffer(&fn, "%s", fmtId(finfo->dobj.name));
! if (is_agg && finfo->nargs == 0)
! appendPQExpBuffer(&fn, "(*)");
! else
! appendPQExpBuffer(&fn, "(%s)", funcargs);
return fn.data;
}
*************** dumpFunc(Archive *fout, FuncInfo *finfo)
*** 9804,9811 ****
if (funcargs)
{
/* 8.4 or later; we rely on server-side code for most of the work */
! funcfullsig = format_function_arguments(finfo, funcargs);
! funcsig = format_function_arguments(finfo, funciargs);
}
else
{
--- 9810,9817 ----
if (funcargs)
{
/* 8.4 or later; we rely on server-side code for most of the work */
! funcfullsig = format_function_arguments(finfo, funcargs, false);
! funcsig = format_function_arguments(finfo, funciargs, false);
}
else
{
*************** dumpAgg(Archive *fout, AggInfo *agginfo)
*** 11405,11411 ****
PQExpBuffer delq;
PQExpBuffer labelq;
PQExpBuffer details;
! char *aggsig;
char *aggsig_tag;
PGresult *res;
int i_aggtransfn;
--- 11411,11418 ----
PQExpBuffer delq;
PQExpBuffer labelq;
PQExpBuffer details;
! char *aggsig; /* identity signature */
! char *aggfullsig; /* full signature */
char *aggsig_tag;
PGresult *res;
int i_aggtransfn;
*************** dumpAgg(Archive *fout, AggInfo *agginfo)
*** 11435,11452 ****
selectSourceSchema(fout, agginfo->aggfn.dobj.namespace->dobj.name);
/* Get aggregate-specific details */
! if (fout->remoteVersion >= 80100)
{
appendPQExpBuffer(query, "SELECT aggtransfn, "
"aggfinalfn, aggtranstype::pg_catalog.regtype, "
"aggsortop::pg_catalog.regoperator, "
"agginitval, "
! "'t'::boolean AS convertok "
"FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
"WHERE a.aggfnoid = p.oid "
"AND p.oid = '%u'::pg_catalog.oid",
agginfo->aggfn.dobj.catId.oid);
}
else if (fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query, "SELECT aggtransfn, "
--- 11442,11473 ----
selectSourceSchema(fout, agginfo->aggfn.dobj.namespace->dobj.name);
/* Get aggregate-specific details */
! if (fout->remoteVersion >= 80400)
{
appendPQExpBuffer(query, "SELECT aggtransfn, "
"aggfinalfn, aggtranstype::pg_catalog.regtype, "
"aggsortop::pg_catalog.regoperator, "
"agginitval, "
! "'t'::boolean AS convertok, "
! "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
! "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs "
"FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
"WHERE a.aggfnoid = p.oid "
"AND p.oid = '%u'::pg_catalog.oid",
agginfo->aggfn.dobj.catId.oid);
}
+ else if (fout->remoteVersion >= 80100)
+ {
+ appendPQExpBuffer(query, "SELECT aggtransfn, "
+ "aggfinalfn, aggtranstype::pg_catalog.regtype, "
+ "aggsortop::pg_catalog.regoperator, "
+ "agginitval, "
+ "'t'::boolean AS convertok "
+ "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
+ "WHERE a.aggfnoid = p.oid "
+ "AND p.oid = '%u'::pg_catalog.oid",
+ agginfo->aggfn.dobj.catId.oid);
+ }
else if (fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query, "SELECT aggtransfn, "
*************** dumpAgg(Archive *fout, AggInfo *agginfo)
*** 11499,11505 ****
agginitval = PQgetvalue(res, 0, i_agginitval);
convertok = (PQgetvalue(res, 0, i_convertok)[0] == 't');
! aggsig = format_aggregate_signature(agginfo, fout, true);
aggsig_tag = format_aggregate_signature(agginfo, fout, false);
if (!convertok)
--- 11520,11543 ----
agginitval = PQgetvalue(res, 0, i_agginitval);
convertok = (PQgetvalue(res, 0, i_convertok)[0] == 't');
! if (fout->remoteVersion >= 80400)
! {
! /* 8.4 or later; we rely on server-side code for most of the work */
! char *funcargs;
! char *funciargs;
!
! funcargs = PQgetvalue(res, 0, PQfnumber(res, "funcargs"));
! funciargs = PQgetvalue(res, 0, PQfnumber(res, "funciargs"));
! aggfullsig = format_function_arguments(&agginfo->aggfn, funcargs, true);
! aggsig = format_function_arguments(&agginfo->aggfn, funciargs, true);
! }
! else
! {
! /* pre-8.4, do it ourselves */
! aggsig = format_aggregate_signature(agginfo, fout, true);
! aggfullsig = aggsig;
! }
!
aggsig_tag = format_aggregate_signature(agginfo, fout, false);
if (!convertok)
*************** dumpAgg(Archive *fout, AggInfo *agginfo)
*** 11559,11565 ****
aggsig);
appendPQExpBuffer(q, "CREATE AGGREGATE %s (\n%s\n);\n",
! aggsig, details->data);
appendPQExpBuffer(labelq, "AGGREGATE %s", aggsig);
--- 11597,11603 ----
aggsig);
appendPQExpBuffer(q, "CREATE AGGREGATE %s (\n%s\n);\n",
! aggfullsig, details->data);
appendPQExpBuffer(labelq, "AGGREGATE %s", aggsig);
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index dad1d5a..ed1c5fd 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeAggregates(const char *pattern,
*** 72,78 ****
gettext_noop("Name"),
gettext_noop("Result data type"));
! if (pset.sversion >= 80200)
appendPQExpBuffer(&buf,
" CASE WHEN p.pronargs = 0\n"
" THEN CAST('*' AS pg_catalog.text)\n"
--- 72,85 ----
gettext_noop("Name"),
gettext_noop("Result data type"));
! if (pset.sversion >= 80400)
! appendPQExpBuffer(&buf,
! " CASE WHEN p.pronargs = 0\n"
! " THEN CAST('*' AS pg_catalog.text)\n"
! " ELSE pg_catalog.pg_get_function_arguments(p.oid)\n"
! " END AS \"%s\",\n",
! gettext_noop("Argument data types"));
! else if (pset.sversion >= 80200)
appendPQExpBuffer(&buf,
" CASE WHEN p.pronargs = 0\n"
" THEN CAST('*' AS pg_catalog.text)\n"
diff --git a/src/include/catalog/pg_aggregate.h b/src/include/catalog/pg_aggregate.h
index 6fb10a9..5ad6ea6 100644
*** a/src/include/catalog/pg_aggregate.h
--- b/src/include/catalog/pg_aggregate.h
*************** DATA(insert ( 3175 json_agg_transfn json
*** 240,247 ****
*/
extern Oid AggregateCreate(const char *aggName,
Oid aggNamespace,
- Oid *aggArgTypes,
int numArgs,
List *aggtransfnName,
List *aggfinalfnName,
List *aggsortopName,
--- 240,251 ----
*/
extern Oid AggregateCreate(const char *aggName,
Oid aggNamespace,
int numArgs,
+ oidvector *parameterTypes,
+ Datum allParameterTypes,
+ Datum parameterModes,
+ Datum parameterNames,
+ List *parameterDefaults,
List *aggtransfnName,
List *aggfinalfnName,
List *aggsortopName,
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index fa9f41f..836c99e 100644
*** a/src/include/commands/defrem.h
--- b/src/include/commands/defrem.h
***************
*** 15,20 ****
--- 15,21 ----
#define DEFREM_H
#include "nodes/parsenodes.h"
+ #include "utils/array.h"
/* commands/dropcmds.c */
extern void RemoveObjects(DropStmt *stmt);
*************** extern void IsThereFunctionInNamespace(c
*** 53,58 ****
--- 54,69 ----
oidvector *proargtypes, Oid nspOid);
extern void ExecuteDoStmt(DoStmt *stmt);
extern Oid get_cast_oid(Oid sourcetypeid, Oid targettypeid, bool missing_ok);
+ extern void interpret_function_parameter_list(List *parameters,
+ Oid languageOid,
+ bool is_aggregate,
+ const char *queryString,
+ oidvector **parameterTypes,
+ ArrayType **allParameterTypes,
+ ArrayType **parameterModes,
+ ArrayType **parameterNames,
+ List **parameterDefaults,
+ Oid *requiredResultType);
/* commands/operatorcmds.c */
extern Oid DefineOperator(List *names, List *parameters);
*************** extern void RemoveOperatorById(Oid operO
*** 60,66 ****
/* commands/aggregatecmds.c */
extern Oid DefineAggregate(List *name, List *args, bool oldstyle,
! List *parameters);
/* commands/opclasscmds.c */
extern Oid DefineOpClass(CreateOpClassStmt *stmt);
--- 71,77 ----
/* commands/aggregatecmds.c */
extern Oid DefineAggregate(List *name, List *args, bool oldstyle,
! List *parameters, const char *queryString);
/* commands/opclasscmds.c */
extern Oid DefineOpClass(CreateOpClassStmt *stmt);
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index a778951..7918537 100644
*** a/src/include/nodes/primnodes.h
--- b/src/include/nodes/primnodes.h
*************** typedef struct Aggref
*** 249,254 ****
--- 249,255 ----
List *aggdistinct; /* DISTINCT (list of SortGroupClause) */
Expr *aggfilter; /* FILTER expression */
bool aggstar; /* TRUE if argument list was really '*' */
+ bool aggvariadic; /* TRUE if VARIADIC was used in call */
Index agglevelsup; /* > 0 if agg belongs to outer query */
int location; /* token location, or -1 if unknown */
} Aggref;
diff --git a/src/include/parser/parse_agg.h b/src/include/parser/parse_agg.h
index 8fa0ca7..b6d9dd3 100644
*** a/src/include/parser/parse_agg.h
--- b/src/include/parser/parse_agg.h
*************** extern void parseCheckAggregates(ParseSt
*** 25,30 ****
--- 25,31 ----
extern void build_aggregate_fnexprs(Oid *agg_input_types,
int agg_num_inputs,
+ bool agg_variadic,
Oid agg_state_type,
Oid agg_result_type,
Oid agg_input_collation,
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 7fa9005..1af79e5 100644
*** a/src/test/regress/expected/aggregates.out
--- b/src/test/regress/expected/aggregates.out
*************** select aggfns(distinct a,b,c order by a,
*** 1249,1251 ****
--- 1249,1264 ----
{"(2,2,bar)","(3,1,baz)"}
(1 row)
+ -- variadic aggregates
+ select least_agg(q1,q2) from int8_tbl;
+ least_agg
+ -------------------
+ -4567890123456789
+ (1 row)
+
+ select least_agg(variadic array[q1,q2]) from int8_tbl;
+ least_agg
+ -------------------
+ -4567890123456789
+ (1 row)
+
diff --git a/src/test/regress/expected/create_aggregate.out b/src/test/regress/expected/create_aggregate.out
index ad14594..6c7566f 100644
*** a/src/test/regress/expected/create_aggregate.out
--- b/src/test/regress/expected/create_aggregate.out
*************** create aggregate aggfns(integer,integer,
*** 59,61 ****
--- 59,68 ----
sfunc = aggfns_trans, stype = aggtype[],
initcond = '{}'
);
+ -- variadic aggregate
+ create function least_accum(anyelement, variadic anyarray)
+ returns anyelement language sql as
+ 'select least($1, min($2[i])) from generate_subscripts($2,1) g(i)';
+ create aggregate least_agg(variadic items anyarray) (
+ stype = anyelement, sfunc = least_accum
+ );
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 256b719..515cd9d 100644
*** a/src/test/regress/expected/opr_sanity.out
--- b/src/test/regress/expected/opr_sanity.out
*************** ORDER BY 1, 2;
*** 843,848 ****
--- 843,850 ----
-- 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.
+ -- (Note: we don't forbid users from creating such aggregates; the policy is
+ -- just to think twice before creating built-in aggregates like this.)
-- The only aggregates that should show up here are count(x) and count(*).
SELECT p1.oid::regprocedure, p2.oid::regprocedure
FROM pg_proc AS p1, pg_proc AS p2
*************** ORDER BY 1;
*** 855,861 ****
count("any") | count()
(1 row)
! -- For the same reason, aggregates with default arguments are no good.
SELECT oid, proname
FROM pg_proc AS p
WHERE proisagg AND proargdefaults IS NOT NULL;
--- 857,871 ----
count("any") | count()
(1 row)
! -- For the same reason, we avoid creating built-in variadic aggregates.
! SELECT oid, proname
! FROM pg_proc AS p
! WHERE proisagg AND provariadic != 0;
! oid | proname
! -----+---------
! (0 rows)
!
! -- For the same reason, built-in aggregates with default arguments are no good.
SELECT oid, proname
FROM pg_proc AS p
WHERE proisagg AND proargdefaults IS NOT NULL;
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 5c0196f..397edff 100644
*** a/src/test/regress/sql/aggregates.sql
--- b/src/test/regress/sql/aggregates.sql
*************** select sum(unique1) FILTER (WHERE
*** 480,482 ****
--- 480,486 ----
select aggfns(distinct a,b,c order by a,c using ~<~,b) filter (where a > 1)
from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
generate_series(1,2) i;
+
+ -- variadic aggregates
+ select least_agg(q1,q2) from int8_tbl;
+ select least_agg(variadic array[q1,q2]) from int8_tbl;
diff --git a/src/test/regress/sql/create_aggregate.sql b/src/test/regress/sql/create_aggregate.sql
index 84f9a4f..3c7d330 100644
*** a/src/test/regress/sql/create_aggregate.sql
--- b/src/test/regress/sql/create_aggregate.sql
*************** create aggregate aggfns(integer,integer,
*** 71,73 ****
--- 71,82 ----
sfunc = aggfns_trans, stype = aggtype[],
initcond = '{}'
);
+
+ -- variadic aggregate
+ create function least_accum(anyelement, variadic anyarray)
+ returns anyelement language sql as
+ 'select least($1, min($2[i])) from generate_subscripts($2,1) g(i)';
+
+ create aggregate least_agg(variadic items anyarray) (
+ stype = anyelement, sfunc = least_accum
+ );
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index a3be0c1..efcd70f 100644
*** a/src/test/regress/sql/opr_sanity.sql
--- b/src/test/regress/sql/opr_sanity.sql
*************** ORDER BY 1, 2;
*** 674,679 ****
--- 674,681 ----
-- 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.
+ -- (Note: we don't forbid users from creating such aggregates; the policy is
+ -- just to think twice before creating built-in aggregates like this.)
-- The only aggregates that should show up here are count(x) and count(*).
SELECT p1.oid::regprocedure, p2.oid::regprocedure
*************** WHERE p1.oid < p2.oid AND p1.proname = p
*** 683,689 ****
array_dims(p1.proargtypes) != array_dims(p2.proargtypes)
ORDER BY 1;
! -- For the same reason, aggregates with default arguments are no good.
SELECT oid, proname
FROM pg_proc AS p
--- 685,697 ----
array_dims(p1.proargtypes) != array_dims(p2.proargtypes)
ORDER BY 1;
! -- For the same reason, we avoid creating built-in variadic aggregates.
!
! SELECT oid, proname
! FROM pg_proc AS p
! WHERE proisagg AND provariadic != 0;
!
! -- For the same reason, built-in aggregates with default arguments are no good.
SELECT oid, proname
FROM pg_proc AS p
Uh, the pg_dump part checks for version 80400, shouldn't it be 90400? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Uh, the pg_dump part checks for version 80400, shouldn't it be 90400?
The reasoning there is that 8.4 is where we added
pg_get_function_arguments(), so this dumping code should work against that
server version or later. (Oh, memo to self: test that.) It's true that
pre-9.4 servers are not going to produce any useful extra info by using
pg_get_function_arguments() on an aggregate; but the general idea of this
patch is to make the aggregate-related code look more like the
plain-function-related code, so using the same version cutoffs in both
cases seemed to make sense.
regards, tom lane