On Mon, Apr 10, 2023 at 01:18:37AM +0000, 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 not control, that need this documentation. > The last version of this patch can't pushdown partial aggregate for all > builtin aggregate functions that can support it. > I will improve this patch to pushdown partial aggregate for all builtin > aggregate functions > that can support it. > > 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 to this assumption, someone should add aggpartialfns definitions of new > builtin aggregate functions to BKI files. > There are two possible ways to address this issue. Would the way1 be > sufficient? > Or would way2 be preferable? > way1) Adding documentaion for how to add these definitions to BKI files > way2) Improving this patch to automatically add these definitions to BKI > files by some tool such as initdb.
I think documentation is sufficient. You already showed that someone can do this with CREATE AGGREGATE for non-builtin aggregates. > > So, let's remove the PARTIALAGG_MINVERSION option from the patch and just > > make it automatic --- we push down builtin partial aggregates if the remote > > server is the same or newer _major_ version than the sending server. For > > extensions, if people have older extensions on the same or newer foreign > > servers, they can adjust 'extensions' above. > Okay, I understand. I will remove PARTIALAGG_MINVERSION option from the patch > and I will add check whether aggpartialfn depends on some extension which > is containd in extensions list of the postgres_fdw's foreign server. 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. > 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. -- Bruce Momjian <br...@momjian.us> https://momjian.us EDB https://enterprisedb.com Embrace your flaws. They make you human, rather than perfect, which you will never be.
