RE: Partial aggregates pushdown

Hi. Bruce.

Sorry for the late. Thank you for comment.

> From: Bruce Momjian <bruce(at)momjian(dot)us>
> 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_Scaleout_FDW_Sharding_20240531.pdf