Re: Partial aggregates pushdown

Fujii(dot)Yuki(at)df(dot)MitsubishiElectric(dot)co(dot)jp писал 2022-11-30 13:01:
> Hi Mr.Pyhalov.
>
>> 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?
> In what case partial aggregate pushdown is unsafe for aggregate which
> has not internal aggtranstype
> and has no aggfinalfn?
> By reading [1], I believe that if aggcombinefn of such aggregate
> recieves return values of original
> aggregate functions in each remote then it must produce same value
> that would have resulted
> from scanning all the input in a single operation.
>

One more issue I started to think about - now we don't check
partialagg_minversion for "simple" aggregates at all. Is it correct? It
seems that , for example, we could try to pushdown bit_or(int8) to old
servers, but it didn't exist, for example, in 8.4. I think it's a
broader issue (it would be also the case already if we push down
aggregates) and shouldn't be fixed here. But there is an issue -
is_shippable() is too optimistic.

--
Best regards,
Alexander Pyhalov,
Postgres Professional