Remove useless entries for aggregate functions from fmgrtab.c.

Gen_fmgrtab.pl treated aggregate functions the same as other built-in
functions, which is wasteful because there is no real need to have
entries for them in the fmgr_builtins[] table.  Suppressing those
entries saves about 3KB in the compiled table on my machine; which
is not a lot but it's not nothing either, considering that that
table is pretty "hot".  The only outside code change needed is
that ExecInitWindowAgg() can't be allowed to call fmgr_info_cxt()
on a plain aggregate function.  But that saves a few cycles anyway.

Having done that, the aggregate_dummy() function is unreferenced
and might as well be dropped.  Using "aggregate_dummy" as the prosrc
value for an aggregate is now just a documentation convention not
something that matters.  There was some discussion of using NULL
instead to save a few bytes in pg_proc, but we'd have to remove
prosrc's BKI_FORCE_NOT_NULL marking which doesn't seem a great idea.
Anyway, it's possible there's client-side code that expects to
see "aggregate_dummy" there, so I'm loath to change it without a
strong reason.

Discussion: https://postgr.es/m/533989.1604263665@sss.pgh.pa.us

diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index 0cf1da6ebba5a2cff9318fd0112200c6ac5c7f0e..7664bb62859d1319850bdfe84f096662b7d0fbf2 100644 (file)
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -620,7 +620,7 @@ AggregateCreate(const char *aggName,
                                                         GetUserId(),   /* proowner */
                                                         INTERNALlanguageId,    /* languageObjectId */
                                                         InvalidOid,    /* no validator */
-                                                        "aggregate_dummy", /* placeholder proc */
+                                                        "aggregate_dummy", /* placeholder (no such proc) */
                                                         NULL,  /* probin */
                                                         PROKIND_AGGREGATE,
                                                         false, /* security invoker (currently not

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 75e5bbf209d536613eaaa8411b073346f3dac1fd..d87677d659983451c718cb4a24b5e5cf33e0c19f 100644 (file)
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -4935,24 +4935,6 @@ AggRegisterCallback(FunctionCallInfo fcinfo,
 }
 
 
-/*
- * aggregate_dummy - dummy execution routine for aggregate functions
- *
- * This function is listed as the implementation (prosrc field) of pg_proc
- * entries for aggregate functions.  Its only purpose is to throw an error
- * if someone mistakenly executes such a function in the normal way.
- *
- * Perhaps someday we could assign real meaning to the prosrc field of
- * an aggregate?
- */
-Datum
-aggregate_dummy(PG_FUNCTION_ARGS)
-{
-       elog(ERROR, "aggregate function %u called as normal function",
-                fcinfo->flinfo->fn_oid);
-       return (Datum) 0;                       /* keep compiler quiet */
-}
-
 /* ----------------------------------------------------------------
  *                                             Parallel Query Support
  * ----------------------------------------------------------------

diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 4cc7da268d79c46d3ec9364429269beabcb2391a..de58df3d3f733e3633597437adb394724264a7cc 100644 (file)
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -2446,11 +2446,6 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
                perfuncstate->wfuncstate = wfuncstate;
                perfuncstate->wfunc = wfunc;
                perfuncstate->numArguments = list_length(wfuncstate->args);
-
-               fmgr_info_cxt(wfunc->winfnoid, &perfuncstate->flinfo,
-                                         econtext->ecxt_per_query_memory);
-               fmgr_info_set_expr((Node *) wfunc, &perfuncstate->flinfo);
-
                perfuncstate->winCollation = wfunc->inputcollid;
 
                get_typlenbyval(wfunc->wintype,
@@ -2479,6 +2474,11 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
                        winobj->argstates = wfuncstate->args;
                        winobj->localmem = NULL;
                        perfuncstate->winobj = winobj;
+
+                       /* It's a real window function, so set up to call it. */
+                       fmgr_info_cxt(wfunc->winfnoid, &perfuncstate->flinfo,
+                                                 econtext->ecxt_per_query_memory);
+                       fmgr_info_set_expr((Node *) wfunc, &perfuncstate->flinfo);
                }
        }
 

diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index 8228ad6db60db89b672c0228617eddb5fb1a362a..ae8cf5bb64cc5849438d18a85980cea9f882048d 100644 (file)
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -75,6 +75,7 @@ foreach my $row (@{ $catalog_data{pg_proc} })
                oid    => $bki_values{oid},
                name   => $bki_values{proname},
                lang   => $bki_values{prolang},
+               kind   => $bki_values{prokind},
                strict => $bki_values{proisstrict},
                retset => $bki_values{proretset},
                nargs  => $bki_values{pronargs},
@@ -195,8 +196,10 @@ foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
        $sqlname .= "_" . $s->{args} if ($proname_counts{ $s->{name} } > 1);
        $sqlname =~ s/\s+/_/g;
        print $ofh "#define F_" . uc $sqlname . " $s->{oid}\n";
-       # We want only one extern per internal-language function
-       if ($s->{lang} eq 'internal' && !$seenit{ $s->{prosrc} })
+       # We want only one extern per internal-language, non-aggregate function
+       if (   $s->{lang} eq 'internal'
+               && $s->{kind} ne 'a'
+               && !$seenit{ $s->{prosrc} })
        {
                $seenit{ $s->{prosrc} } = 1;
                print $pfh "extern Datum $s->{prosrc}(PG_FUNCTION_ARGS);\n";
@@ -214,6 +217,8 @@ my $fmgr_count       = 0;
 foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
 {
        next if $s->{lang} ne 'internal';
+       # We do not need entries for aggregate functions
+       next if $s->{kind} eq 'a';
 
        print $tfh ",\n" if ($fmgr_count > 0);
        print $tfh