public inbox for [email protected]  
help / color / mirror / Atom feed
From: [email protected] <[email protected]>
To: Bruce Momjian <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Alexander Pyhalov <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Ilya Gladyshev <[email protected]>
Cc: [email protected] <[email protected]>
Subject: RE: Partial aggregates pushdown
Date: Thu, 13 Apr 2023 10:56:26 +0000
Message-ID: <OS3PR01MB6660EBFFFA06646F402813A395989@OS3PR01MB6660.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <OS3PR01MB6660CB590AEF3644A9EEE07D95189@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<OS3PR01MB6660A1DAF6619B9A0BF7D39B95E19@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<OS3PR01MB6660B900F33523AAD0370D62958F9@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<OSZPR01MB6662875933E87F37C711B4FC95969@OSZPR01MB6662.jpnprd01.prod.outlook.com>
	<ZDDIU2fJxZm/[email protected]>
	<OS3PR01MB66606C1DA960336516D9A08395959@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<[email protected]>

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






view thread (30+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: RE: Partial aggregates pushdown
  In-Reply-To: <OS3PR01MB6660EBFFFA06646F402813A395989@OS3PR01MB6660.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox