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: Finnerty, Jim <[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, 25 Sep 2023 18:30:32 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <OS3PR01MB6660FA1EC43389E75E0CB9C795FCA@OS3PR01MB6660.jpnprd01.prod.outlook.com>
References: <[email protected]>
<OS3PR01MB666038A12CE1F9D81785D27E9555A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
<[email protected]>
<[email protected]>
<ZJMavbn5trJ/[email protected]>
<OS3PR01MB6660315447C037EB4E6193D39522A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
<[email protected]>
<OS3PR01MB6660DF84634EDAFC55D7B0009530A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
<ZNE4XiLsV/[email protected]>
<OS3PR01MB6660FA1EC43389E75E0CB9C795FCA@OS3PR01MB6660.jpnprd01.prod.outlook.com>
On Mon, Sep 25, 2023 at 03:18:13AM +0000, [email protected] wrote:
> Hi Mr.Bruce, Mr.Pyhalov, Mr.Finnerty, hackers.
>
> Thank you for your valuable comments. I sincerely apologize for the very late reply.
> Here is a response to your comments or a fix to the patch.
>
> Tuesday, August 8, 2023 at 3:31 Bruce Momjian
> > > I have modified the program except for the point "if the version of the remote server is less than PG17".
> > > Instead, we have addressed the following.
> > > "If check_partial_aggregate_support is true and the remote server version is older than the local server
> > > version, postgres_fdw does not assume that the partial aggregate function is on the remote server unless
> > > the partial aggregate function and the aggregate function match."
> > > The reason for this is to maintain compatibility with any aggregate function that does not support partial
> > > aggregate in one version of V1 (V1 is PG17 or higher), even if the next version supports partial aggregate.
> > > For example, string_agg does not support partial aggregation in PG15, but it will support partial aggregation
> > > in PG16.
> >
> > Just to clarify, I think you are saying:
> >
> > If check_partial_aggregate_support is true and the remote server
> > version is older than the local server version, postgres_fdw
> > checks if the partial aggregate function exists on the remote
> > server during planning and only uses it if it does.
> >
> > I tried to phrase it in a positive way, and mentioned the plan time
> > distinction. Also, I am sorry I was away for most of July and am just
> > getting to this.
> Thanks for your comment. In the documentation, the description of check_partial_aggregate_support is as follows
> (please see postgres-fdw.sgml).
> --
> check_partial_aggregate_support (boolean)
> Only if this option is true, during query planning, postgres_fdw connects to the remote server and check if the remote server version is older than the local server version. If so, postgres_fdw assumes that for each built-in aggregate function, the partial aggregate function is not defined on the remote server unless the partial aggregate function and the aggregate function match. The default is false.
> --
My point is that there are three behaviors:
* false - no check
* true, remote version >= sender - no check
* true, remove version < sender - check
Here is your code:
+ * Check that a buit-in aggpartialfunc exists on the remote server. If
+ * check_partial_aggregate_support is false, we assume the partial aggregate
+ * function exsits on the remote server. Otherwise we assume the partial
+ * aggregate function exsits on the remote server only if the remote server
+ * version is not less than the local server version.
+ */
+static bool
+is_builtin_aggpartialfunc_shippable(Oid aggpartialfn, PgFdwRelationInfo *fpinfo)
+{
+ bool shippable = true;
+
+ if (fpinfo->check_partial_aggregate_support)
+ {
+ if (fpinfo->remoteversion == 0)
+ {
+ PGconn *conn = GetConnection(fpinfo->user, false, NULL);
+
+ fpinfo->remoteversion = PQserverVersion(conn);
+ }
+ if (fpinfo->remoteversion < PG_VERSION_NUM)
+ shippable = false;
+ }
+ return shippable;
+}
I think this needs to be explained in the docs. I am ready to adjust
the patch to improve the wording whenever you are ready. Should I do it
now and post an updated version for you to use?
--
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], [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