Re: Partial aggregates pushdown

On Mon, Apr 10, 2023 at 01:18:37AM +0000, Fujii.Yuki@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  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Embrace your flaws.  They make you human, rather than perfect,
  which you will never be.