Re: Push down more full joins in postgres_fdw

On 2016/11/24 18:20, Ashutosh Bapat wrote:

I wrote:
>>>> You missed the point; the foreignrel->reltarget->exprs doesn't contain
>>>> any
>>>> PHVs, so the tlist created by build_tlist_to_depase will be guaranteed to
>>>> be
>>>> one-to-one with the foreignrel->reltarget->exprs.

You wrote:
>>> It's guaranteed now, but can not be forever. This means anybody who
>>> supports PHV tomorrow, will have to "remember" to update this code as
>>> well. If s/he misses it, a bug will be introduced. That's the avenue
>>> for bug, I am talking about.

I wrote:
>> It *can* be guaranteed. See another patch for supporting evaluating PHVs
>> remotely.

> We can't base assumptions in this patch on something in the other
> patch, esp. when that's not even reviewed once. PHV is just one case,
> subqueries involving aggregates is other and there are others.

Let's discuss this together with the patch for PHVs. That was one of
the reasons why I had merged the two patches into one. I'd like to
leave enhancements such as subqueries involving aggregates for future
work, though.

> But
> that's not really the point. The point is
> add_to_flat_tlist(pull_var_clause(rel->reltarget->exprs) can not be
> proved to be same as rel->reltarget->exprs in general.

Really? As mentioned in comments for RelOptInfo in relation.h shown
below, the rel->reltarget->exprs for a base/join relation only contains
Vars and PHVs, so the two things would be proved to be one-to-one.
(Note: we don't need to care about the appendrel-child-relation case
because appendrel child relations wouldn't appear in the main join tree.)

* reltarget - Default Path output tlist for this rel; normally
contains
* Var and PlaceHolderVar nodes for the values we need to
* output from this relation.
* List is in no particular order, but all rels of an
* appendrel set must use corresponding orders.
* NOTE: in an appendrel child relation, may contain
* arbitrary expressions pulled up from a subquery!

Best regards,
Etsuro Fujita