On Tue, Apr 11, 2017 at 10:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
>> On Tue, Apr 11, 2017 at 8:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> OK. I was trying to be noninvasive, but this does look more sensible.
>>> I think this might read better if we inverted the test (so that
>>> the outer-join-joinclause-must-be-pushable case would come first).
>
>> Ok. In fact, thinking more about it, we should probably test
>> JOIN_INNER explicitly instead of !IS_OUTER_JOIN() since SEMI_JOINS are
>> not considered as outer joins and I am not sure how would we be
>> treating the quals for those.
>
> No, that's correct as-is --- or at least, if it's not correct, there
> are a bunch of other places that are also not correct.
Hmm, when we support SEMI and ANTI join push-down, we will have a
bunch of other places to change. This is one of them.
>
> Thinking about this further, though, it seems like a more straightforward
> solution to the original problem is to not store quals in a Plan's
> fdw_private list in the first place. Putting them there is at best
> useless overhead that the finished plan will have to drag around;
> and it's not terribly hard to imagine future changes that would make
> having never-processed-by-setrefs.c qual trees in a plan be broken in
> other ways. Can't we use a field in the rel's PgFdwRelationInfo to
> carry the remote_exprs forward from postgresGetForeignPlan to
> postgresPlanDirectModify?
>
I was thinking of using fdw_recheck_quals by renaming it. We don't
push DML with joins down to the foreign server. So, it's ok to set
fdw_recheck_quals (or whatever name we choose) to be NIL in join and
upper relation case as we do today, without changing anything else.
When we support DMLs with join pushdown, we will have to use subquery
for the scan and thus will not require fdw_recheck_quals to be set.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company