MIME-Version: 1.0
Date: Wed, 27 Sep 2023 15:36:34 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: "Fujii.Yuki@df.MitsubishiElectric.co.jp"
 <Fujii.Yuki@df.mitsubishielectric.co.jp>
Cc: Bruce Momjian <bruce@momjian.us>, PostgreSQL-development
 <pgsql-hackers@postgresql.org>, "Finnerty, Jim" <jfinnert@amazon.com>,
 Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Tomas
 Vondra <tomas.vondra@enterprisedb.com>, Julien Rouhaud <rjuju123@gmail.com>,
 Daniel Gustafsson <daniel@yesql.se>
Subject: Re: Partial aggregates pushdown
In-Reply-To: <OS3PR01MB66608567A5DE381D99BEE1A395C3A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
References: <ZJD2BWVqRlJgz3n6@momjian.us>
 <f47c4cb0965a8de2a19322970e7ba7e3@postgrespro.ru>
 <ZJMavbn5trJ/wQj+@momjian.us>
 <OS3PR01MB6660315447C037EB4E6193D39522A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZJQy4gLC7WwZyKUe@momjian.us>
 <OS3PR01MB6660DF84634EDAFC55D7B0009530A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZNE4XiLsV/Wo4egh@momjian.us>
 <OS3PR01MB6660FA1EC43389E75E0CB9C795FCA@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZRIKCB3GsKtOIM64@momjian.us>
 <OS3PR01MB6660CF2F3643AE7B7C8F797495C3A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZRLnvjkgiPGhXNyF@momjian.us>
 <OS3PR01MB66608567A5DE381D99BEE1A395C3A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <3c97067f40503bd8ebb3659f5daa2ff5@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/3c97067f40503bd8ebb3659f5daa2ff5%40postgrespro.ru>
Precedence: bulk

Fujii.Yuki@df.MitsubishiElectric.co.jp писал 2023-09-27 01:35:
> Hi Mr.Momjian, Mr.Pyhalov.
> 
> Tuesday, 26 September 2023 22:15 Alexander Pyhalov 
> <a.pyhalov@postgrespro.ru>:
>> Do you mean that extra->partial_target->sortgrouprefs is not replaced,
>> and so we preserve tlesortgroupref numbers?
> Yes, that is correct.
> 
>> I'm suspicious about rewriting extra->partial_target->exprs with
>> partial_target->exprs - I'm still not sure why we
>>   don't we loose information, added by add_column_to_pathtarget() to
>> extra->partial_target->exprs?
>> 

Hi.

In postgres_fdw.sql

"Partial aggregates are unsafe to push down having clause when there are 
partial aggregates" - this comment likely should be fixed.

Some comments should be added to setGroupClausePartial() and to 
make_partial_grouping_target() - especially why setGroupClausePartial()
is called prior to add_new_columns_to_pathtarget().

I'm not sure that I like this mechanics of adding sort group clauses - 
it seems we do in core additional work, which is of use only for
one extension, but at least it seems to be working.

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional