MIME-Version: 1.0
Date: Wed, 30 Nov 2022 16:30:24 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: "Fujii.Yuki@df.MitsubishiElectric.co.jp"
 <Fujii.Yuki@df.mitsubishielectric.co.jp>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, PostgreSQL-development
 <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, Julien
 Rouhaud <rjuju123@gmail.com>, Daniel Gustafsson <daniel@yesql.se>, Ilya
 Gladyshev <i.gladyshev@postgrespro.ru>
Subject: Re: Partial aggregates pushdown
In-Reply-To: 
 <OS3PR01MB66604EFC363C61E6D589C53F95159@OS3PR01MB6660.jpnprd01.prod.outlook.com>
References: <05d98f67a4f44b31d90dfb977db4ad71@postgrespro.ru>
 <D327E6BD-D368-48E4-A8A8-4B59BFB48649@yesql.se>
 <317f8de58ce9889e056f2944e200a44d@postgrespro.ru>
 <E1C82737-B148-4975-81C9-5366E4412553@yesql.se>
 <2ed9ad03bc17cfb86da900d541a270b0@postgrespro.ru>
 <20220114121653.mjzqelwgnzwqhkoa@jrouhaud>
 <0cc653ad90dbfc5ca0fd8bb1ca57e1a7@postgrespro.ru>
 <CALNJ-vSm6z6Lg1CgxbGUG6=xP3n+tYC_XGGUm33b9g98Z3bF-g@mail.gmail.com>
 <2ad047732a8b8a9aaaad06fef59fb283@postgrespro.ru>
 <925e2d3f11283d8b2707fc5d827fc3e9@postgrespro.ru>
 <20220322004914.opkwl3beedl4uhks@alap3.anarazel.de>
 <57db913f-02c6-66af-2928-513e2b1c65f7@enterprisedb.com>
 <8dc86e502fbd9d04da0dc6cdd4156f84@postgrespro.ru>
 <OS3PR01MB66604C11E390FECBAA1B6518959A9@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <OS3PR01MB66607C2ABBDAEE150F32E24C950D9@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <313df801f860f653ebb8c6422794a5b5@postgrespro.ru>
 <OS3PR01MB6660E2F999B97D8ADA32AE6B95159@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <c82ca8dba5307cc150a7d156fbd36a9e@postgrespro.ru>
 <OS3PR01MB66604EFC363C61E6D589C53F95159@OS3PR01MB6660.jpnprd01.prod.outlook.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <4c02341e5e1b10f35c7b3dfbc73084f0@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/4c02341e5e1b10f35c7b3dfbc73084f0%40postgrespro.ru>
Precedence: bulk

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