On 2016/12/05 20:20, Ashutosh Bapat wrote:
On Mon, Nov 28, 2016 at 3:52 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
On 2016/11/24 21:45, Etsuro Fujita wrote:
* I removed make_outerrel_subquery/make_innerrel_subquery from fpinfo
and added a new member "is_subquery_rel" to fpinfo, as a flag on whether
to deparse the relation as a subquery.
Replacing make_outerrel_subquery/make_innerrel_subquery with is_subquery_rel
seems to be a bad idea. Whether a relation needs to be deparsed as a subquery
or not depends upon the relation which joins it with another relation. Take for
example a case when a join ABC can pull up the conditions on AB, but ABD can
not. In this case we will mark that AB in ABD needs to be deparsed as a
subquery but will not mark so in ABC. So, if we choose to join ABCD as (ABC)D
we don't need AB to be deparsed as a subquery. If we choose to join ABCD as
(ABD)C, we will need AB to deparsed as a subquery.
This is not true; since (1) currently, a relation needs to be deparsed
as a subquery only when the relation is full-joined with any other
relation, and (2) the join ordering for full joins is preserved, we
wouldn't have such an example. (You might think we would have that when
extending the patch to handle PHVs, but the answer would be no, because
the patch for PHVs would determine whether a relation emitting PHVs
needs to be deparsed as a subquery, depending on the relation itself.
See the patch for PHVs.) I like is_subquery_rel more than
make_outerrel_subquery/make_innerrel_subquery, because it reduces the
number of members in fpinfo. But the choice would be arbitrary, so I
wouldn't object to going back to
make_outerrel_subquery/make_innerrel_subquery.
Some more comments
1. The output of the following query
+SELECT 1 FROM (SELECT c1 FROM ft4 WHERE c1 between 50 and 60) t1 FULL
JOIN (SELECT c1 FROM ft5 WHERE c1 between 50 and 60) t2 ON (TRUE);
produces 21 rows all containing a "1". That's not quite good use of expected
output space, given that all that the test tests is the empty
targetlists are deparsed
correctly. You might want to either just test EXPLAIN output or make "between 50
and 60" condition more stringent to reduce the number of rows output.
Will do.
2. Got lost here. I guess, all you need to say is "test deparsing FOR UPDATE
clause with subqueries.". Anyway, the sentence is very hard to read and needs
simplification.
+-- d. test deparsing the remote queries for simple foreign table scans in
+-- an EPQ subplan of a foreign join in which the foreign tables are full
+-- joined and in the remote join query the foreign tables are deparsed as
+-- subqueries
Will do.
3. Why is foreignrel variable changed to rel?
-extern void deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root,
- RelOptInfo *foreignrel, List *tlist,
- List *remote_conds, List *pathkeys,
- List **retrieved_attrs, List **params_list);
+extern void deparseSelectStmtForRel(StringInfo buf, PlannerInfo
*root, RelOptInfo *rel,
+ List *tlist, List *remote_conds, List *pathkeys,
+ bool is_subquery, List **retrieved_attrs,
+ List **params_list);
I changed the name that way to match with the function definition in
deparse.c.
Thanks for the comments!
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
