nodeAgg has always been willing to accept an aggregate with a finalFunc

and only one transition state, but the CREATE AGGREGATE code rejected
this combination.

diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index bfecb9f1fe0130b61157ce2d0cee4d586e4d55d2..5554752bf3fdbe19e477150a6e026598151b5247 100644 (file)
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/catalog/pg_aggregate.c,v 1.29 2000/01/26 05:56:10 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/catalog/pg_aggregate.c,v 1.30 2000/03/26 19:43:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -30,7 +30,7 @@
  * aggregates overloading has been added.  Instead of the full
  * overload support we have for functions, aggregate overloading only
  * applies to exact basetype matches.  That is, we don't check the
- * the inheritance hierarchy
+ * inheritance hierarchy
  *
  * OLD COMMENTS:
  *     Currently, redefining aggregates using the same name is not
@@ -85,6 +85,10 @@ AggregateCreate(char *aggName,
    if (!aggtransfn1Name && !aggtransfn2Name)
        elog(ERROR, "AggregateCreate: aggregate must have at least one transition function");
 
+   if (aggtransfn1Name && aggtransfn2Name && !aggfinalfnName)
+       elog(ERROR, "AggregateCreate: Aggregate must have final function with both transition functions");
+
+   /* handle the aggregate's base type (input data type) */
    tup = SearchSysCacheTuple(TYPENAME,
                              PointerGetDatum(aggbasetypeName),
                              0, 0, 0);
@@ -92,6 +96,17 @@ AggregateCreate(char *aggName,
        elog(ERROR, "AggregateCreate: Type '%s' undefined", aggbasetypeName);
    xbase = tup->t_data->t_oid;
 
+   /* make sure there is no existing agg of same name and base type */
+   tup = SearchSysCacheTuple(AGGNAME,
+                             PointerGetDatum(aggName),
+                             ObjectIdGetDatum(xbase),
+                             0, 0);
+   if (HeapTupleIsValid(tup))
+       elog(ERROR,
+            "AggregateCreate: aggregate '%s' with base type '%s' already exists",
+            aggName, aggbasetypeName);
+
+   /* handle transfn1 and transtype1 */
    if (aggtransfn1Name)
    {
        tup = SearchSysCacheTuple(TYPENAME,
@@ -114,14 +129,14 @@ AggregateCreate(char *aggName,
                 aggtransfn1Name, aggtransfn1typeName, aggbasetypeName);
        if (((Form_pg_proc) GETSTRUCT(tup))->prorettype != xret1)
            elog(ERROR, "AggregateCreate: return type of '%s' is not '%s'",
-                aggtransfn1Name,
-                aggtransfn1typeName);
+                aggtransfn1Name, aggtransfn1typeName);
        xfn1 = tup->t_data->t_oid;
        if (!OidIsValid(xfn1) || !OidIsValid(xret1) ||
            !OidIsValid(xbase))
-           elog(ERROR, "AggregateCreate: bogus function '%s'", aggfinalfnName);
+           elog(ERROR, "AggregateCreate: bogus function '%s'", aggtransfn1Name);
    }
 
+   /* handle transfn2 and transtype2 */
    if (aggtransfn2Name)
    {
        tup = SearchSysCacheTuple(TYPENAME,
@@ -147,47 +162,57 @@ AggregateCreate(char *aggName,
                 aggtransfn2Name, aggtransfn2typeName);
        xfn2 = tup->t_data->t_oid;
        if (!OidIsValid(xfn2) || !OidIsValid(xret2))
-           elog(ERROR, "AggregateCreate: bogus function '%s'", aggfinalfnName);
+           elog(ERROR, "AggregateCreate: bogus function '%s'", aggtransfn2Name);
    }
 
-   tup = SearchSysCacheTuple(AGGNAME,
-                             PointerGetDatum(aggName),
-                             ObjectIdGetDatum(xbase),
-                             0, 0);
-   if (HeapTupleIsValid(tup))
-       elog(ERROR,
-            "AggregateCreate: aggregate '%s' with base type '%s' already exists",
-            aggName, aggbasetypeName);
-
-   /* more sanity checks */
-   if (aggtransfn1Name && aggtransfn2Name && !aggfinalfnName)
-       elog(ERROR, "AggregateCreate: Aggregate must have final function with both transition functions");
-
-   if ((!aggtransfn1Name || !aggtransfn2Name) && aggfinalfnName)
-       elog(ERROR, "AggregateCreate: Aggregate cannot have final function without both transition functions");
-
+   /* handle finalfn */
    if (aggfinalfnName)
    {
-       fnArgs[0] = xret1;
-       fnArgs[1] = xret2;
+       int nargs = 0;
+
+       if (OidIsValid(xret1))
+           fnArgs[nargs++] = xret1;
+       if (OidIsValid(xret2))
+           fnArgs[nargs++] = xret2;
+       fnArgs[nargs] = 0;      /* make sure slot 2 is empty if just 1 arg */
        tup = SearchSysCacheTuple(PROCNAME,
                                  PointerGetDatum(aggfinalfnName),
-                                 Int32GetDatum(2),
+                                 Int32GetDatum(nargs),
                                  PointerGetDatum(fnArgs),
                                  0);
        if (!HeapTupleIsValid(tup))
-           elog(ERROR, "AggregateCreate: '%s'('%s','%s') does not exist",
-              aggfinalfnName, aggtransfn1typeName, aggtransfn2typeName);
+       {
+           if (nargs == 2)
+               elog(ERROR, "AggregateCreate: '%s'('%s','%s') does not exist",
+                    aggfinalfnName, aggtransfn1typeName, aggtransfn2typeName);
+           else if (OidIsValid(xret1))
+               elog(ERROR, "AggregateCreate: '%s'('%s') does not exist",
+                    aggfinalfnName, aggtransfn1typeName);
+           else
+               elog(ERROR, "AggregateCreate: '%s'('%s') does not exist",
+                    aggfinalfnName, aggtransfn2typeName);
+       }
        ffn = tup->t_data->t_oid;
        proc = (Form_pg_proc) GETSTRUCT(tup);
        fret = proc->prorettype;
        if (!OidIsValid(ffn) || !OidIsValid(fret))
            elog(ERROR, "AggregateCreate: bogus function '%s'", aggfinalfnName);
    }
+   else
+   {
+       /* If no finalfn, aggregate result type is type of the sole
+        * state value (we already checked there is only one)
+        */
+       if (OidIsValid(xret1))
+           fret = xret1;
+       else
+           fret = xret2;
+   }
+   Assert(OidIsValid(fret));
 
    /*
     * If transition function 2 is defined, it must have an initial value,
-    * whereas transition function 1 does not, which allows man and min
+    * whereas transition function 1 need not, which allows max and min
     * aggregates to return NULL if they are evaluated on empty sets.
     */
    if (OidIsValid(xfn2) && !agginitval2)
@@ -205,26 +230,10 @@ AggregateCreate(char *aggName,
    values[Anum_pg_aggregate_aggtransfn1 - 1] = ObjectIdGetDatum(xfn1);
    values[Anum_pg_aggregate_aggtransfn2 - 1] = ObjectIdGetDatum(xfn2);
    values[Anum_pg_aggregate_aggfinalfn - 1] = ObjectIdGetDatum(ffn);
-
    values[Anum_pg_aggregate_aggbasetype - 1] = ObjectIdGetDatum(xbase);
-   if (!OidIsValid(xfn1))
-   {
-       values[Anum_pg_aggregate_aggtranstype1 - 1] = ObjectIdGetDatum(InvalidOid);
-       values[Anum_pg_aggregate_aggtranstype2 - 1] = ObjectIdGetDatum(xret2);
-       values[Anum_pg_aggregate_aggfinaltype - 1] = ObjectIdGetDatum(xret2);
-   }
-   else if (!OidIsValid(xfn2))
-   {
-       values[Anum_pg_aggregate_aggtranstype1 - 1] = ObjectIdGetDatum(xret1);
-       values[Anum_pg_aggregate_aggtranstype2 - 1] = ObjectIdGetDatum(InvalidOid);
-       values[Anum_pg_aggregate_aggfinaltype - 1] = ObjectIdGetDatum(xret1);
-   }
-   else
-   {
-       values[Anum_pg_aggregate_aggtranstype1 - 1] = ObjectIdGetDatum(xret1);
-       values[Anum_pg_aggregate_aggtranstype2 - 1] = ObjectIdGetDatum(xret2);
-       values[Anum_pg_aggregate_aggfinaltype - 1] = ObjectIdGetDatum(fret);
-   }
+   values[Anum_pg_aggregate_aggtranstype1 - 1] = ObjectIdGetDatum(xret1);
+   values[Anum_pg_aggregate_aggtranstype2 - 1] = ObjectIdGetDatum(xret2);
+   values[Anum_pg_aggregate_aggfinaltype - 1] = ObjectIdGetDatum(fret);
 
    if (agginitval1)
        values[Anum_pg_aggregate_agginitval1 - 1] = PointerGetDatum(textin(agginitval1));