public inbox for [email protected]  
help / color / mirror / Atom feed
From: [email protected] <[email protected]>
To: Bruce Momjian <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Alexander Pyhalov <[email protected]>
Cc: [email protected] <[email protected]>
Subject: RE: Partial aggregates pushdown
Date: Tue, 11 Jun 2024 11:39:34 +0000
Message-ID: <TYAPR01MB3088EEECFEB79434E0F4339695C72@TYAPR01MB3088.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <CALDaNm0S5fb_8o+TtR8bXf3zOpqabOBLy0EJ3+oVzHH_dzpXvw@mail.gmail.com>
	<TYAPR01MB5514F0CBD9CD4F84A261198195562@TYAPR01MB5514.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB55141D18188AC86ADCE35FCB952F2@TYAPR01MB5514.jpnprd01.prod.outlook.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<TYAPR01MB30884F8E4D94038CD661FFEF95F02@TYAPR01MB3088.jpnprd01.prod.outlook.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Hi. Bruce.

Sorry for the late. Thank you for comment.

> From: Bruce Momjian <[email protected]>
> Sent: Wednesday, June 5, 2024 11:04 PM
> > > *  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?

I think that you are basically right.
But, I think, in a perfect world we should also add an import/export function for the following
two category.

  Category1. Validation Chek is needed for Safety.
    For example, I think a validation check is needed for avg(float4),
     whose transition type is not internal. (See p.18 in [1])
    I plan to add import functions of avg, count (See p.18, p.19 in [1]).
  Category1. Transition type is a pseudo data type.
    Aggregate functions of this category needs to accept many actual data types,
    including user-defined types. So I think that it is hard to implement import/export functions.
    Consequently, I do not plan to support these category. (See p.19 in [1])

Sincerely yours,
Yuki Fujii

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

[1] https://www.postgresql.org/message-id/attachment/160659/PGConfDev2024_Presentation_Aggregation_Scale...






view thread (45+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: RE: Partial aggregates pushdown
  In-Reply-To: <TYAPR01MB3088EEECFEB79434E0F4339695C72@TYAPR01MB3088.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox