Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-24 Thread Jeevan Chalke
On Sat, Oct 22, 2016 at 9:09 PM, Tom Lane wrote: > brolga is still not terribly happy with this patch: it's choosing not to > push down the aggregates in one of the queries. While I failed to > duplicate that result locally, investigation suggests that brolga's result > is

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-22 Thread Tom Lane
brolga is still not terribly happy with this patch: it's choosing not to push down the aggregates in one of the queries. While I failed to duplicate that result locally, investigation suggests that brolga's result is perfectly sane; in fact it's not very clear why we're not getting that from

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-22 Thread Ashutosh Bapat
On Fri, Oct 21, 2016 at 9:09 PM, Robert Haas wrote: > On Fri, Oct 21, 2016 at 11:20 AM, Tom Lane wrote: >> Robert Haas writes: >>> On Fri, Oct 21, 2016 at 10:47 AM, Tom Lane wrote: dromedary seems to

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-22 Thread Ashutosh Bapat
On Fri, Oct 21, 2016 at 8:50 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Oct 21, 2016 at 10:47 AM, Tom Lane wrote: >>> dromedary seems to have found one, or at least an unstable test result. > >> OK, looking at that now.

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-21 Thread Robert Haas
On Fri, Oct 21, 2016 at 11:20 AM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Oct 21, 2016 at 10:47 AM, Tom Lane wrote: >>> dromedary seems to have found one, or at least an unstable test result. > >> OK, looking at that now.

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-21 Thread Tom Lane
Robert Haas writes: > On Fri, Oct 21, 2016 at 10:47 AM, Tom Lane wrote: >> dromedary seems to have found one, or at least an unstable test result. > OK, looking at that now. Thanks. Looking at further failures, it looks like 32-bit machines in

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-21 Thread Robert Haas
On Fri, Oct 21, 2016 at 10:47 AM, Tom Lane wrote: > Robert Haas writes: >> I didn't find anything structurally wrong with this patch, so I've >> committed it with many cosmetic changes. Judging by what happened >> with join pushdown, there are probably

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-21 Thread Tom Lane
Robert Haas writes: > I didn't find anything structurally wrong with this patch, so I've > committed it with many cosmetic changes. Judging by what happened > with join pushdown, there are probably some residual bugs, but > hopefully not too many. dromedary seems to have

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-21 Thread Robert Haas
On Thu, Oct 20, 2016 at 5:38 AM, Jeevan Chalke wrote: > Changes look good to me. > Thanks for the detailed review. I didn't find anything structurally wrong with this patch, so I've committed it with many cosmetic changes. Judging by what happened with join

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-20 Thread Jeevan Chalke
On Thu, Oct 20, 2016 at 12:49 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > The patch compiles and make check-world doesn't show any failures. > > >> > > > > > > I have tried it. Attached separate patch for it. > > However I have noticed that istoplevel is always false (at-least

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-18 Thread Robert Haas
On Wed, Oct 12, 2016 at 9:18 AM, Jeevan Chalke wrote: > I did performance testing for aggregate push down and see good performance > with the patch. Are you planning another update to this patch based on Ashutosh's comments? -- Robert Haas EnterpriseDB:

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-13 Thread Prabhat Sahu
On Fri, Sep 30, 2016 at 5:23 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > In the attached patch I have fixed all other review comments you have > posted. All the comments were excellent and helped me a lot to improve > in various areas. > Hi, I have tested and created few

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-12 Thread Jeevan Chalke
On Thu, Sep 8, 2016 at 10:41 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > > > I think we should try to measure performance gain because of aggregate > pushdown. The EXPLAIN > doesn't show actual improvement in the execution times. > I did performance testing for aggregate push

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-12 Thread Ashutosh Bapat
Thanks Jeevan for taking care of the comments. On Fri, Sep 30, 2016 at 5:23 PM, Jeevan Chalke wrote: > Hi Ashutosh, > > On Mon, Sep 26, 2016 at 2:28 PM, Ashutosh Bapat > wrote: >> >> Thanks Jeevan for working on the comments. >>

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-10-02 Thread Michael Paquier
On Fri, Sep 30, 2016 at 8:58 PM, Jeevan Chalke wrote: > On Mon, Sep 26, 2016 at 6:15 PM, Ashutosh Bapat > wrote: >> >> This patch will need some changes to conversion_error_callback(). That >> function reports an error in case

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-30 Thread Jeevan Chalke
On Mon, Sep 26, 2016 at 6:15 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > This patch will need some changes to conversion_error_callback(). That > function reports an error in case there was an error converting the > result obtained from the foreign server into an internal datum

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-26 Thread Ashutosh Bapat
This patch will need some changes to conversion_error_callback(). That function reports an error in case there was an error converting the result obtained from the foreign server into an internal datum e.g. when the string returned by the foreign server is not acceptable by local input function

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-26 Thread Ashutosh Bapat
Thanks Jeevan for working on the comments. > > 3. classifyConditions() assumes list expressions of type RestrictInfo. But > HAVING clause expressions (and JOIN conditions) are plain expressions. Do > you mean we should modify the classifyConditions()? If yes, then I think it > should be done as a

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-16 Thread Robert Haas
On Fri, Sep 16, 2016 at 9:45 AM, Jeevan Chalke wrote: > 6. Per my understanding, I think checking for just aggregate function is > enough as we are interested in whole aggregate result. +1. > Meanwhile I will > check whether we need to find and check shippability

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-16 Thread Jeevan Chalke
On Mon, Sep 12, 2016 at 5:19 PM, Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > On Mon, Sep 12, 2016 at 12:20 PM, Prabhat Sahu < > prabhat.s...@enterprisedb.com> wrote: > >> Hi, >> >> While testing "Aggregate pushdown", i found the below error: >> -- GROUP BY alias showing different

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-12 Thread Jeevan Chalke
On Mon, Sep 12, 2016 at 12:20 PM, Prabhat Sahu < prabhat.s...@enterprisedb.com> wrote: > Hi, > > While testing "Aggregate pushdown", i found the below error: > -- GROUP BY alias showing different behavior after adding patch. > > -- Create table "t1", insert few records. > create table t1(c1 int);

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-12 Thread Jeevan Chalke
On Thu, Sep 8, 2016 at 10:41 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > > > > >> While checking for shippability, we build the target list which is passed >> to >> the foreign server as fdw_scan_tlist. The target list contains >> a. All the GROUP BY expressions >> b. Shippable

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-12 Thread Prabhat Sahu
Hi, While testing "Aggregate pushdown", i found the below error: -- GROUP BY alias showing different behavior after adding patch. -- Create table "t1", insert few records. create table t1(c1 int); insert into t1 values(10), (20); -- Create foreign table: create foreign table f_t1 (c1 int)

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-09-08 Thread Ashutosh Bapat
> While checking for shippability, we build the target list which is passed > to > the foreign server as fdw_scan_tlist. The target list contains > a. All the GROUP BY expressions > b. Shippable entries from the target list of upper relation > c. Var and Aggref nodes from non-shippable entries

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-31 Thread Pavel Stehule
2016-08-31 10:03 GMT+02:00 Amit Langote : > On 2016/08/31 16:42, Pavel Stehule wrote: > > 2016-08-31 9:00 GMT+02:00 Robert Haas : > > > >> On Wed, Aug 31, 2016 at 11:56 AM, Pavel Stehule < > pavel.steh...@gmail.com> > >> wrote: > >>> It is

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-31 Thread Amit Langote
On 2016/08/31 16:42, Pavel Stehule wrote: > 2016-08-31 9:00 GMT+02:00 Robert Haas : > >> On Wed, Aug 31, 2016 at 11:56 AM, Pavel Stehule >> wrote: >>> It is pity - lot of performance issues are related to this missing >> feature. >> >> I don't

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-31 Thread Pavel Stehule
2016-08-31 9:00 GMT+02:00 Robert Haas : > On Wed, Aug 31, 2016 at 11:56 AM, Pavel Stehule > wrote: > > It is pity - lot of performance issues are related to this missing > feature. > > I don't think you are being very clear about what feature you

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-31 Thread Robert Haas
On Wed, Aug 31, 2016 at 11:56 AM, Pavel Stehule wrote: > It is pity - lot of performance issues are related to this missing feature. I don't think you are being very clear about what feature you are talking about. The feature that Jeevan has implemented is pushing

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-31 Thread Pavel Stehule
2016-08-31 8:17 GMT+02:00 Jeevan Chalke : > > > On Tue, Aug 30, 2016 at 6:51 PM, Pavel Stehule > wrote: > >> Hi >> >> 2016-08-30 15:02 GMT+02:00 Jeevan Chalke >> : >> >>> Hi all, >>> >>> Attached is the

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-31 Thread Jeevan Chalke
On Tue, Aug 30, 2016 at 6:51 PM, Pavel Stehule wrote: > Hi > > 2016-08-30 15:02 GMT+02:00 Jeevan Chalke : > >> Hi all, >> >> Attached is the patch which adds support to push down aggregation and >> grouping >> to the foreign server for

Re: [HACKERS] Aggregate Push Down - Performing aggregation on foreign server

2016-08-30 Thread Pavel Stehule
Hi 2016-08-30 15:02 GMT+02:00 Jeevan Chalke : > Hi all, > > Attached is the patch which adds support to push down aggregation and > grouping > to the foreign server for postgres_fdw. Performing aggregation on foreign > server results into fetching fewer rows from