Hi, Yuki.
1) In previous version of the patch aggregates, which had partialaggfn,
were ok to push down. And it was a definite sign that aggregate can be
pushed down. Now we allow pushing down an aggregate, which prorettype is
not internal and aggfinalfn is not defined. Is it safe for all
user-defined (or builtin) aggregates, even if they are generally
shippable? Aggcombinefn is executed locally and we check that aggregate
function itself is shippable. Is it enough? Perhaps, we could use
partialagg_minversion (like aggregates with partialagg_minversion == -1
should not be pushed down) or introduce separate explicit flag?
2) Do we really have to look at pg_proc in partial_agg_ok() and
deparseAggref()? Perhaps, looking at aggtranstype is enough?
3) I'm not sure if CREATE AGGREGATE tests with invalid
PARTIALAGGFUNC/PARTIALAGG_MINVERSION should be in postgres_fdw tests or
better should be moved to src/test/regress/sql/create_aggregate.sql, as
they are not specific to postgres_fdw
--
Best regards,
Alexander Pyhalov,
Postgres Professional