public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bruce Momjian <[email protected]>
To: [email protected] <[email protected]>
Cc: Alexander Pyhalov <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Ilya Gladyshev <[email protected]>
Subject: Re: Partial aggregates pushdown
Date: Mon, 19 Jun 2023 20:42:45 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <OS3PR01MB666038A12CE1F9D81785D27E9555A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
References: <[email protected]>
<OS3PR01MB6660EBFFFA06646F402813A395989@OS3PR01MB6660.jpnprd01.prod.outlook.com>
<[email protected]>
<OS3PR01MB66607DC466F7F3B3909DA3CA954EA@OS3PR01MB6660.jpnprd01.prod.outlook.com>
<[email protected]>
<OS3PR01MB66604E404B003A621B0C03A69552A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
<[email protected]>
<OS3PR01MB6660B7C247726BC0E36DF4D69554A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
<[email protected]>
<OS3PR01MB666038A12CE1F9D81785D27E9555A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
On Tue, Jun 13, 2023 at 02:18:15AM +0000, [email protected] wrote:
> Hi Mr.Momjian.
>
> Thank you for advises.
>
> > From: Bruce Momjian <[email protected]>
> > Sent: Monday, June 12, 2023 10:38 PM
> > > I understand what the problem is. I will put a mechanism maintaining compatibility into the patch.
> > > I believe there are three approaches.
> > > Approach 1-1 is preferable because it does not require additional options for postgres_fdw.
> > > I will revise the patch according to Approach 1-1, unless otherwise commented.
> > >
> > > Approach1:
> > > I ensure that postgres_fdw retrieves the version of each remote server
> > > and does not partial aggregate pushd down if the server version is less than 17.
> > > There are two approaches to obtaining remote server versions.
> > > Approach1-1: postgres_fdw connects a remote server and use PQserverVersion().
> > > Approach1-2: Adding a postgres_fdw option about a remote server version (like "server_version").
> > >
> > > Approach2:
> > > Adding a postgres_fdw option for partial aggregate pushdown is enable
> > > or not (like enable_partial_aggregate_pushdown).
> >
> > These are good questions. Adding a postgres_fdw option called enable_partial_aggregate_pushdown helps make the
> > purpose of the option clear, but remote_version can be used for future breakage as well.
> >
> > I think remote_version is the best idea, and in the documentation for the option, let's explicitly say it is useful to disable
> > partial aggregates pushdown on pre-PG 17 servers. If we need to use the option for other cases, we can just update the
> > documentation. When the option is blank, the default, everything is pushed down.
> >
> > I see remote_version a logical addition to match our "extensions" option that controls what extension functions can be
> > pushed down.
>
> Thank you for your perspective.
> So, of the approaches I have presented, you think that approach 1-2 is
> preferable and that the option name remote_server is preferable?
> Indeed, the option of a remote version may have other uses.
> However, this information can be obtained by connecting to a remote server,
> I'm concerned that some people may find this option redundant.
>
> Is the problem with approach 1-1 because the user cannot decide whether to include the compatibility check in the decision to do partial aggregate pushdown or not?
> # If Approach 1-1 is taken, the problem is that this feature cannot be used for all bait-in aggregate functions
> # when the remote server is older than PG17.
> If so, Approache1-3 below seem more desirable.
> Would it be possible for us to hear your thoughts?
>
> Approache1-3：We add a postgres_fdw option about a compatibility check for partial aggregate pushdown
> (like "enable_aggpartialfunc_compatibility_check"). This option is false, the default.
> When this option is true, postgres_fdw obtains the remote server version by connecting the remote server and using PQserverVersion().
> And if the remote server version is older than PG17, then the partial aggregate pushdown feature is enable for all buit-in aggregate functions.
> Otherwise the partial aggregate pushdown feature is disable for all buit-in aggregate functions.
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.
3) Looking at postgresAcquireSampleRowsFunc(), I see this exact method
used:
conn = GetConnection(user, false, NULL);
/* We'll need server version, so fetch it now. */
server_version_num = PQserverVersion(conn);
...
if ((server_version_num < 95000) &&
(method == ANALYZE_SAMPLE_SYSTEM ||
method == ANALYZE_SAMPLE_BERNOULLI))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("remote server does not support TABLESAMPLE feature")));
I am sorry if you already knew all this, but I didn't.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
view thread (38+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Partial aggregates pushdown
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox