On Mon, Jun 12, 2023 at 08:51:30AM +0000, fujii.y...@df.mitsubishielectric.co.jp 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 <br...@momjian.us> > > 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 <br...@momjian.us> > > 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 <br...@momjian.us> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
