Re: [HACKERS] from_collapse_limit considerations

2014-10-22 Thread Antonin Houska
[ I think I responded earlier but don't see my mail in the archive... ]

Tom Lane  wrote:
> Antonin Houska  writes:
> > I noticed that - unlike join_collapse_limit - the from_collapse_limit does 
> > not
> > enforce maximum length of the top-level list. Shouldn't it do?
>
> How would it do that?  You want it to fail outright if there are more than
> N tables?  That seems unhelpful.

Sure, truncation of the FROM list would be crazy, and that's not what I tried 
to propose.

--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] from_collapse_limit considerations

2014-09-22 Thread Tom Lane
Antonin Houska  writes:
> I noticed that - unlike join_collapse_limit - the from_collapse_limit does not
> enforce maximum length of the top-level list. Shouldn't it do?

How would it do that?  You want it to fail outright if there are more than
N tables?  That seems unhelpful.

> Also, the order of FROM-list items seems to affect the way RTEs are grouped
> into (sub)lists.

Yeah.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] from_collapse_limit considerations

2014-09-22 Thread Antonin Houska
While doing experiments with rather long FROM-lists, I looked closely at the
logic related to from_collapse_limit.

I noticed that - unlike join_collapse_limit - the from_collapse_limit does not
enforce maximum length of the top-level list. Shouldn't it do? Too long
FROM-list can obviously lead to excessive planning time.

Also, the order of FROM-list items seems to affect the way RTEs are grouped
into (sub)lists. In this example, the join of tab_0, tab_1, tab_2, tab_3 gets
expanded into 4 separate RTE refs:

SET from_collapse_limit TO 5;

SELECT  *
FROM
(
(
tab_0
JOIN
tab_1
ON tab_0.id = tab_1.id
)
JOIN
(
tab_2
JOIN
tab_3
ON tab_2.id = tab_3.id
)
ON  tab_1.id = tab_2.id
),
tab_4
JOIN
tab_5
ON tab_4.id = tab_5.id
WHERE   tab_3.id = tab_4.id;

However, in the next example (the JOIN of tab_4 and tab_5 moved to the
beginning of the FROM list), the "bigger join" (tab_0 through tab_3) "comes
too late", so it's inserted as a sub-list.

SET from_collapse_limit TO 5;

SELECT  *
FROM
tab_4
JOIN
tab_5
ON tab_4.id = tab_5.id,
(
(
tab_0
JOIN
tab_1
ON tab_0.id = tab_1.id
)
JOIN
(
tab_2
JOIN
tab_3
ON tab_2.id = tab_3.id
)
ON  tab_1.id = tab_2.id
)
WHERE   tab_3.id = tab_4.id;


Is anything wrong about the idea not to estimate the total length of the FROM
list in deconstruct_recurse and to do additional collapsing later instead? The
patch attached here tries to do so.

I wonder if change of the logic behind from_collapse_limit should be
considered acceptable for users or not: although it improves control over
planning of queries having long FROM-list, it can make some plans of existing
applications worse, unless from_collapse_limit is increased accordingly.

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index f88e493..fffc3aa 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -51,6 +51,7 @@ static List *deconstruct_recurse(PlannerInfo *root, Node *jtnode,
 	bool below_outer_join,
 	Relids *qualscope, Relids *inner_join_rels,
 	List **postponed_qual_list);
+static List *collapse_fromlist(List *fromlist);
 static SpecialJoinInfo *make_outerjoininfo(PlannerInfo *root,
    Relids left_rels, Relids right_rels,
    Relids inner_join_rels,
@@ -659,6 +660,9 @@ deconstruct_jointree(PlannerInfo *root)
 	/* Shouldn't be any leftover quals */
 	Assert(postponed_qual_list == NIL);
 
+	/* Create sub-list(s) if the FROM-list appears to be too long.  */
+	result = collapse_fromlist(result);
+
 	return result;
 }
 
@@ -710,7 +714,6 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 	{
 		FromExpr   *f = (FromExpr *) jtnode;
 		List	   *child_postponed_quals = NIL;
-		int			remaining;
 		ListCell   *l;
 
 		/*
@@ -722,12 +725,10 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 		*qualscope = NULL;
 		*inner_join_rels = NULL;
 		joinlist = NIL;
-		remaining = list_length(f->fromlist);
 		foreach(l, f->fromlist)
 		{
 			Relids		sub_qualscope;
 			List	   *sub_joinlist;
-			int			sub_members;
 
 			sub_joinlist = deconstruct_recurse(root, lfirst(l),
 			   below_outer_join,
@@ -735,13 +736,14 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 			   inner_join_rels,
 			   &child_postponed_quals);
 			*qualscope = bms_add_members(*qualscope, sub_qualscope);
-			sub_members = list_length(sub_joinlist);
-			remaining--;
-			if (sub_members <= 1 ||
-list_length(joinlist) + sub_members + remaining <= from_collapse_limit)
-joinlist = list_concat(joinlist, sub_joinlist);
-			else
-joinlist = lappend(joinlist, sub_joinlist);
+
+			/*
+			 * Sub-lists are not created at this stage, as we can't predict how
+			 * many RTEs the possibly following join nodes may contain. Instead,
+			 * length of the top-level list is checked later when
+			 * deconstruct_recurse has finished.
+			 */
+			joinlist = list_concat(joinlist, sub_joinlist);
 		}
 
 		/*
@@ -1013,6 +1015,65 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 	return joinlist;
 }
 
+
+/*
+ * Ensure that neither the top-level FROM-list nor any its sublist exceeds
+ * from_collapse_limit.
+ */
+static List *
+collapse_fromlist(List *fromlist)
+{
+	List *result = f