Re: Partial aggregates pushdown

Fujii(dot)Yuki(at)df(dot)MitsubishiElectric(dot)co(dot)jp писал 2023-06-02 06:54:
> Hi Mr.Bruce, hackers.
>
> I updated the patch.
> The following is a list of comments received on the previous version
> of the patch
> and my update to them in this version of the patch.
>

Hi.

I've looked through the last version of the patch.

Have found one issue -

src/backend/catalog/pg_aggregate.c

585 if(strcmp(strVal(linitial(aggpartialfnName)),
aggName) == 0){
586 if(((aggTransType != INTERNALOID) &&
(finalfn != InvalidOid))
587 || ((aggTransType ==
INTERNALOID) && (finalfn != serialfn)))
588 elog(ERROR, "%s is not its own
aggpartialfunc", aggName);
589 } else {

Here string comparison of aggName and aggpartialfnName looks very
suspicios - it seems you should compare oids, not names (in this case,
likely oids of transition function and partial aggregation function).
The fact that aggregate name matches partial aggregation function name
is not a enough to make any decisions.

In documentation

doc/src/sgml/postgres-fdw.sgml:

930 <filename>postgres_fdw</filename> attempts to optimize remote
queries to reduce
931 the amount of data transferred from foreign servers. This is
done by
932 sending query <literal>WHERE</literal> clauses and ggregate
expressions
933 to the remote server for execution, and by not retrieving table
columns that
934 are not needed for the current query.
935 To reduce the risk of misexecution of queries,
936 <literal>WHERE</literal> clauses and ggregate expressions are
not sent to
937 the remote server unless they use only data types, operators,
and functions
938 that are built-in or belong to an extension that's listed in the
foreign
939 server's <literal>extensions</literal> option.
940 Operators and functions in such clauses must
941 be <literal>IMMUTABLE</literal> as well.

there are misprints in lines 932 and 936 - missing "a" in "aggregate"
expressions.

Note that after these changes "select sum()" will fail for certain
cases, when remote server version is not the latest. In other cases we
tried
to preserve compatibility. Should we have a switch for a foreign server
to turn this optimization off? Or do we just state that users
should use different workarounds if remote server version doesn't match
local one?

--
Best regards,
Alexander Pyhalov,
Postgres Professional