public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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, 12 Jun 2023 09:37:35 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <OS3PR01MB6660B7C247726BC0E36DF4D69554A@OS3PR01MB6660.jpnprd01.prod.outlook.com>
References: <OS3PR01MB66606C1DA960336516D9A08395959@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<[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>

On Mon, Jun 12, 2023 at 08:51:30AM +0000, [email protected] wrote:
> Hi Mr.Bruce, Mr.Pyhalov, hackers.
> 
> Thank you for comments. I will try to respond to both of your comments as follows.
> I plan to start revising the patch next week. If you have any comments on the following
> respondences, I would appreciate it if you could give them to me this week.
> 
> > From: Bruce Momjian <[email protected]>
> > Sent: Saturday, June 10, 2023 1:44 AM
> > I agree that this feature is designed for built-in sharding, but it is possible people could be using aggregates on partitions
> > backed by foreign tables without sharding.  Adding a requirement for non-sharding setups to need PG 17+ servers might
> > be unreasonable.
> Indeed, it is possible to use partial aggregate pushdown feature for purposes other than sharding.
> The description of the section "F.38.6. Built-in sharding in PostgreSQL" assumes the use of
> Built-in sharding and will be modified to eliminate this assumption.
> The title of this section should be changed to something like "Aggregate on partitioned table".

Sounds good.

> > From: Bruce Momjian <[email protected]>
> > Sent: Saturday, June 10, 2023 1:44 AM
> > Looking at previous release note incompatibilities, we don't normally change non-administrative functions in a way that
> > causes errors if run on older servers.  Based on Alexander's observations, I wonder if we need to re-add the postgres_fdw
> > option to control partial aggregate pushdown, and default it to enabled.
> > 
> > If we ever add more function breakage we might need more postgres_fdw options.  Fortunately, such changes are rare.
> 
> 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 documention for the
option, let's explcitly say it is useful to disable partial aggreates
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.

-- 
  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