RE: Partial aggregates pushdown - Mailing list pgsql-hackers
From | Fujii.Yuki@df.MitsubishiElectric.co.jp" |
---|---|
Subject | RE: Partial aggregates pushdown |
Date | |
Msg-id | TYAPR01MB55141D18188AC86ADCE35FCB952F2@TYAPR01MB5514.jpnprd01.prod.outlook.com Whole thread Raw |
In response to | Re: Partial aggregates pushdown (Alexander Pyhalov <a.pyhalov@postgrespro.ru>) |
Responses |
Re: Partial aggregates pushdown
Re: Partial aggregates pushdown |
List | pgsql-hackers |
Hi. Mr.Pyhalov. > From: Alexander Pyhalov <a.pyhalov@postgrespro.ru> > Sent: Wednesday, February 28, 2024 10:43 PM > > 1. Transmitting state value safely between machines > >> From: Robert Haas <robertmhaas@gmail.com> > >> Sent: Wednesday, December 6, 2023 10:25 PM the problems around > >> transmitting serialized bytea blobs between machines that can't be > >> assumed to fully trust each other will need to be addressed in some > >> way, which seems like it will require a good deal of design work, > >> forming some kind of consensus, and then implementation work to > >> follow. > > I have considered methods for safely transmitting state values between > > different machines. > > I have taken into account the version policy of PostgreSQL (5 years of > > support) and the major version release cycle over the past 10 years (1 > > year), and as a result, I have made the assumption that transmission > > is allowed only when the difference between the local version and the > > remote version is 5 or less. > > I believe that by adding new components, "export function" and "import > > function", to the aggregate functions, and further introducing a new > > SQL keyword to the query syntax of aggregate expressions, we can > > address this issue. > > ... > > I honestly think that achieving cross-version compatibility in this way puts a significant burden on developers. Can we > instead always use the more or less universal export and import function to fix possible issues with binary representations > on different architectures and just refuse to push down partial aggregates on server version mismatch? At least at thefirst > step? Thank you for your comment. I agree with your point that the proposed method would impose a significant burden on developers.In order to ensure cross-version compatibility, it is necessary to impose constraints on the format of the statevalues exchanged between servers, which would indeed burden developers. As you mentioned, I think that it is realisticto allow partial aggregation pushdown only when coordinating between the same versions in the first step. > From: Alexander Pyhalov <a.pyhalov@postgrespro.ru> > Sent: Wednesday, February 28, 2024 10:43 PM > > 3. Fixing the behavior when the HAVING clause is present > >> From: Robert Haas <robertmhaas@gmail.com> > >> Sent: Tuesday, November 28, 2023 4:08 AM > >> > >> On Wed, Nov 22, 2023 at 1:32 AM Alexander Pyhalov > >> <a.pyhalov@postgrespro.ru> 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 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 > >> 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 have made modifications in the attached patch to ensure that when > > the HAVING clause is present, the HAVING clause is executed locally > > while the partial aggregations are pushed down. > > > > > > Sorry, I don't see how it works. When we have partial aggregates and having clause, foreign_grouping_ok() returns falseand > add_foreign_grouping_paths() adds no paths. > I'm not saying it's necessary to fix this in the first patch version. Our sincere apologies. I had attached an older version before this modification. > From: Alexander Pyhalov <a.pyhalov@postgrespro.ru> > Sent: Wednesday, February 28, 2024 10:43 PM > contrib/postgres_fdw/deparse.c: comment before appendFunctionName() has gone, this seems to be wrong. Fixed. > From: Alexander Pyhalov <a.pyhalov@postgrespro.ru> > Sent: Wednesday, February 28, 2024 10:43 PM > In finalize_aggregate() > > 1079 /* > 1080 * Apply the agg's finalfn if one is provided, else return > transValue. > 1081 */ > > Comment should be updated to note behavior for agg_partial aggregates. Fixed. > From: Alexander Pyhalov <a.pyhalov@postgrespro.ru> > Sent: Wednesday, February 28, 2024 10:43 PM > In this if branch, should we check just for peragg->aggref->agg_partial and peragg->aggref->aggtranstype == > INTERNALOID? It seems that if > peragg->aggref->aggtranstype == INTERNALOID and there's no > serialfn_oid, it's likely an error (and one should be generated). As you pointed out, I have made modifications to the source code so that it terminates with an error if serialfn is invalid. Sincerely yours, Yuuki Fujii -- Yuuki Fujii Information Technology R&D Center Mitsubishi Electric Corporation
Attachment
pgsql-hackers by date: