Allow a prosupport function to be attached to an aggregate
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253436psql -h localhost -U postgresBuilt from patchset v8 (message #8), August 22, 2026 at 08:20 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253436_8 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253436_8 && git checkout t253436_8Patchset v8 (message #8) is on t253436_8
Hi,
Commit 42473b3b31 added SupportRequestSimplifyAggref, which the planner issues
for any Aggref whose function names a support function. Nice, but extensions
cannot reach it, because there is no way to attach a support function to an
aggregate:
ALTER FUNCTION pg_catalog.sum(numeric) SUPPORT numeric_support;
ERROR: "pg_catalog.sum" is an aggregate function
CREATE/ALTER aggregate doesn't support this feature at all. This does not look
like a decision about support functions. AlterFunction() rejects an aggregate
for every property it can change, not just for SUPPORT, and the other properties
it covers are ones AggregateCreate() fixes for aggregates anyway. prosupport is
different: nothing else in the catalog determines it, so refusing to set it
leaves the capability simply unreachable rather than merely inconvenient.
Updating pg_proc directly is not an answer. It records no pg_depend entry, so
the support function can then be dropped and leave a dangling OID behind.
My interest is extension-defined optimizations of numeric aggregates [1]https://github.com/danolivo/pg_numeric_agg_support or even
more extravagant dumb transformations, like the following (doable for specific
conditions):
SUM(x ORDER BY x) → SUM(x)
SUM(x) FILTER (WHERE true) → SUM(x)
SUM(N) -> N * COUNT(*)
Please find attached a patch adding SUPPORT to CREATE AGGREGATE's option
list and an ALTER AGGREGATE ... SUPPORT form. Both require superuser,
as the equivalent function clauses do, and record a normal dependency.
[1]: https://github.com/danolivo/pg_numeric_agg_support
--
regards, Andrei Lepikhov,
pgEdge
On Mon, 17 Aug 2026 at 18:13, Andrei Lepikhov <lepihov@gmail.com> wrote:
Commit 42473b3b31 added SupportRequestSimplifyAggref, which the planner issues
for any Aggref whose function names a support function. Nice, but extensions
cannot reach it, because there is no way to attach a support function to an
aggregate:ALTER FUNCTION pg_catalog.sum(numeric) SUPPORT numeric_support;
ERROR: "pg_catalog.sum" is an aggregate function
I failed to realise that there was no way to set that for user-defined
aggregates. Not good. While I do agree that lack of extensibility is
not aligned with the spirit of the project, my current thoughts are
that now is a bit late to add this to v19.
Is delaying this until v20 fine for you?
Or does anyone else feel this is a must-fix for v19?
David
On 18/08/2026 06:56, David Rowley wrote:
On Mon, 17 Aug 2026 at 18:13, Andrei Lepikhov <lepihov@gmail.com> wrote:
Commit 42473b3b31 added SupportRequestSimplifyAggref, which the planner issues
for any Aggref whose function names a support function. Nice, but extensions
cannot reach it, because there is no way to attach a support function to an
aggregate:ALTER FUNCTION pg_catalog.sum(numeric) SUPPORT numeric_support;
ERROR: "pg_catalog.sum" is an aggregate functionI failed to realise that there was no way to set that for user-defined
aggregates. Not good. While I do agree that lack of extensibility is
not aligned with the spirit of the project, my current thoughts are
that now is a bit late to add this to v19.Is delaying this until v20 fine for you?
Yes, I can do it manually in pg_catalog for now - rewriting prosupport and
setting the dependency in the pg_depend. There’s no rush - it’s just about
keeping things tidy.
--
regards, Andrei Lepikhov,
pgEdge
On 18/08/2026 08:31, Andrei Lepikhov wrote:
On 18/08/2026 06:56, David Rowley wrote:
Is delaying this until v20 fine for you?
Yes, I can do it manually in pg_catalog for now - rewriting prosupport and
setting the dependency in the pg_depend. There’s no rush - it’s just about
keeping things tidy.
However, I noticed that it is not documented yet.
There are also three other prosupport requests that are not documented. A quick
search showed that documentation usually depends on the author, so it makes
sense to add it now.
Since each was introduced at a different time and serves a different purpose, I
suggest we start by documenting SupportRequestSimplifyAggref, as I am more
familiar with it - see the patch in attachment.
--
regards, Andrei Lepikhov,
pgEdge
David Rowley <dgrowleyml@gmail.com> writes:
I failed to realise that there was no way to set that for user-defined
aggregates. Not good. While I do agree that lack of extensibility is
not aligned with the spirit of the project, my current thoughts are
that now is a bit late to add this to v19.
Is delaying this until v20 fine for you?
Or does anyone else feel this is a must-fix for v19?
That seems quite sad. The entire point of the support-function
mechanism is to allow extensibility, so if an extension cannot use
a support hook we have totally failed at implementing that hook.
(As you yourself remarked in a different context, it's unlikely
we would have implemented SupportRequestSimplifyAggref if the only
possible use-case was COUNT(any).)
So I think it's a must-fix. To put my money where my mouth is,
here's a draft patch. (I could not resist the temptation to
improve compute_common_attribute's existing error message,
which pretty well sucks: it's useless in a context that's not
showing you an accurate error pointer.)
regards, tom lane
On 18/8/2026 16:35, Tom Lane wrote:
David Rowley <dgrowleyml@gmail.com> writes:
So I think it's a must-fix. To put my money where my mouth is,
here's a draft patch. (I could not resist the temptation to
improve compute_common_attribute's existing error message,
which pretty well sucks: it's useless in a context that's not
showing you an accurate error pointer.)
Thank you for the patch. I agree that it is the right shape for v19, so
I assembled it into a small series.
- 0001 documents SupportRequestSimplifyAggref. This is the patch I sent
earlier, unchanged.
- 0002 is your patch, plus documentation and regression tests.
The documentation there provides the rationale for why CREATE/ALTER
AGGREGATE should be added in v.20.
Also, I'm not sure why pg_dump/upgrade has not been implemented. So, I
added an 'unsupported' clause into the docs - let's discuss it later or
correct me if such a feature can't be supported at all.
--
regards, Andrei Lepikhov
Attachments:
t253436_60001-Document-the-SupportRequestSimplifyAggref-support-re.patchtext/plain; charset=UTF-8; name=0001-Document-the-SupportRequestSimplifyAggref-support-re.patchDownload+16-1
0002-Allow-a-planner-support-function-to-be-attached-to-a.patchtext/plain; charset=UTF-8; name=0002-Allow-a-planner-support-function-to-be-attached-to-a.patchDownload+236-24
Andrei Lepikhov <lepihov@gmail.com> writes:
Also, I'm not sure why pg_dump/upgrade has not been implemented. So, I
added an 'unsupported' clause into the docs - let's discuss it later or
correct me if such a feature can't be supported at all.
Shame on me, I didn't think about that angle.
After reflection I realized that the approach I took in my first patch
is wrong: letting ALTER FUNCTION work on an aggregate is pretty
non-orthogonal, if the only property it can set is SUPPORT. The right
way to do this is to add a SUPPORT option to CREATE AGGREGATE, which
slots nicely into getting pg_dump to handle it. As attached.
One thing I don't quite love about this is that the CREATE AGGREGATE
docs already refer to transfn, finalfn, etc as "support functions",
so that there's a potential for confusion as to what SUPPORT really
means. I considered naming the option something else, but it feels
wrong to not use the same name as in CREATE FUNCTION. Any other
opinions about that?
regards, tom lane
I wrote:
After reflection I realized that the approach I took in my first patch
is wrong: letting ALTER FUNCTION work on an aggregate is pretty
non-orthogonal, if the only property it can set is SUPPORT. The right
way to do this is to add a SUPPORT option to CREATE AGGREGATE, which
slots nicely into getting pg_dump to handle it. As attached.
Grumble ... for some reason I was thinking that ProcedureCreate would
handle checking for superuser when specifying a support function,
but it doesn't; AggregateCreate must do that. v3 attached contains
just that one addition.
regards, tom lane
On 20/08/2026 22:06, Tom Lane wrote:
I wrote:
After reflection I realized that the approach I took in my first patch
is wrong: letting ALTER FUNCTION work on an aggregate is pretty
non-orthogonal, if the only property it can set is SUPPORT. The right
way to do this is to add a SUPPORT option to CREATE AGGREGATE, which
slots nicely into getting pg_dump to handle it. As attached.Grumble ... for some reason I was thinking that ProcedureCreate would
handle checking for superuser when specifying a support function,
but it doesn't; AggregateCreate must do that. v3 attached contains
just that one addition.
Thanks. For aggregates that an extension defines itself, this is clearly the
right approach, and the pg_dump part is exactly what v0 was missing.
It does leave out the case I started with, though: attaching a support function
to an existing aggregate. What I want to optimise is the built-in functions like
sum(), avg(), and similar ones, because those are what users actually use. With
only CREATE OR REPLACE AGGREGATE available, an extension would have to restate
the whole definition of pg_catalog.sum(numeric), including sfunc, combinefunc,
and so on, and keep that copy updated with every major release. Even worse,
CREATE OR REPLACE in an extension script should record an extension dependency,
so pg_catalog.sum would become a member of the extension and DROP EXTENSION
would try to drop it.
ALTER AGGREGATE ... SUPPORT avoids both problems, and for this use case it does
not need anything from pg_dump. The statement is part of the extension script,
so CREATE EXTENSION replays it on restore. That is why v0 had both forms. Could
we commit your patch now and add the ALTER form as well, in v20 if it is too
late for v19?
Also, I am not sure I understand the reason for not exporting
interpret_func_support(), as v0 did, instead of repeating the lookup,
return-type, and superuser checks.
One thing I don't quite love about this is that the CREATE AGGREGATE
docs already refer to transfn, finalfn, etc as "support functions",
In v0, I just added a paragraph to the create_aggregate.sgml:
"Note that this is unrelated to the aggregate's transition, final and other
helper functions described above, which implement the aggregate itself; a
planner support function instead advises the planner about calls to the aggregate."
I think this should be enough.
--
regards, Andrei Lepikhov,
pgEdge
Andrei Lepikhov <lepihov@gmail.com> writes:
Thanks. For aggregates that an extension defines itself, this is clearly the
right approach, and the pg_dump part is exactly what v0 was missing.
It does leave out the case I started with, though: attaching a support function
to an existing aggregate.
How so? The expectation is that you'd issue CREATE OR REPLACE
AGGREGATE. It is a bit tedious to have to re-specify all the
existing properties, but that's the way previous extensions of
aggregate options have been handled. I don't feel that this one
new feature is a big enough deal to redesign that (especially
not this late in beta).
What I want to optimise is the built-in functions like
sum(), avg(), and similar ones, because those are what users
actually use.
For those, wouldn't we just modify the initial contents of pg_proc?
With
only CREATE OR REPLACE AGGREGATE available, an extension would have to restate
the whole definition of pg_catalog.sum(numeric), including sfunc, combinefunc,
and so on, and keep that copy updated with every major release.
I'm not particularly on board with this goal: I think letting
extensions modify built-in aggregates is fraught with all sorts
of issues. (To name only one, there's no way for multiple
extensions to each attach new behaviors to the same aggregate.)
I'm certainly not going to risk putting in half-considered
support for such a thing at this stage of the release cycle.
There's enough risk in adding what I proposed yesterday.
regards, tom lane
On 21/08/2026 17:20, Tom Lane wrote:
Andrei Lepikhov <lepihov@gmail.com> writes:>
What I want to optimise is the built-in functions like
sum(), avg(), and similar ones, because those are what users
actually use.For those, wouldn't we just modify the initial contents of pg_proc?
For the trivial ones, yes, and that is a fair point. sum(x) FILTER (WHERE true)
-> sum(x) holds unconditionally; sum(x ORDER BY x) -> sum(x) holds for the exact
types, though not for float, where the order is part of the result. They would
also make a decent worked example of an aggregate support function - AFAICS
int8inc_support is still the only in-core user of SupportRequestSimplifyAggref.
Other optimisations of built-in aggregates in my pocket are an OLAP
specialisation and would be pure overhead for other workloads. I doubt it makes
sense to put a sum(numeric(N,M)) variant into core when it only pays off in a
database whose numeric columns carry a typmod.
With
only CREATE OR REPLACE AGGREGATE available, an extension would have to restate
the whole definition of pg_catalog.sum(numeric), including sfunc, combinefunc,
and so on, and keep that copy updated with every major release.I'm not particularly on board with this goal: I think letting
extensions modify built-in aggregates is fraught with all sorts
of issues. (To name only one, there's no way for multiple
extensions to each attach new behaviors to the same aggregate.)
Agreed as far as it goes, though the limitation is neither new nor specific to
aggregates: prosupport is a single regproc column, so ALTER FUNCTION ... SUPPORT
on a plain function is already last-one-wins. Whether a shared object should
accept support functions from third parties at all is a fair question, and a
larger one than this patch. So, with more evidence of practical need, we can
return to it later.
In my experience that is what applications ask for. An extension also back-ports
the optimisation to already-released majors, which is a large part of why we
write one. In the meantime we will do it by rewriting prosupport directly and
adding the pg_depend entry - the only way I know to attach such an optimisation
without touching the application.
I'm certainly not going to risk putting in half-considered
support for such a thing at this stage of the release cycle.
There's enough risk in adding what I proposed yesterday.
No objection. It closes the case the thread started from and the pg_dump part is
what v0 was missing.
--
regards, Andrei Lepikhov,
pgEdge
Andrei Lepikhov <lepihov@gmail.com> writes:
On 21/08/2026 17:20, Tom Lane wrote:
I'm certainly not going to risk putting in half-considered
support for such a thing at this stage of the release cycle.
There's enough risk in adding what I proposed yesterday.
No objection. It closes the case the thread started from and the pg_dump part is
what v0 was missing.
OK, pushed v3 then.
regards, tom lane