public inbox for [email protected]  

help / color / mirror / Atom feed
From: Alexander Pyhalov <[email protected]>
To: [email protected] <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Zhihong Yu <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Ilya Gladyshev <[email protected]>
Subject: Re: Partial aggregates pushdown
Date: Wed, 30 Nov 2022 11:12:24 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <OS3PR01MB6660E2F999B97D8ADA32AE6B95159@OS3PR01MB6660.jpnprd01.prod.outlook.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<20220114121653.mjzqelwgnzwqhkoa@jrouhaud>
	<[email protected]>
	<CALNJ-vSm6z6Lg1CgxbGUG6=xP3n+tYC_XGGUm33b9g98Z3bF-g@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<OS3PR01MB66604C11E390FECBAA1B6518959A9@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<OS3PR01MB66607C2ABBDAEE150F32E24C950D9@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<OS3PR01MB6660E2F999B97D8ADA32AE6B95159@OS3PR01MB6660.jpnprd01.prod.outlook.com>

Hi, Yuki.

1) In previous version of the patch aggregates, which had partialaggfn, 
were ok to push down. And it was a definite sign that aggregate can be 
pushed down. Now we allow pushing down an aggregate, which prorettype is 
not internal and aggfinalfn is not defined. Is it safe for all 
user-defined (or builtin) aggregates, even if they are generally 
shippable? Aggcombinefn is executed locally and we check that aggregate 
function itself is shippable. Is it enough? Perhaps, we could use 
partialagg_minversion (like aggregates with partialagg_minversion == -1 
should not be pushed down) or introduce separate explicit flag?

2) Do we really have to look at pg_proc in partial_agg_ok() and 
deparseAggref()? Perhaps, looking at aggtranstype is enough?

3) I'm not sure if CREATE AGGREGATE tests with invalid 
PARTIALAGGFUNC/PARTIALAGG_MINVERSION should be in postgres_fdw tests or 
better should be moved to src/test/regress/sql/create_aggregate.sql, as 
they are not specific to postgres_fdw

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional





view thread (46+ 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]
  Subject: Re: Partial aggregates pushdown
  In-Reply-To: <[email protected]>

* 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