Re: Push down more full joins in postgres_fdw

On 2016/11/24 17:39, Ashutosh Bapat wrote:
> On Thu, Nov 24, 2016 at 1:27 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> On 2016/11/24 16:46, Ashutosh Bapat wrote:
>>> table will be misleading as subquery can represent a join and
>>> corresponding alias would represent the join. Relation is better term.

>> But the documentation uses the word "table" for a join relation. See
>> 7.2.1.2. Table and Column Aliases:
>> https://www.postgresql.org/docs/9.6/static/queries-table-expressions.html#QUERIES-TABLE-ALIASES

> The definition there says "A temporary name can be given to tables and
> complex table references to be used for references to the derived
> table in the rest of the query. This is called a table alias.", please
> note the usage "complex table references". Within the code, we do not
> refer to a relation as table. So, the comment in this code should not
> refer to a relation as table either.

OK, will keep using the term "relation".

I wrote:
>>>> I don't think so; in the current version, we essentially deparse from and
>>>> search into the same object, ie, foreignrel->reltarget->exprs, since the
>>>> tlist created by build_tlist_to_deparse is build from the
>>>> foreignrel->reltarget->exprs. Also, since it is just created for
>>>> deparsing
>>>> the relation as a subquery in deparseRangeTblRef and isn't stored into
>>>> fpinfo or anywhere alse, we would need no concern about creating such
>>>> avenues. IMO I think adding comments would be enough for this.

>>> build_tlist_to_depase() calls pull_var_nodes() before creating the
>>> tlist, whereas the code that searches does not do that. Code-wise
>>> those are not the same things.

>> 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.

> 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.

It *can* be guaranteed. See another patch for supporting evaluating
PHVs remotely.

Best regards,
Etsuro Fujita