Re: [HACKERS] WIP: Aggregation push-down

Поиск
Список
Период
Сортировка
От Jeevan Chalke
Тема Re: [HACKERS] WIP: Aggregation push-down
Дата
Msg-id CAM2+6=W2J-iaQBgj-sdMERELQLUm5dvOQEWQ2ho+Q7KZgnonkQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] WIP: Aggregation push-down  (Antonin Houska <ah@cybertec.at>)
Ответы Re: [HACKERS] WIP: Aggregation push-down  (Antonin Houska <ah@cybertec.at>)
Список pgsql-hackers
Hi Antonin,

To understand the feature you have proposed, I have tried understanding
your patch. Here are few comments so far on it:

1.
+         if (aggref->aggvariadic ||
+             aggref->aggdirectargs || aggref->aggorder ||
+             aggref->aggdistinct || aggref->aggfilter)

I did not understand, why you are not pushing aggregate in above cases?
Can you please explain?

2. "make check" in contrib/postgres_fdw crashes.

  SELECT COUNT(*) FROM ft1 t1;
! server closed the connection unexpectedly
!   This probably means the server terminated abnormally
!   before or while processing the request.
! connection to server was lost

From your given setup, if I wrote a query like:
EXPLAIN VERBOSE SELECT count(*) FROM orders_0;
it crashes.

Seems like missing few checks.

3. In case of pushing partial aggregation on the remote side, you use schema
named "partial", I did not get that change. If I have AVG() aggregate,
then I end up getting an error saying
"ERROR:  schema "partial" does not exist".

4. I am not sure about the code changes related to pushing partial
aggregate on the remote side. Basically, you are pushing a whole aggregate
on the remote side and result of that is treated as partial on the
basis of aggtype = transtype. But I am not sure that is correct unless
I miss something here. The type may be same but the result may not.

5. There are lot many TODO comments in the patch-set, are you working
on those?

Thanks


On Thu, Aug 17, 2017 at 8:52 PM, Antonin Houska <ah@cybertec.at> wrote:
Antonin Houska <ah@cybertec.at> wrote:

> Antonin Houska <ah@cybertec.at> wrote:
>
> > This is a new version of the patch I presented in [1].
>
> Rebased.
>
> cat .git/refs/heads/master
> b9a3ef55b253d885081c2d0e9dc45802cab71c7b

This is another version of the patch.

Besides other changes, it enables the aggregation push-down for postgres_fdw,
although only for aggregates whose transient aggregation state is equal to the
output type. For other aggregates (like avg()) the remote nodes will have to
return the transient state value in an appropriate form (maybe bytea type),
which does not depend on PG version.

shard.tgz demonstrates the typical postgres_fdw use case. One query shows base
scans of base relation's partitions being pushed to shard nodes, the other
pushes down a join and performs aggregation of the join result on the remote
node. Of course, the query can only references one particular partition, until
the "partition-wise join" [1] patch gets committed and merged with this my
patch.

One thing I'm not sure about is whether the patch should remove
GetForeignUpperPaths function from FdwRoutine, which it essentially makes
obsolete. Or should it only be deprecated so far? I understand that
deprecation is important for "ordinary SQL users", but FdwRoutine is an
interface for extension developers, so the policy might be different.

[1] https://commitfest.postgresql.org/14/994/

Any feedback is appreciated.

--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers




--
Jeevan Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Etsuro Fujita
Дата:
Сообщение: Re: [HACKERS] Tuple-routing for certain partitioned tables notworking as expected
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: [HACKERS] Re: [COMMITTERS] pgsql: pg_rewind: Fix some problemswhen copying files >2GB.