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 da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
