From 0d3f4406dfa00d848711fdb4af53be663ffc7d0f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 3 Sep 2013 17:08:38 -0400 Subject: [PATCH] Allow aggregate functions to be VARIADIC. There's no inherent reason why an aggregate function can't be variadic (even VARIADIC ANY) if its transition function can handle the case. Indeed, this patch to add the feature touches none of the planner or executor, and little of the parser; the main missing stuff was DDL and pg_dump support. It is true that variadic aggregates can create the same sort of ambiguity about parameters versus ORDER BY keys that was complained of when we (briefly) had both one- and two-argument forms of string_agg(). However, the policy formed in response to that discussion only said that we'd not create any built-in aggregates with varying numbers of arguments, not that we shouldn't allow users to do it. So the logical extension of that is we can allow users to make variadic aggregates as long as we're wary about shipping any such in core. In passing, this patch allows aggregate function arguments to be named, to the extent of remembering the names in pg_proc and dumping them in pg_dump. You can't yet call an aggregate using named-parameter notation. That seems like a likely future extension, but it'll take some work, and it's not what this patch is really about. Likewise, there's still some work needed to make window functions handle VARIADIC fully, but I left that for another day. initdb forced because of new aggvariadic field in Aggref parse nodes. --- doc/src/sgml/ref/alter_aggregate.sgml | 37 +++++++++-- doc/src/sgml/ref/alter_extension.sgml | 4 +- doc/src/sgml/ref/comment.sgml | 8 +-- doc/src/sgml/ref/create_aggregate.sgml | 36 ++++++++-- doc/src/sgml/ref/drop_aggregate.sgml | 32 ++++++++- doc/src/sgml/ref/security_label.sgml | 10 +-- doc/src/sgml/syntax.sgml | 7 ++ doc/src/sgml/xaggr.sgml | 36 ++++++++++ src/backend/catalog/pg_aggregate.c | 18 +++-- src/backend/commands/aggregatecmds.c | 55 +++++++++++----- src/backend/commands/functioncmds.c | 66 +++++++++++++------ src/backend/executor/nodeAgg.c | 1 + src/backend/executor/nodeWindowAgg.c | 1 + src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/parser/gram.y | 47 +++++++++---- src/backend/parser/parse_agg.c | 16 +++-- src/backend/parser/parse_func.c | 16 +++-- src/backend/tcop/utility.c | 3 +- src/backend/utils/adt/ruleutils.c | 20 +++++- src/bin/pg_dump/pg_dump.c | 60 +++++++++++++---- src/bin/psql/describe.c | 9 ++- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_aggregate.h | 6 +- src/include/commands/defrem.h | 13 +++- src/include/nodes/primnodes.h | 1 + src/include/parser/parse_agg.h | 1 + src/test/regress/expected/aggregates.out | 13 ++++ .../regress/expected/create_aggregate.out | 7 ++ src/test/regress/expected/opr_sanity.out | 12 +++- src/test/regress/sql/aggregates.sql | 4 ++ src/test/regress/sql/create_aggregate.sql | 9 +++ src/test/regress/sql/opr_sanity.sql | 10 ++- 35 files changed, 448 insertions(+), 116 deletions(-) diff --git a/doc/src/sgml/ref/alter_aggregate.sgml b/doc/src/sgml/ref/alter_aggregate.sgml index 571a50a502e..aab5b2b695e 100644 --- a/doc/src/sgml/ref/alter_aggregate.sgml +++ b/doc/src/sgml/ref/alter_aggregate.sgml @@ -21,9 +21,12 @@ PostgreSQL documentation -ALTER AGGREGATE name ( argtype [ , ... ] ) RENAME TO new_name -ALTER AGGREGATE name ( argtype [ , ... ] ) OWNER TO new_owner -ALTER AGGREGATE name ( argtype [ , ... ] ) SET SCHEMA new_schema +ALTER AGGREGATE name ( [ argmode ] [ arg_name ] arg_data_type [ , ... ] ) + RENAME TO new_name +ALTER AGGREGATE name ( [ argmode ] [ arg_name ] arg_data_type [ , ... ] ) + OWNER TO new_owner +ALTER AGGREGATE name ( [ argmode ] [ arg_name ] arg_data_type [ , ... ] ) + SET SCHEMA new_schema @@ -62,12 +65,36 @@ ALTER AGGREGATE name ( argtype - argtype + argmode + + + + The mode of an argument: IN or VARIADIC. + If omitted, the default is IN. + + + + + + arg_name + + + + The name of an argument. + Note that ALTER AGGREGATE does not actually pay + any attention to argument names, since only the argument data + types are needed to determine the aggregate function's identity. + + + + + + arg_data_type An input data type on which the aggregate function operates. To reference a zero-argument aggregate function, write * - in place of the list of input data types. + in place of the list of argument specifications. diff --git a/doc/src/sgml/ref/alter_extension.sgml b/doc/src/sgml/ref/alter_extension.sgml index 2dbba0c0bbb..a14fcb48683 100644 --- a/doc/src/sgml/ref/alter_extension.sgml +++ b/doc/src/sgml/ref/alter_extension.sgml @@ -30,7 +30,7 @@ ALTER EXTENSION name DROP where member_object is: - AGGREGATE agg_name (agg_type [, ...] ) | + AGGREGATE agg_name ( [ argmode ] [ argname ] agg_type [, ...] ) | CAST (source_type AS target_type) | COLLATION object_name | CONVERSION object_name | @@ -179,7 +179,7 @@ ALTER EXTENSION name DROP An input data type on which the aggregate function operates. To reference a zero-argument aggregate function, write * - in place of the list of input data types. + in place of the list of argument specifications. diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml index e94dd4b8ded..e55050042a8 100644 --- a/doc/src/sgml/ref/comment.sgml +++ b/doc/src/sgml/ref/comment.sgml @@ -23,7 +23,7 @@ PostgreSQL documentation COMMENT ON { - AGGREGATE agg_name (agg_type [, ...] ) | + AGGREGATE agg_name ( [ argmode ] [ argname ] agg_type [, ...] ) | CAST (source_type AS target_type) | COLLATION object_name | COLUMN relation_name.column_name | @@ -126,7 +126,7 @@ COMMENT ON An input data type on which the aggregate function operates. To reference a zero-argument aggregate function, write * - in place of the list of input data types. + in place of the list of argument specifications. @@ -156,7 +156,7 @@ COMMENT ON The mode of a function argument: IN, OUT, INOUT, or VARIADIC. If omitted, the default is IN. - Note that COMMENT ON FUNCTION does not actually pay + Note that COMMENT does not actually pay any attention to OUT arguments, since only the input arguments are needed to determine the function's identity. So it is sufficient to list the IN, INOUT, @@ -170,7 +170,7 @@ COMMENT ON The name of a function argument. - Note that COMMENT ON FUNCTION does not actually pay + Note that COMMENT does not actually pay any attention to argument names, since only the argument data types are needed to determine the function's identity. diff --git a/doc/src/sgml/ref/create_aggregate.sgml b/doc/src/sgml/ref/create_aggregate.sgml index d5e4e272fce..2b35fa4d522 100644 --- a/doc/src/sgml/ref/create_aggregate.sgml +++ b/doc/src/sgml/ref/create_aggregate.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation -CREATE AGGREGATE name ( input_data_type [ , ... ] ) ( +CREATE AGGREGATE name ( [ argmode ] [ arg_name ] arg_data_type [ , ... ] ) ( SFUNC = sfunc, STYPE = state_data_type [ , FINALFUNC = ffunc ] @@ -118,7 +118,7 @@ CREATE AGGREGATE name ( Note that this behavior is only available when state_data_type is the same as the first - input_data_type. + arg_data_type. When these types are different, you must supply a nonnull initial condition or use a nonstrict transition function. @@ -187,12 +187,36 @@ SELECT col FROM tab ORDER BY col USING sortop LIMIT 1; - input_data_type + argmode + + + + The mode of an argument: IN or VARIADIC. + (Aggregate functions do not support OUT arguments.) + If omitted, the default is IN. Only the last argument + can be marked VARIADIC. + + + + + + arg_name + + + + The name of an argument. This is currently only useful for + documentation purposes. If omitted, the argument has no name. + + + + + + arg_data_type An input data type on which this aggregate function operates. To create a zero-argument aggregate function, write * - in place of the list of input data types. (An example of such an + in place of the list of argument specifications. (An example of such an aggregate is count(*).) @@ -205,8 +229,8 @@ SELECT col FROM tab ORDER BY col USING sortop LIMIT 1; In the old syntax for CREATE AGGREGATE, the input data type is specified by a 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 basetype as + only one input parameter. To define a zero-argument aggregate function + with this syntax, specify the basetype as "ANY" (not *). diff --git a/doc/src/sgml/ref/drop_aggregate.sgml b/doc/src/sgml/ref/drop_aggregate.sgml index 1ed152f6a05..06060fb4f7b 100644 --- a/doc/src/sgml/ref/drop_aggregate.sgml +++ b/doc/src/sgml/ref/drop_aggregate.sgml @@ -21,7 +21,9 @@ PostgreSQL documentation -DROP AGGREGATE [ IF EXISTS ] name ( argtype [ , ... ] ) [ CASCADE | RESTRICT ] +DROP AGGREGATE [ IF EXISTS ] + name ( [ argmode ] [ arg_name ] arg_data_type [ , ... ] ) + [ CASCADE | RESTRICT ] @@ -60,12 +62,36 @@ DROP AGGREGATE [ IF EXISTS ] name ( - argtype + argmode + + + + The mode of an argument: IN or VARIADIC. + If omitted, the default is IN. + + + + + + arg_name + + + + The name of an argument. + Note that DROP AGGREGATE does not actually pay + any attention to argument names, since only the argument data + types are needed to determine the aggregate function's identity. + + + + + + arg_data_type An input data type on which the aggregate function operates. To reference a zero-argument aggregate function, write * - in place of the list of input data types. + in place of the list of argument specifications. diff --git a/doc/src/sgml/ref/security_label.sgml b/doc/src/sgml/ref/security_label.sgml index 52cb1d16f4c..76c131f94ee 100644 --- a/doc/src/sgml/ref/security_label.sgml +++ b/doc/src/sgml/ref/security_label.sgml @@ -25,7 +25,7 @@ SECURITY LABEL [ FOR provider ] ON { TABLE object_name | COLUMN table_name.column_name | - AGGREGATE agg_name (agg_type [, ...] ) | + AGGREGATE agg_name ( [ argmode ] [ argname ] agg_type [, ...] ) | DATABASE object_name | DOMAIN object_name | EVENT TRIGGER object_name | @@ -107,12 +107,12 @@ SECURITY LABEL [ FOR provider ] ON - arg_type + agg_type An input data type on which the aggregate function operates. To reference a zero-argument aggregate function, write * - in place of the list of input data types. + in place of the list of argument specifications. @@ -125,7 +125,7 @@ SECURITY LABEL [ FOR provider ] ON The mode of a function argument: IN, OUT, INOUT, or VARIADIC. If omitted, the default is IN. - Note that SECURITY LABEL ON FUNCTION does not actually + Note that SECURITY LABEL does not actually pay any attention to OUT arguments, since only the input arguments are needed to determine the function's identity. So it is sufficient to list the IN, INOUT, @@ -140,7 +140,7 @@ SECURITY LABEL [ FOR provider ] ON The name of a function argument. - Note that SECURITY LABEL ON FUNCTION does not actually + Note that SECURITY LABEL does not actually pay any attention to argument names, since only the argument data types are needed to determine the function's identity. diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index 803ed855c82..e3dbc4b5ea5 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -2524,6 +2524,13 @@ SELECT concat_lower_or_upper('Hello', 'World', uppercase := true); having numerous parameters that have default values, named or mixed notation can save a great deal of writing and reduce chances for error. + + + + Named and mixed call notations can currently be used only with regular + functions, not with aggregate functions or window functions. + + diff --git a/doc/src/sgml/xaggr.sgml b/doc/src/sgml/xaggr.sgml index 1822f6d4abd..9ed7d99f7c0 100644 --- a/doc/src/sgml/xaggr.sgml +++ b/doc/src/sgml/xaggr.sgml @@ -169,6 +169,42 @@ SELECT attrelid::regclass, array_accum(atttypid::regtype) + + An aggregate function can be made to accept a varying number of arguments + by declaring its last argument as a VARIADIC array, in much + the same fashion as for regular functions; see + . The aggregate's transition + function must have the same array type as its last argument. The + transition function typically would also be marked VARIADIC, + but this is not strictly required. + + + + + Variadic aggregates are easily misused in connection with + the ORDER BY option (see ), + 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 ORDER BY is a sort key, not an argument to the + aggregate. For example, in + +SELECT myaggregate(a ORDER BY a, b, c) FROM ... + + the parser will see this as a single aggregate function argument and + three sort keys. However, the user might have intended + +SELECT myaggregate(a, b, c ORDER BY a) FROM ... + + If myaggregate is variadic, both these calls could be + perfectly valid. + + + + For the same reason, it's wise to think twice before creating aggregate + functions with the same names and different numbers of regular arguments. + + + A function written in C can detect that it is being called as an aggregate transition or final function by calling diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 480c17cd390..d9e961ebfad 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -45,8 +45,12 @@ static Oid lookup_agg_function(List *fnName, int nargs, Oid *input_types, Oid AggregateCreate(const char *aggName, Oid aggNamespace, - Oid *aggArgTypes, int numArgs, + oidvector *parameterTypes, + Datum allParameterTypes, + Datum parameterModes, + Datum parameterNames, + List *parameterDefaults, List *aggtransfnName, List *aggfinalfnName, List *aggsortopName, @@ -61,6 +65,7 @@ AggregateCreate(const char *aggName, Oid transfn; Oid finalfn = InvalidOid; /* can be omitted */ Oid sortop = InvalidOid; /* can be omitted */ + Oid *aggArgTypes = parameterTypes->values; bool hasPolyArg; bool hasInternalArg; Oid rettype; @@ -244,12 +249,11 @@ AggregateCreate(const char *aggName, 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 */ + 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 4a03786210a..78af0924654 100644 --- a/src/backend/commands/aggregatecmds.c +++ b/src/backend/commands/aggregatecmds.c @@ -45,10 +45,12 @@ * * "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). + * "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) +DefineAggregate(List *name, List *args, bool oldstyle, List *parameters, + const char *queryString) { char *aggName; Oid aggNamespace; @@ -59,8 +61,12 @@ DefineAggregate(List *name, List *args, bool oldstyle, List *parameters) TypeName *baseType = NULL; TypeName *transType = NULL; char *initval = NULL; - Oid *aggArgTypes; int numArgs; + oidvector *parameterTypes; + ArrayType *allParameterTypes; + ArrayType *parameterModes; + ArrayType *parameterNames; + List *parameterDefaults; Oid transTypeId; char transTypeType; ListCell *pl; @@ -131,6 +137,8 @@ DefineAggregate(List *name, List *args, bool oldstyle, List *parameters) * 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), @@ -139,22 +147,26 @@ DefineAggregate(List *name, List *args, bool oldstyle, List *parameters) if (pg_strcasecmp(TypeNameToString(baseType), "ANY") == 0) { numArgs = 0; - aggArgTypes = NULL; + aggArgTypes[0] = InvalidOid; } else { numArgs = 1; - aggArgTypes = (Oid *) palloc(sizeof(Oid)); 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 TypeNames (possibly zero of 'em). + * New style: args is a list of FunctionParameters (possibly zero of + * 'em). We share functioncmds.c's code for processing them. */ - ListCell *lc; - int i = 0; + Oid requiredResultType; if (baseType != NULL) ereport(ERROR, @@ -162,13 +174,20 @@ DefineAggregate(List *name, List *args, bool oldstyle, List *parameters) 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); - } + 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); } /* @@ -219,8 +238,12 @@ DefineAggregate(List *name, List *args, bool oldstyle, List *parameters) */ return AggregateCreate(aggName, /* aggregate name */ aggNamespace, /* namespace */ - aggArgTypes, /* input data type(s) */ 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 0a9facfbaaa..ca754b47ff9 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -88,7 +88,6 @@ compute_return_type(TypeName *returnType, Oid languageOid, typtup = LookupTypeName(NULL, returnType, NULL); - if (typtup) { if (!((Form_pg_type) GETSTRUCT(typtup))->typisdefined) @@ -158,22 +157,31 @@ compute_return_type(TypeName *returnType, Oid languageOid, } /* - * Interpret the parameter list of the CREATE FUNCTION statement. + * 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. */ -static void -examine_parameter_list(List *parameters, Oid languageOid, - const char *queryString, - oidvector **parameterTypes, - ArrayType **allParameterTypes, - ArrayType **parameterModes, - ArrayType **parameterNames, - List **parameterDefaults, - Oid *requiredResultType) +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; @@ -223,6 +231,12 @@ examine_parameter_list(List *parameters, Oid languageOid, (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), @@ -246,9 +260,16 @@ examine_parameter_list(List *parameters, Oid languageOid, aclcheck_error_type(aclresult, toid); if (t->setof) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("functions cannot accept set arguments"))); + { + 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) @@ -890,13 +911,16 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString) * Convert remaining parameters of CREATE to form wanted by * ProcedureCreate. */ - examine_parameter_list(stmt->parameters, languageOid, queryString, - ¶meterTypes, - &allParameterTypes, - ¶meterModes, - ¶meterNames, - ¶meterDefaults, - &requiredResultType); + 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 7a0c2541cbe..e02a6ffa8c3 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1696,6 +1696,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) /* 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 bbc53361d63..544ba989de1 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -1817,6 +1817,7 @@ initialize_peragg(WindowAggState *winstate, WindowFunc *wfunc, /* 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 788907ec0b5..65f3b984d64 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -1141,6 +1141,7 @@ _copyAggref(const Aggref *from) 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 496e31dd2f8..4c9b05e1e4d 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -198,6 +198,7 @@ _equalAggref(const Aggref *a, const Aggref *b) 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 cff4734d40e..a2903f92520 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -962,6 +962,7 @@ _outAggref(StringInfo str, const Aggref *node) 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 aad63e58cfd..d325bb32129 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -497,6 +497,7 @@ _readAggref(void) 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 22e82ba146b..a9812af7cfc 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -324,8 +324,9 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); 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 - aggr_args old_aggr_definition old_aggr_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 @@ -352,7 +353,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type into_clause create_as_target create_mv_target %type createfunc_opt_item common_func_opt_item dostmt_opt_item -%type func_arg func_arg_with_default table_func_column +%type func_arg func_arg_with_default table_func_column aggr_arg %type arg_class %type func_return func_type @@ -3659,7 +3660,7 @@ AlterExtensionContentsStmt: n->action = $4; n->objtype = OBJECT_AGGREGATE; n->objname = $6; - n->objargs = $7; + n->objargs = extractArgTypes($7); $$ = (Node *)n; } | ALTER EXTENSION name add_drop CAST '(' Typename AS Typename ')' @@ -4760,10 +4761,6 @@ def_arg: func_type { $$ = (Node *)$1; } | Sconst { $$ = (Node *)makeString($1); } ; -aggr_args: '(' type_list ')' { $$ = $2; } - | '(' '*' ')' { $$ = NIL; } - ; - old_aggr_definition: '(' old_aggr_list ')' { $$ = $2; } ; @@ -5242,7 +5239,7 @@ CommentStmt: CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_AGGREGATE; n->objname = $4; - n->objargs = $5; + n->objargs = extractArgTypes($5); n->comment = $7; $$ = (Node *) n; } @@ -5408,7 +5405,7 @@ SecLabelStmt: n->provider = $3; n->objtype = OBJECT_AGGREGATE; n->objname = $6; - n->objargs = $7; + n->objargs = extractArgTypes($7); n->label = $9; $$ = (Node *) n; } @@ -6395,6 +6392,28 @@ func_arg_with_default: } ; +/* 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 */ @@ -6594,7 +6613,7 @@ RemoveAggrStmt: DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; n->objects = list_make1($3); - n->arguments = list_make1($4); + n->arguments = list_make1(extractArgTypes($4)); n->behavior = $5; n->missing_ok = false; n->concurrent = false; @@ -6605,7 +6624,7 @@ RemoveAggrStmt: DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; n->objects = list_make1($5); - n->arguments = list_make1($6); + n->arguments = list_make1(extractArgTypes($6)); n->behavior = $7; n->missing_ok = true; n->concurrent = false; @@ -6821,7 +6840,7 @@ RenameStmt: ALTER AGGREGATE func_name aggr_args RENAME TO name RenameStmt *n = makeNode(RenameStmt); n->renameType = OBJECT_AGGREGATE; n->object = $3; - n->objarg = $4; + n->objarg = extractArgTypes($4); n->newname = $7; n->missing_ok = false; $$ = (Node *)n; @@ -7295,7 +7314,7 @@ AlterObjectSchemaStmt: AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt); n->objectType = OBJECT_AGGREGATE; n->object = $3; - n->objarg = $4; + n->objarg = extractArgTypes($4); n->newschema = $7; n->missing_ok = false; $$ = (Node *)n; @@ -7524,7 +7543,7 @@ AlterOwnerStmt: ALTER AGGREGATE func_name aggr_args OWNER TO RoleId AlterOwnerStmt *n = makeNode(AlterOwnerStmt); n->objectType = OBJECT_AGGREGATE; n->object = $3; - n->objarg = $4; + 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 4e4e1cddd83..98cb58a7cc0 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -965,6 +965,7 @@ check_ungrouped_columns_walker(Node *node, 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, @@ -975,6 +976,7 @@ build_aggregate_fnexprs(Oid *agg_input_types, { Param *argp; List *args; + FuncExpr *fexpr; int i; /* @@ -1005,12 +1007,14 @@ build_aggregate_fnexprs(Oid *agg_input_types, args = lappend(args, argp); } - *transfnexpr = (Expr *) makeFuncExpr(transfn_oid, - agg_state_type, - args, - InvalidOid, - agg_input_collation, - COERCE_EXPLICIT_CALL); + 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 1f02c9a5757..2bd24c89c87 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -385,7 +385,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, } /* - * When function is called an explicit VARIADIC labeled parameter, + * 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. */ @@ -425,8 +425,9 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, 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; + aggref->aggstar = agg_star; + aggref->aggvariadic = func_variadic; /* agglevelsup will be set by transformAggregateCall */ aggref->location = location; @@ -448,10 +449,13 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, 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. + * 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 c9408970b10..b1023c4e882 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1103,7 +1103,8 @@ ProcessUtilitySlow(Node *parsetree, { case OBJECT_AGGREGATE: DefineAggregate(stmt->defnames, stmt->args, - stmt->oldstyle, stmt->definition); + 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 2b005d6e973..37d66e18e8f 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -7405,6 +7405,7 @@ get_agg_expr(Aggref *aggref, deparse_context *context) Oid argtypes[FUNC_MAX_ARGS]; List *arglist; int nargs; + bool use_variadic; ListCell *l; /* Extract the regular arguments, ignoring resjunk stuff for the moment */ @@ -7430,13 +7431,26 @@ get_agg_expr(Aggref *aggref, deparse_context *context) appendStringInfo(buf, "%s(%s", generate_function_name(aggref->aggfnoid, nargs, NIL, argtypes, - false, NULL), + aggref->aggvariadic, + &use_variadic), (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); + { + 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 "); @@ -8581,7 +8595,7 @@ generate_relation_name(Oid relid, List *namespaces) * 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 + * 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 e1ef55f69a8..57320cc83a6 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -229,7 +229,8 @@ static void getTableData(TableInfo *tblinfo, int numTables, bool oids); 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(FuncInfo *finfo, char *funcargs, + bool is_agg); static char *format_function_arguments_old(Archive *fout, FuncInfo *finfo, int nallargs, char **allargtypes, @@ -9365,15 +9366,20 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang) * 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. + * 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) +format_function_arguments(FuncInfo *finfo, char *funcargs, bool is_agg) { PQExpBufferData fn; initPQExpBuffer(&fn); - appendPQExpBuffer(&fn, "%s(%s)", fmtId(finfo->dobj.name), funcargs); + appendPQExpBuffer(&fn, "%s", fmtId(finfo->dobj.name)); + if (is_agg && finfo->nargs == 0) + appendPQExpBuffer(&fn, "(*)"); + else + appendPQExpBuffer(&fn, "(%s)", funcargs); return fn.data; } @@ -9804,8 +9810,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo) 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); + funcfullsig = format_function_arguments(finfo, funcargs, false); + funcsig = format_function_arguments(finfo, funciargs, false); } else { @@ -11405,7 +11411,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo) PQExpBuffer delq; PQExpBuffer labelq; PQExpBuffer details; - char *aggsig; + char *aggsig; /* identity signature */ + char *aggfullsig; /* full signature */ char *aggsig_tag; PGresult *res; int i_aggtransfn; @@ -11435,14 +11442,28 @@ dumpAgg(Archive *fout, AggInfo *agginfo) selectSourceSchema(fout, agginfo->aggfn.dobj.namespace->dobj.name); /* Get aggregate-specific details */ - if (fout->remoteVersion >= 80100) + 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 " + "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); @@ -11499,7 +11520,24 @@ dumpAgg(Archive *fout, AggInfo *agginfo) agginitval = PQgetvalue(res, 0, i_agginitval); convertok = (PQgetvalue(res, 0, i_convertok)[0] == 't'); - aggsig = format_aggregate_signature(agginfo, fout, true); + 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) @@ -11559,7 +11597,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo) aggsig); appendPQExpBuffer(q, "CREATE AGGREGATE %s (\n%s\n);\n", - aggsig, details->data); + aggfullsig, details->data); appendPQExpBuffer(labelq, "AGGREGATE %s", aggsig); diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index dad1d5ac0de..ed1c5fdabc8 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -72,7 +72,14 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem) gettext_noop("Name"), gettext_noop("Result data type")); - if (pset.sversion >= 80200) + 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/catversion.h b/src/include/catalog/catversion.h index 2e51039c24b..9e46c55ed56 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201307221 +#define CATALOG_VERSION_NO 201309031 #endif diff --git a/src/include/catalog/pg_aggregate.h b/src/include/catalog/pg_aggregate.h index 6fb10a94567..5ad6ea6e3d1 100644 --- a/src/include/catalog/pg_aggregate.h +++ b/src/include/catalog/pg_aggregate.h @@ -240,8 +240,12 @@ DATA(insert ( 3175 json_agg_transfn json_agg_finalfn 0 2281 _null_ )); */ extern Oid AggregateCreate(const char *aggName, Oid aggNamespace, - Oid *aggArgTypes, 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 fa9f41f88f0..836c99e97ea 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -15,6 +15,7 @@ #define DEFREM_H #include "nodes/parsenodes.h" +#include "utils/array.h" /* commands/dropcmds.c */ extern void RemoveObjects(DropStmt *stmt); @@ -53,6 +54,16 @@ extern void IsThereFunctionInNamespace(const char *proname, int pronargs, 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); @@ -60,7 +71,7 @@ extern void RemoveOperatorById(Oid operOid); /* commands/aggregatecmds.c */ extern Oid DefineAggregate(List *name, List *args, bool oldstyle, - List *parameters); + 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 a778951362d..791853730b3 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -249,6 +249,7 @@ typedef struct Aggref 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 8fa0ca7f54d..b6d9dd37b04 100644 --- a/src/include/parser/parse_agg.h +++ b/src/include/parser/parse_agg.h @@ -25,6 +25,7 @@ extern void parseCheckAggregates(ParseState *pstate, Query *qry); 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 7fa900578c7..1af79e57e92 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -1249,3 +1249,16 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) filter (where a > 1) {"(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 ad1459419f8..6c7566f1728 100644 --- a/src/test/regress/expected/create_aggregate.out +++ b/src/test/regress/expected/create_aggregate.out @@ -59,3 +59,10 @@ create aggregate aggfns(integer,integer,text) ( 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 256b7196d0d..515cd9daaa8 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -843,6 +843,8 @@ ORDER BY 1, 2; -- 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 @@ -855,7 +857,15 @@ ORDER BY 1; count("any") | count() (1 row) --- For the same reason, aggregates with default arguments are no good. +-- 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 5c0196f5cf9..397edffd3ad 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -480,3 +480,7 @@ select sum(unique1) FILTER (WHERE 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 84f9a4f1e0f..3c7d330960f 100644 --- a/src/test/regress/sql/create_aggregate.sql +++ b/src/test/regress/sql/create_aggregate.sql @@ -71,3 +71,12 @@ create aggregate aggfns(integer,integer,text) ( 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 a3be0c1114f..efcd70f03be 100644 --- a/src/test/regress/sql/opr_sanity.sql +++ b/src/test/regress/sql/opr_sanity.sql @@ -674,6 +674,8 @@ ORDER BY 1, 2; -- 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 @@ -683,7 +685,13 @@ WHERE p1.oid < p2.oid AND p1.proname = p2.proname AND array_dims(p1.proargtypes) != array_dims(p2.proargtypes) ORDER BY 1; --- For the same reason, aggregates with default arguments are no good. +-- 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