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 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. Thank you for your opinion. I will modify this patch according to the way1.
> > > 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. Yes. The last version of this patch(and original postgres_fdw) checks if user-defined aggregate depends some extension which is contained in 'extensions'. But, in the last version of this patch, there is no such check for aggpartialfn of user-defined aggregate. So, I will add such check to this patch. I think that this modification is easy to do . > In summary, we don't do any version check for built-in function pushdown, so > we don't need it for aggregates either. We check extension functions against > the extension pushdown list, so we should be checking this for partial > aggregate pushdown, and for partition-wise aggregate pushdown. If we don't > do that last check already, it is a bug. I understood. Sincerely yours, Yuuki Fujii -- Yuuki Fujii Information Technology R&D Center Mitsubishi Electric Corporation
