MIME-Version: 1.0
Date: Tue, 20 Jun 2023 09:59:11 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: Bruce Momjian <bruce@momjian.us>
Cc: "Fujii.Yuki@df.MitsubishiElectric.co.jp"
 <Fujii.Yuki@df.mitsubishielectric.co.jp>, PostgreSQL-development
 <pgsql-hackers@postgresql.org>, 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>, Ilya
 Gladyshev <i.gladyshev@postgrespro.ru>
Subject: Re: Partial aggregates pushdown
In-Reply-To: <ZJD2BWVqRlJgz3n6@momjian.us>
References: <ZDemPMLOyXNzKkhw@momjian.us>
 <OS3PR01MB6660EBFFFA06646F402813A395989@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZDj4nwJANUU+kam8@momjian.us>
 <OS3PR01MB66607DC466F7F3B3909DA3CA954EA@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <8ffedecae0d21a3cd93805baf1dcc0de@postgrespro.ru>
 <OS3PR01MB66604E404B003A621B0C03A69552A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZINW022iuYzf2ZwO@momjian.us>
 <OS3PR01MB6660B7C247726BC0E36DF4D69554A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZIcfn8pnO4t29O5g@momjian.us>
 <OS3PR01MB666038A12CE1F9D81785D27E9555A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <ZJD2BWVqRlJgz3n6@momjian.us>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <f47c4cb0965a8de2a19322970e7ba7e3@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/f47c4cb0965a8de2a19322970e7ba7e3%40postgrespro.ru>
Precedence: bulk

Bruce Momjian писал 2023-06-20 03:42:
> Apologies for the delay in my reply to this email.  I looked into the
> existing code and I found three things:
> 
> 1)  PQserverVersion() just pulls the conn->sversion value from the
> existing connection because pqSaveParameterStatus() pulls the
> server_version sent by the backend;  no need to issue SELECT version().
> 
> 2)  postgres_fdw already has nine calls to GetConnection(), and only
> opens a connection if it already doesn't have one.  Here is an example:
> 
> 	/* Get the remote estimate */
> 	conn = GetConnection(fpinfo->user, false, NULL);
> 	get_remote_estimate(sql.data, conn, &rows, &width,
> 			    &startup_cost, &total_cost);
> 	ReleaseConnection(conn);
> 
> Therefore, it seems like it would be near-zero cost to just call conn =
> GetConnection() and then PQserverVersion(conn), and 
> ReleaseConnection().
> You can then use the return value of PQserverVersion() to determine if
> you can push down partial aggregates.
> 

Hi.
Currently we don't get remote connection while planning if 
use_remote_estimate is not set.
Such change would require to get remote connection in planner, not in 
executor.
This can lead to change of behavior (like errors in explain when user 
mapping is wrong - e.g. bad password is specified).
Also this potentially can lead to establishing connections even when 
plan node is not actually used
(like extreme example - select sum(score) from t limit 0).
I'm not saying we shouldn't do it - just hint at possible consequences.

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional