diff --git a/doc/src/sgml/ref/create_aggregate.sgml b/doc/src/sgml/ref/create_aggregate.sgml index b8cd2e7af9..ca0e9db8b1 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 ( [ argmode ] [ argname ] arg_data_type [ , ... ] ) ( +CREATE [ OR REPLACE ] AGGREGATE name ( [ argmode ] [ argname ] arg_data_type [ , ... ] ) ( SFUNC = sfunc, STYPE = state_data_type [ , SSPACE = state_data_size ] @@ -44,7 +44,7 @@ CREATE AGGREGATE name ( [ name ( [ [ argmode ] [ argname ] arg_data_type [ , ... ] ] +CREATE [ OR REPLACE ] AGGREGATE name ( [ [ argmode ] [ argname ] arg_data_type [ , ... ] ] ORDER BY [ argmode ] [ argname ] arg_data_type [ , ... ] ) ( SFUNC = sfunc, STYPE = state_data_type @@ -59,7 +59,7 @@ CREATE AGGREGATE name ( [ [ or the old syntax -CREATE AGGREGATE name ( +CREATE [ OR REPLACE ] AGGREGATE name ( BASETYPE = base_type, SFUNC = sfunc, STYPE = state_data_type @@ -88,12 +88,21 @@ CREATE AGGREGATE name ( Description - CREATE AGGREGATE defines a new aggregate - function. Some basic and commonly-used aggregate functions are - included with the distribution; they are documented in . If one defines new types or needs - an aggregate function not already provided, then CREATE - AGGREGATE can be used to provide the desired features. + CREATE AGGREGATE defines a new aggregate function. + CREATE OR REPLACE AGGREGATE will either define a new + aggregate function or replace an existing definition. Some basic and + commonly-used aggregate functions are included with the distribution; they + are documented in . If one defines new + types or needs an aggregate function not already provided, then + CREATE AGGREGATE can be used to provide the desired + features. + + + + When replacing an existing definition, the argument types, result type, + and number of direct arguments may not be changed. Also, the new definition + must be of the same kind (ordinary aggregate, ordered-set aggregate, or + hypothetical-set aggregate) as the old one. diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 19e3171bf7..cdc8d9453d 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -45,6 +45,7 @@ static Oid lookup_agg_function(List *fnName, int nargs, Oid *input_types, ObjectAddress AggregateCreate(const char *aggName, Oid aggNamespace, + bool replace, char aggKind, int numArgs, int numDirectArgs, @@ -77,8 +78,10 @@ AggregateCreate(const char *aggName, { Relation aggdesc; HeapTuple tup; + HeapTuple oldtup; bool nulls[Natts_pg_aggregate]; Datum values[Natts_pg_aggregate]; + bool replaces[Natts_pg_aggregate]; Form_pg_proc proc; Oid transfn; Oid finalfn = InvalidOid; /* can be omitted */ @@ -609,7 +612,7 @@ AggregateCreate(const char *aggName, myself = ProcedureCreate(aggName, aggNamespace, - false, /* no replacement */ + replace, /* maybe replacement */ false, /* doesn't return a set */ finaltype, /* returnType */ GetUserId(), /* proowner */ @@ -648,6 +651,7 @@ AggregateCreate(const char *aggName, { nulls[i] = false; values[i] = (Datum) NULL; + replaces[i] = true; } values[Anum_pg_aggregate_aggfnoid - 1] = ObjectIdGetDatum(procOid); values[Anum_pg_aggregate_aggkind - 1] = CharGetDatum(aggKind); @@ -678,8 +682,51 @@ AggregateCreate(const char *aggName, else nulls[Anum_pg_aggregate_aggminitval - 1] = true; - tup = heap_form_tuple(tupDesc, values, nulls); - CatalogTupleInsert(aggdesc, tup); + if (replace) + oldtup = SearchSysCache1(AGGFNOID, ObjectIdGetDatum(procOid)); + else + oldtup = NULL; + + if (HeapTupleIsValid(oldtup)) + { + Form_pg_aggregate oldagg = (Form_pg_aggregate) GETSTRUCT(oldtup); + + /* + * If we're replacing an existing entry, we need to validate that + * we're not changing anything that would break callers. + * Specifically we must not change aggkind or aggnumdirectargs, + * which affect how an aggregate call is treated in parse + * analysis. + */ + if (aggKind != oldagg->aggkind) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot change routine kind"), + (oldagg->aggkind == AGGKIND_NORMAL ? + errdetail("\"%s\" is an ordinary aggregate function.", aggName) : + oldagg->aggkind == AGGKIND_ORDERED_SET ? + errdetail("\"%s\" is an ordered-set aggregate.", aggName) : + oldagg->aggkind == AGGKIND_HYPOTHETICAL ? + errdetail("\"%s\" is a hypothetical-set aggregate.", aggName) : + 0))); + if (numDirectArgs != oldagg->aggnumdirectargs) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("cannot change number of direct args of an aggregate function"))); + + replaces[Anum_pg_aggregate_aggfnoid - 1] = false; + replaces[Anum_pg_aggregate_aggkind - 1] = false; + replaces[Anum_pg_aggregate_aggnumdirectargs - 1] = false; + + tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces); + CatalogTupleUpdate(aggdesc, &tup->t_self, tup); + ReleaseSysCache(oldtup); + } + else + { + tup = heap_form_tuple(tupDesc, values, nulls); + CatalogTupleInsert(aggdesc, tup); + } table_close(aggdesc, RowExclusiveLock); @@ -688,6 +735,10 @@ AggregateCreate(const char *aggName, * made by ProcedureCreate). Note: we don't need an explicit dependency * on aggTransType since we depend on it indirectly through transfn. * Likewise for aggmTransType using the mtransfunc, if it exists. + * + * If we're replacing an existing definition, ProcedureCreate deleted all + * our existing dependencies, so we have to do the same things here either + * way. */ /* Depends on transition function */ diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 557e0ea1f1..fb22035a2a 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -404,7 +404,9 @@ ProcedureCreate(const char *procedureName, errdetail("\"%s\" is a window function.", procedureName) : 0))); - dropcmd = (prokind == PROKIND_PROCEDURE ? "DROP PROCEDURE" : "DROP FUNCTION"); + dropcmd = (prokind == PROKIND_PROCEDURE ? "DROP PROCEDURE" : + prokind == PROKIND_AGGREGATE ? "DROP AGGREGATE" : + "DROP FUNCTION"); /* * Not okay to change the return type of the existing proc, since @@ -421,7 +423,7 @@ ProcedureCreate(const char *procedureName, prokind == PROKIND_PROCEDURE ? errmsg("cannot change whether a procedure has output parameters") : errmsg("cannot change return type of existing function"), - /* translator: first %s is DROP FUNCTION or DROP PROCEDURE */ + /* translator: first %s is DROP FUNCTION, DROP PROCEDURE or DROP AGGREGATE */ errhint("Use %s %s first.", dropcmd, format_procedure(oldproc->oid)))); diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c index d00765fbc7..d569067dc4 100644 --- a/src/backend/commands/aggregatecmds.c +++ b/src/backend/commands/aggregatecmds.c @@ -54,7 +54,12 @@ static char extractModify(DefElem *defel); * "parameters" is a list of DefElem representing the agg's definition clauses. */ ObjectAddress -DefineAggregate(ParseState *pstate, List *name, List *args, bool oldstyle, List *parameters) +DefineAggregate(ParseState *pstate, + List *name, + List *args, + bool oldstyle, + List *parameters, + bool replace) { char *aggName; Oid aggNamespace; @@ -436,6 +441,7 @@ DefineAggregate(ParseState *pstate, List *name, List *args, bool oldstyle, List */ return AggregateCreate(aggName, /* aggregate name */ aggNamespace, /* namespace */ + replace, aggKind, numArgs, numDirectArgs, diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index a8a735c247..6f3565ad20 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3372,6 +3372,7 @@ _copyDefineStmt(const DefineStmt *from) COPY_NODE_FIELD(args); COPY_NODE_FIELD(definition); COPY_SCALAR_FIELD(if_not_exists); + COPY_SCALAR_FIELD(replace); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 3cab90e9f8..813606ce0e 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1265,6 +1265,7 @@ _equalDefineStmt(const DefineStmt *a, const DefineStmt *b) COMPARE_NODE_FIELD(args); COMPARE_NODE_FIELD(definition); COMPARE_SCALAR_FIELD(if_not_exists); + COMPARE_SCALAR_FIELD(replace); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index e814939a25..502e51bb0e 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -5618,25 +5618,27 @@ CreateAssertionStmt: *****************************************************************************/ DefineStmt: - CREATE AGGREGATE func_name aggr_args definition + CREATE opt_or_replace AGGREGATE func_name aggr_args definition { DefineStmt *n = makeNode(DefineStmt); n->kind = OBJECT_AGGREGATE; n->oldstyle = false; - n->defnames = $3; - n->args = $4; - n->definition = $5; + n->replace = $2; + n->defnames = $4; + n->args = $5; + n->definition = $6; $$ = (Node *)n; } - | CREATE AGGREGATE func_name old_aggr_definition + | CREATE opt_or_replace AGGREGATE func_name old_aggr_definition { /* old-style (pre-8.2) syntax for CREATE AGGREGATE */ DefineStmt *n = makeNode(DefineStmt); n->kind = OBJECT_AGGREGATE; n->oldstyle = true; - n->defnames = $3; + n->replace = $2; + n->defnames = $4; n->args = NIL; - n->definition = $4; + n->definition = $5; $$ = (Node *)n; } | CREATE OPERATOR any_operator definition diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index bdfaa506e7..5053ef05ef 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1237,7 +1237,8 @@ ProcessUtilitySlow(ParseState *pstate, address = DefineAggregate(pstate, stmt->defnames, stmt->args, stmt->oldstyle, - stmt->definition); + stmt->definition, + stmt->replace); break; case OBJECT_OPERATOR: Assert(stmt->args == NIL); diff --git a/src/include/catalog/pg_aggregate.h b/src/include/catalog/pg_aggregate.h index 832b7c2145..0b111b1283 100644 --- a/src/include/catalog/pg_aggregate.h +++ b/src/include/catalog/pg_aggregate.h @@ -142,6 +142,7 @@ typedef FormData_pg_aggregate *Form_pg_aggregate; extern ObjectAddress AggregateCreate(const char *aggName, Oid aggNamespace, + bool replace, char aggKind, int numArgs, int numDirectArgs, diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index e592a914a4..3bc2e8eb16 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -94,7 +94,7 @@ extern void UpdateStatisticsForTypeChange(Oid statsOid, /* commands/aggregatecmds.c */ extern ObjectAddress DefineAggregate(ParseState *pstate, List *name, List *args, bool oldstyle, - List *parameters); + List *parameters, bool replace); /* commands/opclasscmds.c */ extern ObjectAddress DefineOpClass(CreateOpClassStmt *stmt); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index fcfba4be4c..81278e4019 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2532,6 +2532,7 @@ typedef struct DefineStmt List *args; /* a list of TypeName (if needed) */ List *definition; /* a list of DefElem */ bool if_not_exists; /* just do nothing if it already exists? */ + bool replace; /* replace if already exists? */ } DefineStmt; /* ---------------------- diff --git a/src/test/regress/expected/create_aggregate.out b/src/test/regress/expected/create_aggregate.out index 3d92084e13..a2eb9996e1 100644 --- a/src/test/regress/expected/create_aggregate.out +++ b/src/test/regress/expected/create_aggregate.out @@ -160,6 +160,77 @@ WHERE aggfnoid = 'myavg'::REGPROC; myavg | numeric_avg_accum | numeric_avg_combine | internal | numeric_avg_serialize | numeric_avg_deserialize | s (1 row) +DROP AGGREGATE myavg (numeric); +-- create or replace aggregate +CREATE AGGREGATE myavg (numeric) +( + stype = internal, + sfunc = numeric_avg_accum, + finalfunc = numeric_avg +); +CREATE OR REPLACE AGGREGATE myavg (numeric) +( + stype = internal, + sfunc = numeric_avg_accum, + finalfunc = numeric_avg, + serialfunc = numeric_avg_serialize, + deserialfunc = numeric_avg_deserialize, + combinefunc = numeric_avg_combine, + finalfunc_modify = shareable -- just to test a non-default setting +); +-- Ensure all these functions made it into the catalog again +SELECT aggfnoid, aggtransfn, aggcombinefn, aggtranstype::regtype, + aggserialfn, aggdeserialfn, aggfinalmodify +FROM pg_aggregate +WHERE aggfnoid = 'myavg'::REGPROC; + aggfnoid | aggtransfn | aggcombinefn | aggtranstype | aggserialfn | aggdeserialfn | aggfinalmodify +----------+-------------------+---------------------+--------------+-----------------------+-------------------------+---------------- + myavg | numeric_avg_accum | numeric_avg_combine | internal | numeric_avg_serialize | numeric_avg_deserialize | s +(1 row) + +-- can change stype: +CREATE OR REPLACE AGGREGATE myavg (numeric) +( + stype = numeric, + sfunc = numeric_add +); +SELECT aggfnoid, aggtransfn, aggcombinefn, aggtranstype::regtype, + aggserialfn, aggdeserialfn, aggfinalmodify +FROM pg_aggregate +WHERE aggfnoid = 'myavg'::REGPROC; + aggfnoid | aggtransfn | aggcombinefn | aggtranstype | aggserialfn | aggdeserialfn | aggfinalmodify +----------+-------------+--------------+--------------+-------------+---------------+---------------- + myavg | numeric_add | - | numeric | - | - | r +(1 row) + +-- can't change return type: +CREATE OR REPLACE AGGREGATE myavg (numeric) +( + stype = numeric, + sfunc = numeric_add, + finalfunc = numeric_out +); +ERROR: cannot change return type of existing function +HINT: Use DROP AGGREGATE myavg(numeric) first. +-- can't change to a different kind: +CREATE OR REPLACE AGGREGATE myavg (order by numeric) +( + stype = numeric, + sfunc = numeric_add +); +ERROR: cannot change routine kind +DETAIL: "myavg" is an ordinary aggregate function. +-- can't change plain function to aggregate: +create function sum4(int8,int8,int8,int8) returns int8 as +'select $1 + $2 + $3 + $4' language sql strict immutable; +CREATE OR REPLACE AGGREGATE sum3 (int8,int8,int8) +( + stype = int8, + sfunc = sum4 +); +ERROR: cannot change routine kind +DETAIL: "sum3" is a function. +drop function sum4(int8,int8,int8,int8); DROP AGGREGATE myavg (numeric); -- invalid: bad parallel-safety marking CREATE AGGREGATE mysum (int) diff --git a/src/test/regress/sql/create_aggregate.sql b/src/test/regress/sql/create_aggregate.sql index cb6552e2d6..fd7cd400c1 100644 --- a/src/test/regress/sql/create_aggregate.sql +++ b/src/test/regress/sql/create_aggregate.sql @@ -174,6 +174,71 @@ WHERE aggfnoid = 'myavg'::REGPROC; DROP AGGREGATE myavg (numeric); +-- create or replace aggregate +CREATE AGGREGATE myavg (numeric) +( + stype = internal, + sfunc = numeric_avg_accum, + finalfunc = numeric_avg +); + +CREATE OR REPLACE AGGREGATE myavg (numeric) +( + stype = internal, + sfunc = numeric_avg_accum, + finalfunc = numeric_avg, + serialfunc = numeric_avg_serialize, + deserialfunc = numeric_avg_deserialize, + combinefunc = numeric_avg_combine, + finalfunc_modify = shareable -- just to test a non-default setting +); + +-- Ensure all these functions made it into the catalog again +SELECT aggfnoid, aggtransfn, aggcombinefn, aggtranstype::regtype, + aggserialfn, aggdeserialfn, aggfinalmodify +FROM pg_aggregate +WHERE aggfnoid = 'myavg'::REGPROC; + +-- can change stype: +CREATE OR REPLACE AGGREGATE myavg (numeric) +( + stype = numeric, + sfunc = numeric_add +); +SELECT aggfnoid, aggtransfn, aggcombinefn, aggtranstype::regtype, + aggserialfn, aggdeserialfn, aggfinalmodify +FROM pg_aggregate +WHERE aggfnoid = 'myavg'::REGPROC; + +-- can't change return type: +CREATE OR REPLACE AGGREGATE myavg (numeric) +( + stype = numeric, + sfunc = numeric_add, + finalfunc = numeric_out +); + +-- can't change to a different kind: +CREATE OR REPLACE AGGREGATE myavg (order by numeric) +( + stype = numeric, + sfunc = numeric_add +); + +-- can't change plain function to aggregate: +create function sum4(int8,int8,int8,int8) returns int8 as +'select $1 + $2 + $3 + $4' language sql strict immutable; + +CREATE OR REPLACE AGGREGATE sum3 (int8,int8,int8) +( + stype = int8, + sfunc = sum4 +); + +drop function sum4(int8,int8,int8,int8); + +DROP AGGREGATE myavg (numeric); + -- invalid: bad parallel-safety marking CREATE AGGREGATE mysum (int) (