From: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Foreign Join pushdowns not working properly for outer joins |
Date: | 2017-03-14 13:00:36 |
Message-ID: | CAFjFpRco0UwAqakPK8Lqw5KSo7A167-JZQ_wHR_i9n1jF4w_0A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Added this to 2017/07 commitfest.
On Fri, Mar 10, 2017 at 10:03 AM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>>>
>>> The new name merge_fdw_options() is shorter than the one I chose, but
>>> we are not exactly merging options for an upper relation since there
>>> isn't the other fpinfo to merge from. But I think we can live with
>>> merge_fdw_options().
>>
>> Perhaps "combine" is a better word? I didn't really see a problem with
>> that. After I posted this I wished I'd made the function even more
>> generic to accept either side as NULL and make up the new fpinfo from
>> the non-NULL one, or Merge if both were non-NULL. I liked that way
>> much better than giving the function too much knowledge of what its
>> purpose actually is. It's more likely to get used for something else
>> in the future, which means there's less chance that someone else will
>> make the same mistake.
>
> It's more like copy for an upper rel and merge for a join rel. Your
> point about making it side-agnostic is good but I doubt if we want to
> spend more effort there. If somebody writes a code with the input plan
> stuffed in the innerrel instead of the outerrel, s/he will notice it
> immediately when testing as assertion would fail or there will be a
> straight segfault. We will decide what to do then.
>
>>
>>> Once I fixed that, the testcases started showing an assertion failure,
>>> since fpinfo of a base relation can not have an outerrel. Fixed the
>>> assertion as well. If we are passing fpinfo's of joining relations,
>>> there's no need to have outerrel and innerrel in fpinfo of join.
>>
>> Looks like you forgot to update the comment on the Assert()
>
> Yes and I also forgot to update the function prologue to refer to the
> fpinfo_o/i instead of inner and outer relations. Attached patch
> corrects it.
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Bapat | 2017-03-14 13:05:24 | Re: dropping partitioned tables without CASCADE |
Previous Message | Ashutosh Bapat | 2017-03-14 12:55:32 | Re: IF NOT EXISTS option for CREATE SERVER and CREATE USER MAPPING statements |