While fooling with Gen_fmgrtab.pl for a nearby patch [1], I noticed
that fmgrtab.c had a lot of entries pointing at aggregate_dummy,
which seemed rather useless. So I experimented with removing them.
It turns out that nodeWindowAgg.c is carelessly expecting them to be
there, because it does fmgr_info_cxt() on the target window function
even if it will never call it because it's a plain aggregate.
But that's pretty trivial to fix, just need to relocate that call.
With that, we don't actually need aggregate_dummy() to exist at
all, because it's never referenced. Having "aggregate_dummy"
as the prosrc value for an aggregate function is now just a
random convention; any other string would do as well. (We could
save a few bytes in pg_proc by choosing a shorter string, but
probably it's better to stick to the existing convention.)
Anyway, this saves about 3KB in fmgrtab.o, without any downside
that I can see. If someone accidentally called an aggregate as
a normal function, they'd now get a different error message,
namely "internal function "aggregate_dummy" is not in internal lookup
table" instead of "aggregate function NNN called as normal function".
That doesn't really seem like a problem.
The attached patch is a delta over the one in [1].
regards, tom lane
[1] https://www.postgresql.org/message-id/472274.1604258384%40sss.pgh.pa.us
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index 0cf1da6ebb..7664bb6285 100644
--- 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 75e5bbf209..d87677d659 100644
--- 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 4cc7da268d..de58df3d3f 100644
--- 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 1edd5195d7..34e27950e7 100644
--- 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 =~ tr/ /_/;
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
