RE: Partial aggregates pushdown

Hi. Mr.Momjian, Mr.Lane, Mr.Haas, hackers. I apologize for any misunderstanding regarding the context of the attached patch and the points on which I requested a review. Could you please allow me to clarify? In the review around early December 2023, I received the following three issues

RE: [CAUTION!! freemail] Re: Partial aggregates pushdown

Hi Mr.Haas. > -Original Message- > From: Robert Haas > Sent: Wednesday, December 6, 2023 10:25 PM > On Wed, Dec 6, 2023 at 3:41 AM fujii.y...@df.mitsubishielectric.co.jp > wrote: > > Are you concerned about the hassle and potential human errors of > >

RE: Partial aggregates pushdown

ial counts locally, > we would locally evaluate the HAVING clause > afterward. That is, partial aggregation is a barrier to pushing down HAVING > clause itself, but it doesn't preclude pushing > down the aggregation. I understand what the problem is. I will try to fix it in the next

RE: Partial aggregates pushdown

Hi Mr.Momjian, Mr.Haas, hackers. > From: Bruce Momjian > Sent: Thursday, November 23, 2023 6:16 AM > On Wed, Nov 22, 2023 at 10:16:16AM +, > fujii.y...@df.mitsubishielectric.co.jp wrote: > > 2. Approach 2 > > (1) Advantages > > (a) No need to add partial aggre

RE: Partial aggregates pushdown

Hi Mr. Haas, hackers. Thank you for your thoughtful comments. > From: Robert Haas > Sent: Tuesday, November 21, 2023 5:52 AM > I do have a concern about this, though. It adds a lot of bloat. It adds a > whole lot of additional entries to pg_aggregate, and > every new aggregate we add in the

RE: Partial aggregates pushdown

Hi Momjian. Thank you for your improvement. As a matter of detail, I think that the areas marked below are erroneous. -- + Pushdown causes aggregate function cals to send partial aggregate ^ + function calls to the remote server. If the partial

RE: Partial aggregates pushdown

Hi Mr.Momjian. > Fujii-san, to get this patch closer to finished, can I modify this version of > the patch to improve some wording and post an > updated version you can use for future changes? Yes, I greatly appreciate your offer. I would very much appreciate your modifications. Sincerely

RE: Partial aggregates pushdown

Hi Mr.Pyhalov. > From: Alexander Pyhalov > Sent: Friday, July 14, 2023 10:40 PM > 1) In foreign_join_ok() should we set fpinfo->user if > fpinfo->check_partial_aggregate_support is set like it's done for > fpinfo->use_remote_estimate? It seems we can end up with fpinfo->user > fpinfo->= > NULL

RE: Partial aggregates pushdown

Hi Mr.Momjian, Mr.Pyhalov, hackers. > From: Bruce Momjian > Sent: Thursday, June 22, 2023 12:44 AM > On Tue, Jun 20, 2023 at 09:59:11AM +0300, Alexander Pyhalov wrote: > > > Therefore, it seems like it would be near-zero cost to just call > > > conn = > > > GetConnection() and then

RE: Partial aggregates pushdown

Hi Mr.Momjian. Thank you for advises. > From: Bruce Momjian > 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

RE: Partial aggregates pushdown

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

RE: Partial aggregates pushdown

Hi Mr.Momjian. > > There is one more thing I would like your opinion on. > > As the major version of PostgreSQL increase, it is possible that the > > new builtin aggregate functions are added to the newer PostgreSQL. > > This patch assume that aggpartialfns definitions exist in BKI files. > > Due

RE: Partial aggregates pushdown

Hi Mr.Momjian, Mr.Lane, Mr.Freund. Thank you for advices. > From: Bruce Momjian > > > > 2. Automation of creating definition of partialaggfuncs In > > > > development of v17, I manually create definition of > > > > partialaggfuncs for avg, min, max, sum, > > > count. > > > > I am concerned that

RE: Partial aggregates pushdown

Hi Mr.Momjian > First, my apologies for not addressing this sooner. I was so focused on my > own tasks that I didn't realize this very important patch was not getting > attention. I will try my best to get it into PG 17. Thank you very much for your comments. I will improve this patch for

RE: Partial aggregates pushdown

Hi Mr.Momjian > First, am I correct? Yes, you are correct. This patch uses new special aggregate functions for partial aggregate (then we call this partialaggfunc). > Second, how far away is this from being committable > and/or what work needs to be done to get it committable, either for PG 16

RE: Partial aggregates pushdown

Hi Mr.Freund. > cfbot complains about some compiler warnings when building with clang: > https://cirrus-ci.com/task/6606268580757504 > > deparse.c:3459:22: error: equality comparison with extraneous parentheses > [-Werror,-Wparentheses-equality] > if ((node->aggsplit == AGGSPLIT_SIMPLE))

RE: Add semi-join pushdown to postgres_fdw

Hi Mr.Pyhalov. Thank you for fixing it and giving more explanation. > IIRC, planner can create semi-join, which targetlist references Vars > from inner join relation. However, it's deparsed as exists and so we > can't reference it from SQL. So, there's this check - if Var is > referenced in

RE: Partial aggregates pushdown

Hi Mr.Pyhalov. > Attaching minor fixes. I haven't proof-read all comments (but perhaps, they > need attention from some native speaker). Thank you. I fixed according to your patch. And I fixed have proof-read all comments and messages. > Tested it with queries from >

RE: Add semi-join pushdown to postgres_fdw

Hi Mr.Pyhalov. Thank you for work on this useful patch. I'm starting to review v2 patch. I have cheked we can apply v2 patch to commit ec386948948c1708c0c28c48ef08b9c4dd9d47cc (Date:Thu Dec 1 12:56:21 2022 +0100). I briefly looked at this whole thing and did step execute this by running simple

RE: Partial aggregates pushdown

Hi Mr.Pyhalov. > One more issue I started to think about - now we don't check > partialagg_minversion for "simple" aggregates at all. Is it correct? It seems > that , > for example, we could try to pushdown bit_or(int8) to old servers, but it > didn't > exist, for example, in 8.4. I think it's

RE: Partial aggregates pushdown

Hi Mr.Pyhalov. > 1) In previous version of the patch aggregates, which had partialaggfn, were > ok > to push down. And it was a definite sign that aggregate can be pushed down. > Now we allow pushing down an aggregate, which prorettype is not internal and > aggfinalfn is not defined. Is it safe

RE: Partial aggregates pushdown

Hi Mr.Pyhalov. Thank you for comments. > I've looked through the patch. Overall I like this approach, but have > the following comments. > > 1) Why should we require partialaggfn for min()/max()/count()? We could > just use original functions for a lot of aggregates, and so it would be >

RE: Partial aggregates pushdown

Hi Mr.Vondra, Mr.Pyhalov. I'm interesied in Mr.Pyhalov's patch due to the following background. --Background I develop postgresql's extension such as fdw in my work. I'm interested in using postgresql for OLAP. I think the function of a previous patch "Push aggregation down to base relations

WIP: Aggregation push-down - take2

Hi everyone. I develop postgresql's extension such as fdw in my work. I'm interested in using postgresql for OLAP. After [1] having been withdrawn, I reviewed [1]. I think that this patch is realy useful when using OLAP queries. Furthermore, I think it would be more useful if this patch works

RE: WIP: Aggregation push-down

Hi everyone. I develop postgresql's extension such as fdw in my work. I'm interested in using postgresql for OLAP. I think that this patch is realy useful when using OLAP queries. Furthermore, I think it would be more useful if this patch works on a foreign table. Actually, I changed this