Re: Push down more full joins in postgres_fdw

On 2016/12/07 20:23, Etsuro Fujita wrote:
> On 2016/12/07 15:39, Ashutosh Bapat wrote:

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

> I wrote:
>>> 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.

>> IIRC, this is inline with my original proposal of creating tlists
>> before deparsing anything. Along-with that I also proposed to bubble
>> this array of tlist up the join hierarchy to avoid recursion [1] point
>> #15, further elaborated in [2]
>>
>> [1]
>> https://www.postgresql.org/message-id/ad449b25-66ee-4c06-568c-0eb2e1bef9f9%40lab.ntt.co.jp
>>
>> [2]
>> https://www.postgresql.org/message-id/CAFjFpRcn7%3DDNOXm-PJ_jVDqAmghKVf6tApQC%2B_RgMZ8tOPExcA%40mail.gmail.com

> My proposal here would be a bit different from what you proposed; right
> before deparseSelectSql in deparseSelectStmtForRel, build the tlists for
> relations present in the given jointree that will be deparsed as
> subqueries, by (1) traversing the given jointree and (2) applying
> build_tlist_to_deparse to each relation to be deparsed as a subquery and
> saving the result in that relation's fpinfo. I think that would be
> implemented easily, and the overhead would be small.

I implemented that to address your concern:
* deparseRangeTblRef uses the tlist created as above, to build the
SELECT clause of the subquery. (You said "Then in deparseRangeTblRef()
assert that there's a tlist in fpinfo", but the tlist may be empty, so I
didn't add any assertion to that function.)
* get_relation_column_alias_ids uses tlist_member with the tlist.

I also addressed the comments #1, #2 and #3 in [1]. For #1, I added
"LIMIT 10" to the query. Attached is the new version of the patch.

Other changes:
* As discussed before, renamed grouped_tlist in fpinfo to "tlist" and
used it to store the tlist created as above.
* Also, renamed SS_REL_ALIAS_PREFIX to SUBQUERY_REL_ALIAS_PREFIX
(Likewise for SS_COL_ALIAS_PREFIX).
* Relocated some functions.
* Revised comments a little bit.

Best regards,
Etsuro Fujita

[1]
https://www.postgresql.org/message-id/CAFjFpRfU4-gxqZ8ahoKM1ZtDJEThe3K8Lb_6beRKa8mmP%3Dv%3DfA%40mail.gmail.com