Re: Partial aggregates pushdown

Fujii.Yuki@df.MitsubishiElectric.co.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