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 patc

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 pointed

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

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

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

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 on

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

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 aggrega

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 fut

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

en, > after we get the results back and combine the various partial 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 aggrega

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

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

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 f

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

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 > https://github.com/swarm64/s6

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 se

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 an

Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop()

Hi. Tender. > From: Tender Wang > Sent: Tuesday, June 11, 2024 5:11 PM > > > From: Tender Wang mailto:tndrw...@gmail.com> > > > Sent: Tuesday, June 4, 2024 7:51 PM > > Please add more tests. Especially please add some negative > tests; > > current patch chec

RE: Partial aggregates pushdown

Hi. Bruce. Sorry for the late. Thank you for comment. > From: Bruce Momjian > Sent: Wednesday, June 5, 2024 11:04 PM > > > * Passes unsafe binary data from the foreign server. > > > > > > Can someone show me where that last item is in the patch, and why > > > can't we just pass back values cast

RE: Partial aggregates pushdown

Hi Jelte, Thank you for your comment! > From: Jelte Fennema-Nio > Sent: Tuesday, June 11, 2024 11:00 PM > How about instead of trying to serialize the output of serialfn/deserialfn, > instead we don't use the "internal" type and > create actual types in pg_type for these transtypes? Then we can

RE: Partial aggregates pushdown

Hi Alexander, hackers. > From: Alexander Pyhalov > Sent: Tuesday, May 28, 2024 2:45 PM > The fix was to set child_agg->agg_partial to orig_agg->agg_partial in > convert_combining_aggrefs(), it's already in the patch, as well as the > example - > without this fix I've just realized that you've ad

RE: Partial aggregates pushdown

Hi. Jelte, hackers. Thank you for your proposal and comments. > From: Jelte Fennema-Nio > Sent: Monday, June 24, 2024 6:09 PM > > 1. Generality > > I believe we should develop a method that can theoretically apply to any > aggregate function, even if we cannot implement it immediately. However,

RE: Partial aggregates pushdown

Hi Jelte, hackers. Thank you for explanations. Actually, I have other tasks about "PARTIAL_AGGREAGATE" keyword to respond Requirement1 and Requirement2 in the following mail. https://www.postgresql.org/message-id/TYAPR01MB3088755F2281D41F5EEF06D495F92%40TYAPR01MB3088.jpnprd01.prod.outlook.com Af

RE: Partial aggregates pushdown

> From: Fujii Yuki > Sent: Monday, July 1, 2024 6:42 AM > Hi hackers. > > On Wed, Jun 5, 2024 at 9:15?AM fujii.y...@df.mitsubishielectric.co.jp > wrote: > > Requirement2. Consider appropriate position of the new keyword > > "PARTIAL_AGGREGATE". (wit

RE: Partial aggregates pushdown

Hi Jelte and hackers, I've reconsidered which of the following two approaches is the best. Approach1: Adding export/import functions to transmit state values. Approach 2: Adding native types which are equal to state values. In my mind, Approach1 is superior. Therefore, if there are no objecti

RE: Partial aggregates pushdown

Hi Bruce. > From: Bruce Momjian > Is there a reason the documentation is no longer a part of this patch? > Can I help you keep it current? Here are the reasons: Reason1. The approach differs significantly from the previous patch that included documentation, the below. https://www.postgre

RE: Partial aggregates pushdown

Hi Jelte, Thank you for comments and advises. > From: Jelte Fennema-Nio > Sent: Monday, July 8, 2024 5:31 PM > On Sun, 7 Jul 2024 at 23:46, fujii.y...@df.mitsubishielectric.co.jp > wrote: > > In my mind, Approach1 is superior. Therefore, if there are no objections > &

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: Should consider materializing the cheapest inner path in consider_parallel_nestloop()

Hi. Tender. Thank you for modification. > From: Tender Wang > Sent: Tuesday, June 4, 2024 7:51 PM > Please add more tests. Especially please add some negative tests; > current patch checks that it is safe to apply materialization. It would > be helpful to add tests checking th

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