Date: Wed, 5 Jun 2024 10:04:29 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Cc: "Fujii.Yuki@df.MitsubishiElectric.co.jp" <Fujii.Yuki@df.mitsubishielectric.co.jp>,
	Robert Haas <robertmhaas@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	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>
Subject: Re: Partial aggregates pushdown
Message-ID: <ZmBwbSRe_2d3IK4Y@momjian.us>
References: <CALDaNm0S5fb_8o+TtR8bXf3zOpqabOBLy0EJ3+oVzHH_dzpXvw@mail.gmail.com>
 <TYAPR01MB5514F0CBD9CD4F84A261198195562@TYAPR01MB5514.jpnprd01.prod.outlook.com>
 <bf816db00fb5eda86358e10a18a330e5@postgrespro.ru>
 <TYAPR01MB55141D18188AC86ADCE35FCB952F2@TYAPR01MB5514.jpnprd01.prod.outlook.com>
 <31ab8f7b91cef6e837b3e31ea1e35a9a@postgrespro.ru>
 <b6205c8ccce1b7bf7c750d6f01c665d4@postgrespro.ru>
 <f35243f029bf292e297b16774b4d5116@postgrespro.ru>
 <TYAPR01MB30884F8E4D94038CD661FFEF95F02@TYAPR01MB3088.jpnprd01.prod.outlook.com>
 <Zl9K5BZU2Gw4JI30@momjian.us>
 <a22d250b61a3faa425864573364db66d@postgrespro.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a22d250b61a3faa425864573364db66d@postgrespro.ru>
Archived-At: <https://www.postgresql.org/message-id/ZmBwbSRe_2d3IK4Y%40momjian.us>
Precedence: bulk

On Wed, Jun  5, 2024 at 08:19:04AM +0300, Alexander Pyhalov wrote:
> > *  Passes unsafe binary data from the foreign server.
> > 
> > Can someone show me where that last item is in the patch, and why can't
> > we just pass back values cast to text?
> 
> In finalize_aggregate() when we see partial aggregate with
> peragg->aggref->aggtranstype = INTERNALOID
> we call aggregate's serialization function and return it as bytea.
> 
> The issue is that this internal representation isn't guaranteed to be
> compatible between servers
> of different versions (or architectures?). So, likely, we instead should
> have called some export function for aggregate
> and later - some import function on postgres_fdw side. It doesn't matter
> much, what this export function
> generates - text, json or some portable binary format,
> 1) export/import functions should just "understand" it,
> 2) it should be a stable representation.

Okay, so looking at the serialization output functions already defined, I
see many zeros, which I assume means just the base data type, and eight
more:

	SELECT DISTINCT aggserialfn from pg_aggregate WHERE aggserialfn::oid != 0;
	        aggserialfn
	---------------------------
	 numeric_avg_serialize
	 string_agg_serialize
	 array_agg_array_serialize
	 numeric_serialize
	 int8_avg_serialize
	 array_agg_serialize
	 interval_avg_serialize
	 numeric_poly_serialize

I realize we need to return the sum and count for average, so that makes
sense.

So, we need import/export text representation for the partial aggregate
mode for these eight, and call the base data type text import/export
functions for the zero ones when in this mode?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.