Re: Partial aggregates pushdown

2024-03-25 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал(а) 2024-03-16 05:28: Hi. Mr.Pyhalov. >> >> I don't see it that way. What we would push to the foreign server >> would be something like SELECT count(a) FROM t. Then, after we get >> the results back and combine the various partial counts locally, we

Re: Partial aggregates pushdown

2024-03-21 Thread Bruce Momjian
On Thu, Mar 21, 2024 at 11:37:50AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > 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

RE: Partial aggregates pushdown

2024-03-21 Thread fujii.y...@df.mitsubishielectric.co.jp
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: Partial aggregates pushdown

2024-03-19 Thread Bruce Momjian
On Tue, Mar 19, 2024 at 05:29:07PM -0400, Tom Lane wrote: > I'd like to vociferously protest both of those decisions. > > "No version check by default" means "unsafe by default", which is not > project style in general and is especially not so for postgres_fdw. > We have tried very hard for years

Re: Partial aggregates pushdown

2024-03-19 Thread Tom Lane
Bruce Momjian writes: > The current patch has: > if ((OidIsValid(aggform->aggfinalfn) || > (aggform->aggtranstype == INTERNALOID)) && > fpinfo->check_partial_aggregate_support) > { > if (fpinfo->remoteversion == 0) > { >

Re: Partial aggregates pushdown

2024-03-19 Thread Bruce Momjian
On Sat, Mar 16, 2024 at 02:28:50AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > Hi. Mr.Pyhalov. > > > From: Alexander Pyhalov Sent: Wednesday, > > February 28, 2024 10:43 PM > > > 1. Transmitting state value safely between machines > > >> From: Robert Haas Sent: Wednesday, > > >>

Re: Partial aggregates pushdown

2024-02-28 Thread Alexander Pyhalov
Hi. fujii.y...@df.mitsubishielectric.co.jp писал(а) 2024-02-22 10:20: Hi. Mr.Haas, hackers. I apologize for the significant delay since my last post. I have conducted investigations and considerations regarding the remaining tasks as follows. Would it be possible for you to review them? In

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

2024-01-26 Thread vignesh C
On Thu, 7 Dec 2023 at 05:41, fujii.y...@df.mitsubishielectric.co.jp wrote: > > 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

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

2023-12-11 Thread Robert Haas
On Thu, Dec 7, 2023 at 4:12 PM Bruce Momjian wrote: > Second, the patch already has a mechanism to check the remote server > version to see if it is the same or newer. Here is the version check > documentation patch: Right. This feature can certainly be implemented in a backward-compatible

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

2023-12-07 Thread Bruce Momjian
On Thu, Dec 7, 2023 at 09:56:08AM -0500, Robert Haas wrote: > On Wed, Dec 6, 2023 at 7:11 PM fujii.y...@df.mitsubishielectric.co.jp > wrote: > > I would be grateful if we can resolve this issue gradually. I would also > > like to continue the discussion if possible in the future. > > I think

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

2023-12-07 Thread Robert Haas
On Wed, Dec 6, 2023 at 7:11 PM fujii.y...@df.mitsubishielectric.co.jp wrote: > I would be grateful if we can resolve this issue gradually. I would also like > to continue the discussion if possible in the future. I think that would be good. Thanks for your work on this. It is a hard problem.

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

2023-12-06 Thread fujii.y...@df.mitsubishielectric.co.jp
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 > > manually adding new partial

Re: Partial aggregates pushdown

2023-12-06 Thread Robert Haas
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 manually > adding new partial > aggregation functions, rather than the catalog becoming bloated? I'm concerned about both. > The process of creating

RE: Partial aggregates pushdown

2023-12-06 Thread fujii.y...@df.mitsubishielectric.co.jp
Hi Mr.Haas, hackers. > From: Robert Haas > Sent: Tuesday, November 28, 2023 5:03 AM > Also, I want to make one other point here about security and reliability. > Right now, there is no way for a user to feed > arbitrary data to a deserialization function. Since serialization and >

Re: Partial aggregates pushdown

2023-11-28 Thread Stephen Frost
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Mon, Nov 27, 2023 at 4:23 PM Tom Lane wrote: > > Well, one of the founding principles of postgres_fdw was to be able > > to talk to PG servers that are not of the same version as yours. > > If we break that in the name of performance,

Re: Partial aggregates pushdown

2023-11-28 Thread Robert Haas
On Tue, Nov 28, 2023 at 5:24 AM Ashutosh Bapat wrote: > If my memory serves me right, PGXC implemented partial aggregation > only when the output of partial aggregate was a SQL data type > (non-Internal, non-Unknown). But I may be wrong. But at that time, > JSONB wasn't there or wasn't that

Re: Partial aggregates pushdown

2023-11-28 Thread Ashutosh Bapat
On Tue, Nov 28, 2023 at 5:21 AM Robert Haas wrote: > > TBH, I suspect even some PG forks have made this work, like maybe PGXC > or PGXL, although I don't know for certain. We might not like the > trade-offs they made to get there, but we haven't even talked through > possible design ideas yet, so

Re: Partial aggregates pushdown

2023-11-27 Thread Robert Haas
First of all, that last email of mine was snippy, and I apologize for it. Sorry. That said: On Mon, Nov 27, 2023 at 4:23 PM Tom Lane wrote: > Well, one of the founding principles of postgres_fdw was to be able > to talk to PG servers that are not of the same version as yours. > If we break that

Re: Partial aggregates pushdown

2023-11-27 Thread Tom Lane
Robert Haas writes: > On Mon, Nov 27, 2023 at 3:59 PM Tom Lane wrote: >> TBH, I think this entire proposal is dead in the water. Which is >> sad from a performance standpoint, but I can't see any way that >> we would not regret shipping a feature that makes such assumptions. > I think it's

Re: Partial aggregates pushdown

2023-11-27 Thread Robert Haas
On Mon, Nov 27, 2023 at 3:59 PM Tom Lane wrote: > Even if the partial-aggregate serialization value isn't "internal" > but some more-narrowly-defined type, it is still an internal > implementation detail of the aggregate. You have no right to assume > that the remote server implements the

Re: Partial aggregates pushdown

2023-11-27 Thread Tom Lane
Robert Haas writes: > Also, I want to make one other point here about security and > reliability. Right now, there is no way for a user to feed arbitrary > data to a deserialization function. Since serialization and > deserialization functions are only used in the context of parallel > query, we

Re: Partial aggregates pushdown

2023-11-27 Thread Robert Haas
On Wed, Nov 22, 2023 at 5:16 AM fujii.y...@df.mitsubishielectric.co.jp wrote: > I did not choose Approach2 because I was not confident that the disadvantage > mentioned in 2.(2)(a) > would be accepted by the PostgreSQL development community. > If it is accepted, I think Approach 2 is smarter. >

Re: Partial aggregates pushdown

2023-11-27 Thread Robert Haas
On Wed, Nov 22, 2023 at 1:32 AM Alexander Pyhalov wrote: > Hi. HAVING is also a problem. Consider the following query > > SELECT count(a) FROM t HAVING count(a) > 10 - we can't push it down to > foreign server as HAVING needs full aggregate result, but foreign server > don't know it. I don't see

RE: Partial aggregates pushdown

2023-11-26 Thread fujii.y...@df.mitsubishielectric.co.jp
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 aggregate functions to the catalogs

Re: Partial aggregates pushdown

2023-11-22 Thread Bruce Momjian
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 aggregate functions to the catalogs for each > aggregation > (2) Disadvantages > (a) Need to add non-standard keywords to the SQL syntax. > > I

RE: Partial aggregates pushdown

2023-11-22 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-11-21 Thread Alexander Pyhalov
Robert Haas писал 2023-11-21 20:16: > I don't think the patch does a good job explaining why HAVING, > DISTINCT, and ORDER BY are a problem. It seems to me that HAVING > shouldn't really be a problem, because HAVING is basically a WHERE > clause that occurs after aggregation is complete, and

Re: Partial aggregates pushdown

2023-11-21 Thread Bruce Momjian
On Tue, Nov 21, 2023 at 12:16:41PM -0500, Robert Haas wrote: > On Mon, Nov 20, 2023 at 5:48 PM Bruce Momjian wrote: > > > 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

2023-11-21 Thread Robert Haas
On Mon, Nov 20, 2023 at 5:48 PM Bruce Momjian wrote: > > 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 future will require a bonus entry for this, > > and it needs a bunch of

Re: Partial aggregates pushdown

2023-11-20 Thread Bruce Momjian
On Mon, Nov 20, 2023 at 03:51:33PM -0500, Robert Haas wrote: > On Mon, Nov 13, 2023 at 3:26 AM fujii.y...@df.mitsubishielectric.co.jp > wrote: > > In postgres_fdw.sql, I have corrected the output format for floating point > > numbers > > by extra_float_digits. > > Looking at this, I find that

Re: Partial aggregates pushdown

2023-11-20 Thread Robert Haas
On Mon, Nov 13, 2023 at 3:26 AM fujii.y...@df.mitsubishielectric.co.jp wrote: > In postgres_fdw.sql, I have corrected the output format for floating point > numbers > by extra_float_digits. Looking at this, I find that it's not at all clear to me how the partial aggregate function is defined.

Re: Partial aggregates pushdown

2023-10-26 Thread Bruce Momjian
On Fri, Oct 27, 2023 at 02:44:42AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > 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 >

RE: Partial aggregates pushdown

2023-10-26 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-10-26 Thread Bruce Momjian
On Thu, Oct 26, 2023 at 11:11:09AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > >and checks 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

Re: Partial aggregates pushdown

2023-10-25 Thread Bruce Momjian
On Tue, Oct 24, 2023 at 12:12:41AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > 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? >

RE: Partial aggregates pushdown

2023-10-23 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-10-23 Thread Bruce Momjian
On Wed, Oct 18, 2023 at 05:22:34AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > Hi hackers. > > Because there is a degrade in pg_dump.c, I fixed it. 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

Re: Partial aggregates pushdown

2023-09-28 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-09-28 07:40: I'm not sure that I like this mechanics of adding sort group clauses - it seems we do in core additional work, which is of use only for one extension, but at least it seems to be working. We cannot deparse the original sort group

Re: Partial aggregates pushdown

2023-09-27 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-09-27 01:35: Hi Mr.Momjian, Mr.Pyhalov. Tuesday, 26 September 2023 22:15 Alexander Pyhalov : Do you mean that extra->partial_target->sortgrouprefs is not replaced, and so we preserve tlesortgroupref numbers? Yes, that is correct. I'm

Re: Partial aggregates pushdown

2023-09-26 Thread Bruce Momjian
On Tue, Sep 26, 2023 at 06:26:25AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > Hi Mr.Bruce. > > 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

Re: Partial aggregates pushdown

2023-09-26 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-09-25 06:18: 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

Re: Partial aggregates pushdown

2023-09-25 Thread Bruce Momjian
On Mon, Sep 25, 2023 at 03:18:13AM +, fujii.y...@df.mitsubishielectric.co.jp 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. > >

Re: Partial aggregates pushdown

2023-08-07 Thread Bruce Momjian
On Mon, Jul 10, 2023 at 07:35:27AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > > > I will add a postgres_fdw option "check_partial_aggregate_support". > > > This option is false, default. > > > Only if this option is true, postgres_fdw connect to the remote server > > > and get the

Re: Partial aggregates pushdown

2023-08-01 Thread Finnerty, Jim
When it is valid to filter based on a HAVING clause predicate, it should already have been converted into a WHERE clause predicate, except in the special case of an LIMIT TO .k .. ORDER BY case where the HAVING clause predicate can be determined approximately after having found k fully

Re: Partial aggregates pushdown

2023-07-20 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-07-19 03:43: Hi Mr.Pyhalov, hackers. 3) I modified the patch to safely do a partial aggregate pushdown for queries which contain having clauses. Hi. Sorry, but I don't see how it could work. For example, the attached test returns wrong

RE: Partial aggregates pushdown

2023-07-17 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-07-14 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-07-10 10:35: 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

Re: Partial aggregates pushdown

2023-06-22 Thread Bruce Momjian
On Thu, Jun 22, 2023 at 05:23:33AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > Approach1-3: > I will add a postgres_fdw option "check_partial_aggregate_support". > This option is false, default. > Only if this option is true, postgres_fdw connect to the remote server and > get the

RE: Partial aggregates pushdown

2023-06-21 Thread fujii.y...@df.mitsubishielectric.co.jp
Sincerely yours, Yuuki Fujii -- Yuuki Fujii Information Technology R Center Mitsubishi Electric Corporation > -Original Message- > From: Bruce Momjian > Sent: Thursday, June 22, 2023 12:44 AM > To: Alexander Pyhalov > Cc: Fujii Yuki/藤井 雄規(MELCO/情報総研 DM最適G) > ; > PostgreSQL-deve

Re: Partial aggregates pushdown

2023-06-21 Thread Bruce Momjian
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 PQserverVersion(conn), and ReleaseConnection(). > > You can then use the return value of PQserverVersion() to determine if > >

Re: Partial aggregates pushdown

2023-06-20 Thread Alexander Pyhalov
Bruce Momjian писал 2023-06-20 03:42: Apologies for the delay in my reply to this email. I looked into the existing code and I found three things: 1) PQserverVersion() just pulls the conn->sversion value from the existing connection because pqSaveParameterStatus() pulls the server_version

Re: Partial aggregates pushdown

2023-06-19 Thread Bruce Momjian
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 documentation for the > > option, let's explicitly say it is useful

RE: Partial aggregates pushdown

2023-06-12 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-06-12 Thread Bruce Momjian
On Mon, Jun 12, 2023 at 08:51:30AM +, 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 >

RE: Partial aggregates pushdown

2023-06-12 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-06-09 Thread Bruce Momjian
query changes). However, this patch changes the behavior of old > > queries, which worked prior to update. This seems to be different to me. > > You are right. > However, for now, partial aggregates pushdown is mainly used when using > built-in sharding in PostgreSQL. > I belie

Re: Partial aggregates pushdown

2023-06-09 Thread Alexander Pyhalov
Hi. + An aggregate function, called the partial aggregate function for partial aggregate + that corresponding to the aggregate function, is defined on the primary node and + the postgres_fdw node. Something is clearly wrong here. + When using built-in sharding feature in

Re: Partial aggregates pushdown

2023-06-08 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-06-08 02:08: From: Alexander Pyhalov Sent: Wednesday, June 7, 2023 6:47 PM This seems to be more robust, but the interface became more strange. I'm not sure what to do with it. Some ideas I had to avoid introducing this parameter. Not sure I

Re: Partial aggregates pushdown

2023-06-07 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-06-06 15:31: Thanks for the explanation. I understand that the method of comparing two function name strings is incorrect. Instead, I added the parameter isaggpartialfunc indicating whether the aggregate function and its aggpartialfunc are the

Re: Partial aggregates pushdown

2023-06-05 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-06-06 06:08: Hi Mr.Pyhalov. Thank you for your always thoughtful review. From: Alexander Pyhalov Sent: Monday, June 5, 2023 6:00 PM Have found one issue - src/backend/catalog/pg_aggregate.c 585

Re: Partial aggregates pushdown

2023-06-05 Thread Bruce Momjian
On Fri, Jun 2, 2023 at 03:54:06AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > Hi Mr.Bruce, hackers. > > I updated the patch. > The following is a list of comments received on the previous version of the > patch > and my update to them in this version of the patch. This thread

Re: Partial aggregates pushdown

2023-06-05 Thread Alexander Pyhalov
Bruce Momjian писал 2023-06-05 19:26: On Mon, Jun 5, 2023 at 12:00:27PM +0300, Alexander Pyhalov wrote: Note that after these changes "select sum()" will fail for certain cases, when remote server version is not the latest. In other cases we tried to preserve compatibility. Should we have a

Re: Partial aggregates pushdown

2023-06-05 Thread Bruce Momjian
On Mon, Jun 5, 2023 at 12:00:27PM +0300, Alexander Pyhalov wrote: > Note that after these changes "select sum()" will fail for certain cases, > when remote server version is not the latest. In other cases we tried > to preserve compatibility. Should we have a switch for a foreign server to > turn

Re: Partial aggregates pushdown

2023-06-05 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2023-06-02 06:54: Hi Mr.Bruce, hackers. I updated the patch. The following is a list of comments received on the previous version of the patch and my update to them in this version of the patch. Hi. I've looked through the last version of the

Re: Partial aggregates pushdown

2023-04-14 Thread Bruce Momjian
On Thu, Apr 13, 2023 at 10:56:26AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > > Yes, good. Did we never push down aggregates before? I thought we > > pushed down partitionwise aggregates already, and such a check should > > already be done. If the check isn't there, it should be. >

Re: Partial aggregates pushdown

2023-04-13 Thread Robert Haas
On Wed, Nov 30, 2022 at 3:12 AM Alexander Pyhalov wrote: > 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

RE: Partial aggregates pushdown

2023-04-13 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-04-13 Thread Bruce Momjian
On Thu, Apr 13, 2023 at 02:12:44AM -0400, Bruce Momjian wrote: > > In the next version of this patch, > > we can pushdown partial aggregate for an user-defined aggregate function > > only > > when the function pass through this check. > > Understood. In summary, we don't do any version check

Re: Partial aggregates pushdown

2023-04-13 Thread Bruce Momjian
On Mon, Apr 10, 2023 at 01:18:37AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > > Uh, we actually want the patch to implement partial aggregate pushdown for > > all > > builtin data types that can support it. Is that done? I think it is only > > extension > > aggregates, which we do

RE: Partial aggregates pushdown

2023-04-09 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-04-08 Thread Bruce Momjian
On Sat, Apr 8, 2023 at 10:15:40AM -0400, Bruce Momjian wrote: > On Fri, Apr 7, 2023 at 09:16:14PM -0700, Andres Freund wrote: > > extensions (string) > > > > This option is a comma-separated list of names of PostgreSQL extensions > > that are installed, in compatible versions, on both the

Re: Partial aggregates pushdown

2023-04-08 Thread Bruce Momjian
On Fri, Apr 7, 2023 at 09:16:14PM -0700, Andres Freund wrote: > On 2023-04-07 22:53:53 -0400, Bruce Momjian wrote: > > > postgres_fdw has no business pushing down calls to non-builtin functions > > > unless the user has explicitly authorized that via the existing > > > whitelisting mechanism. I

Re: Partial aggregates pushdown

2023-04-07 Thread Andres Freund
On 2023-04-07 22:53:53 -0400, Bruce Momjian wrote: > On Fri, Apr 7, 2023 at 10:44:09PM -0400, Tom Lane wrote: > > Bruce Momjian writes: > > > On Fri, Apr 7, 2023 at 09:55:00PM -0400, Tom Lane wrote: > > >> Uh, what? Why would we not be able to tell from the remote server's > > >> version

Re: Partial aggregates pushdown

2023-04-07 Thread Bruce Momjian
On Fri, Apr 7, 2023 at 10:53:53PM -0400, Bruce Momjian wrote: > On Fri, Apr 7, 2023 at 10:44:09PM -0400, Tom Lane wrote: > > Bruce Momjian writes: > > > On Fri, Apr 7, 2023 at 09:55:00PM -0400, Tom Lane wrote: > > >> Uh, what? Why would we not be able to tell from the remote server's > > >>

Re: Partial aggregates pushdown

2023-04-07 Thread Bruce Momjian
On Fri, Apr 7, 2023 at 10:44:09PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Fri, Apr 7, 2023 at 09:55:00PM -0400, Tom Lane wrote: > >> Uh, what? Why would we not be able to tell from the remote server's > >> version number whether it has this ability? > > > The issue is not a

Re: Partial aggregates pushdown

2023-04-07 Thread Tom Lane
Bruce Momjian writes: > On Fri, Apr 7, 2023 at 09:55:00PM -0400, Tom Lane wrote: >> Uh, what? Why would we not be able to tell from the remote server's >> version number whether it has this ability? > The issue is not a mismatch of postgres_fdw versions but the extension > versions and whether

Re: Partial aggregates pushdown

2023-04-07 Thread Bruce Momjian
On Fri, Apr 7, 2023 at 09:55:00PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > What I don't want is an error-prone setup where administrators have to > > remember what the per-server settings are. Based on your suggestion, > > let's allow CREATE SERVER to have an option

Re: Partial aggregates pushdown

2023-04-07 Thread Tom Lane
Bruce Momjian writes: > What I don't want is an error-prone setup where administrators have to > remember what the per-server settings are. Based on your suggestion, > let's allow CREATE SERVER to have an option 'enable_async_aggregate' (is > that the right name?), which defaults to 'true' for

Re: Partial aggregates pushdown

2023-04-07 Thread Bruce Momjian
On Fri, Apr 7, 2023 at 09:25:52AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > 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

RE: Partial aggregates pushdown

2023-04-07 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-04-06 Thread Bruce Momjian
On Fri, Mar 31, 2023 at 05:49:21AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > 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). First, my apologies for not

RE: Partial aggregates pushdown

2023-03-30 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2023-03-30 Thread Bruce Momjian
On Thu, Dec 15, 2022 at 10:23:05PM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > 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

RE: Partial aggregates pushdown

2022-12-15 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2022-12-07 Thread Andres Freund
Hi, On 2022-12-05 02:03:49 +, fujii.y...@df.mitsubishielectric.co.jp wrote: > > 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

RE: Partial aggregates pushdown

2022-12-04 Thread fujii.y...@df.mitsubishielectric.co.jp
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: Partial aggregates pushdown

2022-12-01 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2022-12-01 05:23: Hi Mr.Pyhalov. Hi. Attaching minor fixes. I haven't proof-read all comments (but perhaps, they need attention from some native speaker). Tested it with queries from https://github.com/swarm64/s64da-benchmark-toolkit, works as

RE: Partial aggregates pushdown

2022-11-30 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2022-11-30 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2022-11-30 13:01: 2) Do we really have to look at pg_proc in partial_agg_ok() and deparseAggref()? Perhaps, looking at aggtranstype is enough? You are right. I fixed according to your comment. partial_agg_ok() still looks at pg_proc. -- Best

Re: Partial aggregates pushdown

2022-11-30 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2022-11-30 13:01: 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

RE: Partial aggregates pushdown

2022-11-30 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2022-11-30 Thread Alexander Pyhalov
Hi, Yuki. 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 for all

RE: Partial aggregates pushdown

2022-11-29 Thread fujii.y...@df.mitsubishielectric.co.jp
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

2022-11-22 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал 2022-11-22 04:01: Hi Mr.Vondra, Mr.Pyhalov, Everyone. I discussed with Mr.Pyhalov about the above draft by directly sending mail to him(outside of pgsql-hackers). Mr.Pyhalov allowed me to update his patch along with the above draft. So I update

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

2022-11-22 Thread Ted Yu
On Tue, Nov 22, 2022 at 1:11 AM fujii.y...@df.mitsubishielectric.co.jp < fujii.y...@df.mitsubishielectric.co.jp> wrote: > Hi Mr.Yu. > > Thank you for comments. > > > + * Check that partial aggregate agg has compatibility > > > > If the `agg` refers to func parameter, the parameter name is aggform

Re: Partial aggregates pushdown

2022-11-21 Thread Ted Yu
On Mon, Nov 21, 2022 at 5:02 PM fujii.y...@df.mitsubishielectric.co.jp < fujii.y...@df.mitsubishielectric.co.jp> wrote: > Hi Mr.Vondra, Mr.Pyhalov, Everyone. > > I discussed with Mr.Pyhalov about the above draft by directly sending mail > to > him(outside of pgsql-hackers). Mr.Pyhalov allowed me

RE: Partial aggregates pushdown

2022-07-31 Thread fujii.y...@df.mitsubishielectric.co.jp
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

Re: Partial aggregates pushdown

2022-03-22 Thread Alexander Pyhalov
Tomas Vondra писал 2022-03-22 15:28: On 3/22/22 01:49, Andres Freund wrote: On 2022-01-17 15:27:53 +0300, Alexander Pyhalov wrote: Alexander Pyhalov писал 2022-01-17 15:26: Updated patch. Sorry, missed attachment. Needs another update: http://cfbot.cputube.org/patch_37_3369.log Marked as

Re: Partial aggregates pushdown

2022-03-22 Thread Tomas Vondra
On 3/22/22 01:49, Andres Freund wrote: > On 2022-01-17 15:27:53 +0300, Alexander Pyhalov wrote: >> Alexander Pyhalov писал 2022-01-17 15:26: >>> Updated patch. >> >> Sorry, missed attachment. > > Needs another update: http://cfbot.cputube.org/patch_37_3369.log > > Marked as waiting on author. >

Re: Partial aggregates pushdown

2022-03-21 Thread Andres Freund
On 2022-01-17 15:27:53 +0300, Alexander Pyhalov wrote: > Alexander Pyhalov писал 2022-01-17 15:26: > > Updated patch. > > Sorry, missed attachment. Needs another update: http://cfbot.cputube.org/patch_37_3369.log Marked as waiting on author. - Andres

  1   2   >