Re: Partial aggregates pushdown

2025-03-29 Thread Bruce Momjian
On Fri, Mar 28, 2025 at 02:00:44AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > Hi Bruce, Jelte, hackers. > > I apologize for my late response. > > > From: Jelte Fennema-Nio > > Sent: Thursday, August 8, 2024 8:49 PM > > SUMMARY OF THREAD > > > > The design of patch 0001 is agreed up

Re: Partial aggregates pushdown

2024-08-22 Thread Tomas Vondra
On 8/22/24 22:07, Bruce Momjian wrote: > On Thu, Aug 22, 2024 at 09:54:02PM +0200, Tomas Vondra wrote: >> On 8/22/24 20:56, Bruce Momjian wrote: >>> You make a very good point above. Would there ever be cases where a >>> targetlist would have multiple aggregates, and some can be pushed down, >>>

Re: Partial aggregates pushdown

2024-08-22 Thread Bruce Momjian
On Thu, Aug 22, 2024 at 09:54:02PM +0200, Tomas Vondra wrote: > On 8/22/24 20:56, Bruce Momjian wrote: > > You make a very good point above. Would there ever be cases where a > > targetlist would have multiple aggregates, and some can be pushed down, > > and some have to return all matching rows s

Re: Partial aggregates pushdown

2024-08-22 Thread Tomas Vondra
On 8/22/24 20:56, Bruce Momjian wrote: > On Wed, Aug 21, 2024 at 05:41:02PM +0200, Tomas Vondra wrote: >> On 8/8/24 13:48, Jelte Fennema-Nio wrote: >>> SUMMARY OF THREAD >>> >>> The design of patch 0001 is agreed upon by everyone on the thread (so >>> far). This adds the PARTIAL_AGGREGATE label for

Re: Partial aggregates pushdown

2024-08-22 Thread Bruce Momjian
On Thu, Aug 22, 2024 at 08:31:11PM +0200, Tomas Vondra wrote: > > My question is related to #3 and #4. For #3, if we are going to be > > building infrastructure to handle passing int128 for AVG, wouldn't it be > > wiser to create an int128 type and an int128 array type, and then use > > method #2

Re: Partial aggregates pushdown

2024-08-22 Thread Bruce Momjian
On Wed, Aug 21, 2024 at 05:41:02PM +0200, Tomas Vondra wrote: > On 8/8/24 13:48, Jelte Fennema-Nio wrote: > > SUMMARY OF THREAD > > > > The design of patch 0001 is agreed upon by everyone on the thread (so > > far). This adds the PARTIAL_AGGREGATE label for aggregates, which will > > cause the fin

Re: Partial aggregates pushdown

2024-08-22 Thread Tomas Vondra
On 8/22/24 19:22, Bruce Momjian wrote: > On Wed, Aug 21, 2024 at 04:59:12PM +0200, Tomas Vondra wrote: >> On 8/20/24 20:41, Bruce Momjian wrote: >>> SELECT (oid, relname) FROM pg_class LIMIT 1; >>> row >>> - >>> (2619,pg_statistic) >>> >>> SELECT pg

Re: Partial aggregates pushdown

2024-08-22 Thread Bruce Momjian
On Wed, Aug 21, 2024 at 04:59:12PM +0200, Tomas Vondra wrote: > On 8/20/24 20:41, Bruce Momjian wrote: > > SELECT (oid, relname) FROM pg_class LIMIT 1; > > row > > - > > (2619,pg_statistic) > > > > SELECT pg_typeof((oid, relname)) FROM pg_class LIM

Re: Partial aggregates pushdown

2024-08-22 Thread Ashutosh Bapat
On Wed, Aug 21, 2024 at 9:11 PM Tomas Vondra wrote: > > > > On 8/8/24 13:48, Jelte Fennema-Nio wrote: > > SUMMARY OF THREAD > > > > The design of patch 0001 is agreed upon by everyone on the thread (so > > far). This adds the PARTIAL_AGGREGATE label for aggregates, which will > > cause the finalfu

Re: Partial aggregates pushdown

2024-08-21 Thread Tomas Vondra
On 8/8/24 13:48, Jelte Fennema-Nio wrote: > SUMMARY OF THREAD > > The design of patch 0001 is agreed upon by everyone on the thread (so > far). This adds the PARTIAL_AGGREGATE label for aggregates, which will > cause the finalfunc not to run. It also starts using PARTIAL_AGGREGATE > for pushdow

Re: Partial aggregates pushdown

2024-08-21 Thread Tomas Vondra
On 8/20/24 20:41, Bruce Momjian wrote: > On Tue, Aug 20, 2024 at 07:03:56PM +0200, Jelte Fennema-Nio wrote: >> On Tue, 20 Aug 2024 at 18:50, Bruce Momjian wrote: >>> Okay, so we can do MAX easily, and AVG if the count can be represented >>> as the same data type as the sum? Is that correct? O

Re: Partial aggregates pushdown

2024-08-20 Thread Bruce Momjian
On Tue, Aug 20, 2024 at 07:03:56PM +0200, Jelte Fennema-Nio wrote: > On Tue, 20 Aug 2024 at 18:50, Bruce Momjian wrote: > > Okay, so we can do MAX easily, and AVG if the count can be represented > > as the same data type as the sum? Is that correct? Our only problem is > > that something like AV

Re: Partial aggregates pushdown

2024-08-20 Thread Jelte Fennema-Nio
On Tue, 20 Aug 2024 at 18:50, Bruce Momjian wrote: > Okay, so we can do MAX easily, and AVG if the count can be represented > as the same data type as the sum? Is that correct? Our only problem is > that something like AVG(interval) can't use an array because arrays have > to have the same data

Re: Partial aggregates pushdown

2024-08-20 Thread Bruce Momjian
On Tue, Aug 20, 2024 at 10:07:32AM +0200, Jelte Fennema-Nio wrote: > On Thu, 15 Aug 2024 at 23:12, Bruce Momjian wrote: > > Third, I would like to show a more specific example to clarify what is > > being considered above. If we look at MAX(), we can have FDWs return > > the max for each FDW, and

Re: Partial aggregates pushdown

2024-08-20 Thread Jelte Fennema-Nio
On Thu, 15 Aug 2024 at 23:12, Bruce Momjian wrote: > Third, I would like to show a more specific example to clarify what is > being considered above. If we look at MAX(), we can have FDWs return > the max for each FDW, and the coordinator can chose the highest value. > This is the patch 1 listed

Re: Partial aggregates pushdown

2024-08-15 Thread Bruce Momjian
On Thu, Aug 8, 2024 at 01:48:49PM +0200, Jelte Fennema-Nio wrote: > SUMMARY OF THREAD > > The design of patch 0001 is agreed upon by everyone on the thread (so > far). This adds the PARTIAL_AGGREGATE label for aggregates, which will > cause the finalfunc not to run. It also starts using PARTIAL_A

Re: Partial aggregates pushdown

2024-08-15 Thread Bruce Momjian
On Sun, Jul 7, 2024 at 09:52:27PM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > 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 differ

Re: Partial aggregates pushdown

2024-08-08 Thread Jelte Fennema-Nio
SUMMARY OF THREAD The design of patch 0001 is agreed upon by everyone on the thread (so far). This adds the PARTIAL_AGGREGATE label for aggregates, which will cause the finalfunc not to run. It also starts using PARTIAL_AGGREGATE for pushdown of aggregates in postgres_fdw. In 0001 PARTIAL_AGGREGAT

Re: Partial aggregates pushdown

2024-07-08 Thread Jelte Fennema-Nio
On Mon, 8 Jul 2024 at 14:12, fujii.y...@df.mitsubishielectric.co.jp wrote: > The best thing about Approach2 is that it lets us send state values using the > existing data type system. > I'm worried that if we choose Approach2, we might face some limits because we > have to create new types. > Bu

RE: Partial aggregates pushdown

2024-07-08 Thread fujii.y...@df.mitsubishielectric.co.jp
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 > > this week, I plan to resu

Re: Partial aggregates pushdown

2024-07-08 Thread Jelte Fennema-Nio
On Sun, 7 Jul 2024 at 23:52, fujii.y...@df.mitsubishielectric.co.jp wrote: > My plan for advancing the patch involves the following steps: > Step1. Decide the approach on transmitting state value. > Step2. Implement code (including comments) and tests to support a subset of > aggregate functi

Re: Partial aggregates pushdown

2024-07-08 Thread Jelte Fennema-Nio
On Sun, 30 Jun 2024 at 23:42, fujii.y...@df.mitsubishielectric.co.jp wrote: > Instead, I can make PARTIAL_AGGREGATE an unreserved word by placing it after > the FILTER clause, like avg(c1) FILTER (WHERE c2 > 0) PARTIAL_AGGREGATE, and > by marking it as an ASLABEL word like FILTER. > I attached t

Re: Partial aggregates pushdown

2024-07-08 Thread Jelte Fennema-Nio
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 this > week, I plan to resume implementing Approach1 next week. I would appreciate > it if anyone could discuss the topic with me or ask questions

RE: Partial aggregates pushdown

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

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

2024-07-05 Thread Bruce Momjian
On Sun, Jun 30, 2024 at 09:42:19PM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > On Mon, Jun 24, 2024 at 6:09?PM Jelte Fennema-Nio wrote: > > 4. Related to 3, I think it would be good to have some tests of > > PARTIAL_AGGREGATE that don't involve postgres_fdw at all. I also > > spotted s

RE: Partial aggregates pushdown

2024-06-30 Thread fujii.y...@df.mitsubishielectric.co.jp
> 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". (with Robert) > > Existing patch: Before th

Re: Partial aggregates pushdown

2024-06-25 Thread Jelte Fennema-Nio
ently, so far, I want to support the cases in which local and remote > have the same major version. > If we try to resolve the limitation, it seems to need more additional codes. Hmm, that's a very good point. Indeed cross-major-version partial aggregates pushdown would not be fu

RE: Partial aggregates pushdown

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

2024-06-24 Thread Jelte Fennema-Nio
On Mon, 24 Jun 2024 at 15:03, fujii.y...@df.mitsubishielectric.co.jp wrote: > I see. I maybe got your proposal. > Refer to your proposal, for avg(int8), > I create a new native type like state_int8_avg > with the new typsend/typreceive functions > and use them to transmit the state value, right?

RE: Partial aggregates pushdown

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

2024-06-24 Thread Jelte Fennema-Nio
On Sun, 23 Jun 2024 at 10:24, fujii.y...@df.mitsubishielectric.co.jp wrote: > I attached the POC patch(the second one) which supports avg(int8) whose > standard format is _numeric type. Okay, very interesting. So instead of defining the serialization/deserialization functions to text/binary, you

RE: Partial aggregates pushdown

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

2024-06-12 Thread Jelte Fennema-Nio
On Wed, 12 Jun 2024 at 07:27, fujii.y...@df.mitsubishielectric.co.jp wrote: > Could you please clarify what you mean? > Are you referring to: > Option 1: Modifying existing aggregate functions to minimize the use of > internal state values. > Option 2: Not supporting the push down of partial

RE: Partial aggregates pushdown

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

2024-06-11 Thread Jelte Fennema-Nio
On Tue, 11 Jun 2024 at 13:40, fujii.y...@df.mitsubishielectric.co.jp wrote: > > From: Bruce Momjian > > So, we need import/export text representation for the partial aggregate > > mode for these eight, and call the base data type > > text import/export functions for the zero ones when in this mo

RE: Partial aggregates pushdown

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

2024-06-05 Thread Bruce Momjian
On Wed, Jun 5, 2024 at 08:19:04AM +0300, Alexander Pyhalov wrote: > > * 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 to text? > > In finalize_aggregate() when we see partial a

Re: Partial aggregates pushdown

2024-06-04 Thread Alexander Pyhalov
Bruce Momjian писал(а) 2024-06-04 20:12: On Mon, May 27, 2024 at 09:30:59PM +, fujii.y...@df.mitsubishielectric.co.jp wrote: Hi Mr. Pyhalov. Sorry for the late reply. Thank you for your modification and detailed review. I attach a fixed patch, have been not yet rebased. I know this patch

Re: Partial aggregates pushdown

2024-06-04 Thread Bruce Momjian
On Wed, Jun 5, 2024 at 12:14:45AM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > I will add sufficient document and comments to the next patch as Bruce and > Robert said. Great, I am available to help improve the documentation. -- Bruce Momjian https://momjian.us EDB

Re: Partial aggregates pushdown

2024-06-04 Thread Bruce Momjian
On Mon, May 27, 2024 at 09:30:59PM +, fujii.y...@df.mitsubishielectric.co.jp wrote: > Hi Mr. Pyhalov. > > Sorry for the late reply. > Thank you for your modification and detailed review. > I attach a fixed patch, have been not yet rebased. I know this patch was discussed at the Vancouver con

Re: Partial aggregates pushdown

2024-05-27 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал(а) 2024-05-28 00:30: Hi Mr. Pyhalov. Sorry for the late reply. Thank you for your modification and detailed review. I attach a fixed patch, have been not yet rebased. Monday, 25 March 2024 16:01 Alexander Pyhalov :. Comment in nodeAgg.c seems to be

Re: Partial aggregates pushdown

2024-05-27 Thread Alexander Pyhalov
fujii.y...@df.mitsubishielectric.co.jp писал(а) 2024-05-28 00:30: Hi Mr. Pyhalov. Hi. Found one more problem. You can fire partial aggregate over partitioned table, but convert_combining_aggrefs() will make non-partial copy, which leads to 'variable not found in subplan target list' error.

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 to

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 pointed

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

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 pa

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 conc

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

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 th

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 aggregat

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 pa

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

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

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 widesp

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 ridi

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 aggrega

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 a

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

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 f

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 di

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 fut

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 whe

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 fut

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 it

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

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 aggrega

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 partia

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

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

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 versi

Re: Partial aggregates pushdown

2023-09-27 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 cl

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 suspici

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 you

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 Mom

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

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 ver

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 qualified

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 resul

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 tha

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 vers

RE: Partial aggregates pushdown

2023-06-21 Thread fujii.y...@df.mitsubishielectric.co.jp
r is disable. Sincerely yours, Yuuki Fujii -- Yuuki Fujii Information Technology R&D 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最

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

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 u

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

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

  1   2   >