Re: Push down more full joins in postgres_fdw

On 1/30/17 6:30 AM, Etsuro Fujita wrote:
> On 2017/01/27 21:25, Etsuro Fujita wrote:
>> On 2017/01/27 20:04, Ashutosh Bapat wrote:
>>> I think we should pick up your patch on
>>> 27th December, update the comment per your mail on 5th Jan. I will
>>> review it once and list down the things left to committer's judgement.
>
>> Sorry, I started thinking we went in the wrong direction. I added to
>> deparseSelectStmtForRel build_subquery_tlists, which creates a tlist for
>> each subquery present in a given join tree prior to deparsing a whole
>> remote query. But that's nothing but an overhead; we need to create a
>> tlist for the top-level query because we use it as fdw_scan_tlist, but
>> not for subqueries, and we need to create retrieved_attrs for the
>> top-level query while deparsing the targetlist in
>> deparseExplicitTargetList, but not for subqueries. So, we should need
>> some work to avoid such a useless overhead.
>
> I think we can avoid the useless overhead by adding a new function,
> deparseSubqueryTargetList, that deparses expressions in the given
> relation's reltarget, not the tlist, as a SELECT clause of the subquery
> representing the given relation. That would also allow us to make the
> 1-to-1 relationship between the subquery output columns and their
> aliases more explicit, which was your original comment. Please find
> attached the new version. (The patch doesn't need the patch to avoid
> duplicate construction of the tlist, discussed upthread.)
>
> Other changes:
> * I went back to make_outerrel_subquery and make_innerrel_subquery,
> which are flags to indicate whether to deparse the input relations as
> subqueries. is_subquery_rel would work well for handling the cases of
> full joins with restrictions on the input relations, but I noticed that
> that wouldn't work well when extending to handle the cases where we
> deparse the input relations as subqueries to evaluate PHVs remotely.
> * Since appendSubqueryAlias in the previous patch is pretty small, I
> included the code into deparseRangeTblRef.

This patch does not apply cleanly at cccbdde:

$ patch -p1 < ../other/postgres-fdw-subquery-support-v15.patch
patching file contrib/postgres_fdw/deparse.c
Hunk #11 succeeded at 1371 (offset -3 lines).
Hunk #12 succeeded at 1419 (offset -3 lines).
Hunk #13 succeeded at 1486 (offset -3 lines).
Hunk #14 succeeded at 2186 (offset -3 lines).
Hunk #15 succeeded at 3082 (offset -3 lines).
patching file contrib/postgres_fdw/expected/postgres_fdw.out
patching file contrib/postgres_fdw/postgres_fdw.c
Hunk #1 succeeded at 669 (offset 1 line).
Hunk #2 succeeded at 1245 (offset -1 lines).
Hunk #3 succeeded at 2557 (offset -1 lines).
Hunk #4 succeeded at 4157 (offset 3 lines).
Hunk #5 succeeded at 4183 (offset 3 lines).
Hunk #6 succeeded at 4212 (offset 3 lines).
Hunk #7 succeeded at 4315 (offset 3 lines).
patching file contrib/postgres_fdw/postgres_fdw.h
patching file contrib/postgres_fdw/sql/postgres_fdw.sql

Since these are just offsets I'll leave the patch as "Needs review" but
an updated patch would be appreciated.

Thanks,
--
-David
david(at)pgmasters(dot)net