public inbox for [email protected]  

help / color / mirror / Atom feed
From: Alexander Pyhalov <[email protected]>
To: [email protected] <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Ilya Gladyshev <[email protected]>
Subject: Re: Partial aggregates pushdown
Date: Mon, 05 Jun 2023 12:00:27 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <OS3PR01MB66607DC466F7F3B3909DA3CA954EA@OS3PR01MB6660.jpnprd01.prod.outlook.com>
References: <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]>
	<OS3PR01MB6660EBFFFA06646F402813A395989@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<OS3PR01MB66607DC466F7F3B3909DA3CA954EA@OS3PR01MB6660.jpnprd01.prod.outlook.com>

[email protected] писал 2023-06-02 06:54:
> Hi Mr.Bruce, hackers.
> 
> I updated the patch.
> The following is a list of comments received on the previous version
> of the patch
> and my update to them in this version of the patch.
> 

Hi.

I've looked through the last version of the patch.

Have found one issue -

src/backend/catalog/pg_aggregate.c

585                 if(strcmp(strVal(linitial(aggpartialfnName)), 
aggName) == 0){
586                         if(((aggTransType != INTERNALOID) && 
(finalfn != InvalidOid))
587                                         || ((aggTransType == 
INTERNALOID) && (finalfn != serialfn)))
588                                 elog(ERROR, "%s is not its own 
aggpartialfunc", aggName);
589                 } else {

Here string comparison of aggName and aggpartialfnName looks very 
suspicios - it seems you should compare oids, not names (in this case,
likely oids of transition function and partial aggregation function). 
The fact that aggregate name matches partial aggregation function name
is not a enough to make any decisions.


In documentation

doc/src/sgml/postgres-fdw.sgml:

  930    <filename>postgres_fdw</filename> attempts to optimize remote 
queries to reduce
  931    the amount of data transferred from foreign servers.  This is 
done by
  932    sending query <literal>WHERE</literal> clauses and ggregate 
expressions
  933    to the remote server for execution, and by not retrieving table 
columns that
  934    are not needed for the current query.
  935    To reduce the risk of misexecution of queries,
  936    <literal>WHERE</literal> clauses and ggregate expressions are 
not sent to
  937    the remote server unless they use only data types, operators, 
and functions
  938    that are built-in or belong to an extension that's listed in the 
foreign
  939    server's <literal>extensions</literal> option.
  940    Operators and functions in such clauses must
  941    be <literal>IMMUTABLE</literal> as well.

there are misprints in lines 932 and 936 - missing "a" in "aggregate" 
expressions.

Note that after these changes "select sum()" will fail for certain 
cases, when remote server version is not the latest. In other cases we 
tried
to preserve compatibility. Should we have a switch for a foreign server 
to turn this optimization off? Or do we just state that users
should use different workarounds if remote server version doesn't match 
local one?

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional






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: <[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