RE: Partial aggregates pushdown

Hi Bruce, Jelte, hackers.

I apologize for my late response.

> From: Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
> Sent: Thursday, August 8, 2024 8:49 PM
> SUMMARY OF THREAD
>
> The design of patch 0001 is agreed upon by everyone on the thread (so far). This adds the PARTIAL_AGGREGATE label for
> aggregates, which will cause the finalfunc not to run. It also starts using PARTIAL_AGGREGATE for pushdown of
> aggregates in postgres_fdw. In 0001 PARTIAL_AGGREGATE is only supported for aggregates with a non-internal/pseudo
> type as the stype.
>
> The design for patch 0002 is still under debate. This would expand on the functionality added by adding support for
> PARTIAL_AGGREGATE for aggregates with an internal stype. This is done by returning a byte array containing the bytes
> that the serialfunc of the aggregate returns.
>
> A competing proposal for 0002 is to instead change aggregates to not use an internal stype anymore, and create dedicated
> types. The main downside here is that infunc and outfunc would need to be added for text serialization, in addition to the
> binary serialization. An open question is: Can we change the requirements for CREATE TYPE, so that types can be created
> without infunc and outfunc.

I rebased the patch 0001 and add the documentation to it.

Best regards, Yuki Fujii
--
Yuki Fujii
Information Technology R&D Center, Mitsubishi Electric Corporation

> -----Original Message-----
> From: Bruce Momjian <bruce(at)momjian(dot)us>
> Sent: Friday, August 23, 2024 4:31 AM
> To: Tomas Vondra <tomas(at)vondra(dot)me>
> Cc: Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>; Fujii Yuki/藤井 雄規(MELCO/情報総研 ＤＭ最適Ｇ)
> <Fujii(dot)Yuki(at)df(dot)MitsubishiElectric(dot)co(dot)jp>; PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>; Robert Haas
> <robertmhaas(at)gmail(dot)com>; Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>; Stephen Frost <sfrost(at)snowman(dot)net>; Andres Freund
> <andres(at)anarazel(dot)de>; Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>; Julien Rouhaud <rjuju123(at)gmail(dot)com>;
> Daniel Gustafsson <daniel(at)yesql(dot)se>; vignesh C <vignesh21(at)gmail(dot)com>; Alexander Pyhalov
> <a(dot)pyhalov(at)postgrespro(dot)ru>
> Subject: Re: Partial aggregates pushdown
>
> On Thu, Aug 22, 2024 at 08:31:11PM +0200, Tomas Vondra wrote:
> > > My question is related to #3 and #4. For #3, if we are going to be
> > > building infrastructure to handle passing int128 for AVG, wouldn't
> > > it be wiser to create an int128 type and an int128 array type, and
> > > then use method #2 to handle those, rather than creating custom code
> > > just to read/write int128 values for FDWs aggregate passing alone.
> > >
> >
> > Yep, adding int128 as a data type would extend this to aggregates that
> > store state as int128 (or array of int128).
>
> Great, I am not too far off then.
>
> > > For #4, can we use or improve the RECORD data type to handle #4 ---
> > > that seems preferable to creating custom FDWs aggregate passing code.
> > >
> > > I know the open question was whether we should create custom FDWs
> > > aggregate passing functions or custom data types for FDWs aggregate
> > > passing, but I am asking if we can improve existing facilities, like
> > > int128 or record passing, to reduce the need for some of these.
> > >
> >
> > But which code would produce the record? AFAIK it can't happen in some
> > generic executor code, because that only sees "internal" for each
> > aggregate. The exact structure of the aggstate is private within the
> > code of each aggregate - the record would have to be built there, no?
> >
> > I imagine we'd add this for each aggregate as a new pair of functions
> > to build/parse the record, but that's kinda the serial/deserial way we
> > discussed earlier.
> >
> > Or are you suggesting we'd actually say:
> >
> > CREATE AGGREGATE xyz(...) (
> > STYPE = record,
> > ...
> > )
>
> So my idea from the email I just sent is to create a pg_proc.proargtypes-like column (data type oidvector) for pg_aggregate
> which stores the oids of the values we want to return, so AVG(interval) would have an array of the oids for interval and int8,
> e.g.:
>
> SELECT oid FROM pg_type WHERE typname = 'interval';
> oid
> ------
> 1186
>
> SELECT oid FROM pg_type WHERE typname = 'int8';
> oid
> -----
> 20
>
> SELECT '1186 20'::oidvector;
> oidvector
> -----------
> 1186 20
>
> It seems all four methods could use this, again assuming we create
> int128/int16 and whatever other types we need.
>
> --
> Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
> EDB https://enterprisedb.com
>
> Only you can decide what is important to you.