Date: Thu, 22 Aug 2024 15:30:53 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Tomas Vondra <tomas@vondra.me>
Cc: Jelte Fennema-Nio <postgres@jeltef.nl>,
	"Fujii.Yuki@df.MitsubishiElectric.co.jp" <Fujii.Yuki@df.mitsubishielectric.co.jp>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Stephen Frost <sfrost@snowman.net>,
	Andres Freund <andres@anarazel.de>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	Julien Rouhaud <rjuju123@gmail.com>,
	Daniel Gustafsson <daniel@yesql.se>,
	vignesh C <vignesh21@gmail.com>,
	Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Subject: Re: Partial aggregates pushdown
Message-ID: <ZseR7c0LgSSmouB6@momjian.us>
References: <CAGECzQR_qwYupAEEVeY6sC14hQ9V2BGLNQQdROWvF60QmMsZFQ@mail.gmail.com>
 <CAGECzQSB-Y=WGe4bGA7+cxLTrkuz8t_x+kpgJS+NP4mP0gVSFg@mail.gmail.com>
 <Zr5vUoXA4sCyZjsX@momjian.us>
 <CAGECzQRFPBsKFk4D56M8yt2TrFdRu_0gDbbBE1887x7LWU_ukg@mail.gmail.com>
 <ZsTJXZq2pcx1Pst-@momjian.us>
 <CAGECzQTx5Ofe8zHXgQHn5cKwaydqHOK8qdWvDyyBPCTnEfvUvw@mail.gmail.com>
 <ZsTjTkUxZ8TnoVWp@momjian.us>
 <91f71f11-3431-49ab-95d0-1c5c90be9fb0@vondra.me>
 <ZsdzvMLLQ-KYoy7J@momjian.us>
 <ea31e84d-609d-47f5-9490-2d1c2551d587@vondra.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ea31e84d-609d-47f5-9490-2d1c2551d587@vondra.me>
Archived-At: <https://www.postgresql.org/message-id/ZseR7c0LgSSmouB6%40momjian.us>
Precedence: bulk

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@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.