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
usecorresponding orders. *                  NOTE: in an appendrel child relation, may contain *
arbitraryexpressions pulled up from a subquery!
 

Best regards,
Etsuro Fujita