Re: Push down more full joins in postgres_fdw

On 2016/12/07 5:27, Robert Haas wrote:
> On Mon, Dec 5, 2016 at 6:20 AM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>> 4. I am still not happy with this change
>> +        /*
>> +         * Since (1) the expressions in foreignrel's reltarget doesn't contain
>> +         * any PHVs and (2) foreignrel's local_conds is empty, the tlist
>> +         * created by build_tlist_to_deparse must be one-to-one with the
>> +         * expressions.
>> +         */
>> +        Assert(list_length(tlist) ==
>> list_length(foreignrel->reltarget->exprs));
>> the assertion only checks that the number of elements in both the lists are
>> same but does not check whether those lists are same i.e. they contain the same
>> elements in the same order. This equality is crucial to deparsing logic. If
>> somehow build_tlist_to_deparse() breaks that assumption in future, we have no
>> way to detect it, unless a regression test fails.

> If there's an easy way to do a more exact comparison, great.

Ashutosh proposed this to do the comparison:

On 2016/11/22 18:28, Ashutosh Bapat wrote:
> I guess, the reason why you are doing it this way, is SELECT clause for the
> outermost query gets deparsed before FROM clause. For later we call
> deparseRangeTblRef(), which builds the tlist. So, while deparsing SELECT
> clause, we do not have tlist to build from. In that case, I guess, we have to
> build the tlist in get_subselect_alias_id() if it's not available and stick it
> in fpinfo. Subsequent calls to get_subselect_alias_id() should find tlist
> there. Then in deparseRangeTblRef() assert that there's a tlist in fpinfo
> and use it to build the SELECT clause of subquery. That way, we don't build
> tlist unless it's needed and also use the same tlist for all searches. Please
> use tlist_member() to search into the tlist.

This would probably work, but seems to me a bit complicated.  Instead, 
I'd like to propose that we build the tlist for each relation being 
deparsed as a subquery in a given join tree, right before deparsing the 
SELECT clause in deparseSelectStmtForRel, if is_subquery is false and 
lower_subquery_rels isn't NULL, and store the tlist into the relation's 
fpinfo.  That would allow us to build the tlist only when we need it, 
and to use tlist_member for the exact comparison.  I think it would be 
much easier to implement that.

Best regards,
Etsuro Fujita