Re: Push down more full joins in postgres_fdw

On 2016/11/24 16:46, Ashutosh Bapat wrote:
>>> Sorry. I think the current version is better than previous one. The
>>> term "subselect alias" is confusing in the previous version. In the
>>> current version, "Get the relation and column alias for a given Var
>>> node," we need to add word "identifiers" like "Get the relation and
>>> column identifiers for a given Var node".

I wrote:
>> OK, but one thing I'm concerned about is the term "relation alias" seems a
>> bit confusing because we already used the term for the alias of the form
>> "rN". To avoid that, how about saying "table alias", not "relation alias"?
>> (in which case, the comment would be something like "Get the table and
>> column identifiers for a given Var node".)

> 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

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

Best regards,
Etsuro Fujita