From f003419791f35eeab0150cd097bcf7929622b0fc Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 9 Sep 2014 18:35:17 -0400 Subject: [PATCH] Preserve AND/OR flatness while extracting restriction OR clauses. The code I added in commit f343a880d5555faf1dad0286c5632047c8f599ad was careless about preserving AND/OR flatness: it could create a structure with an OR node directly underneath another one. That breaks an assumption that's fairly important for planning efficiency, not to mention triggering various Asserts (as reported by Benjamin Smith). Add a trifle more logic to handle the case properly. --- src/backend/optimizer/util/orclauses.c | 12 ++++++++++-- src/test/regress/expected/join.out | 27 ++++++++++++++++++++++++++ src/test/regress/sql/join.sql | 4 ++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c index 9e954d0d35..8fecabbcda 100644 --- a/src/backend/optimizer/util/orclauses.c +++ b/src/backend/optimizer/util/orclauses.c @@ -178,6 +178,7 @@ extract_or_clause(RestrictInfo *or_rinfo, RelOptInfo *rel) { Node *orarg = (Node *) lfirst(lc); List *subclauses = NIL; + Node *subclause; /* OR arguments should be ANDs or sub-RestrictInfos */ if (and_clause(orarg)) @@ -226,9 +227,16 @@ extract_or_clause(RestrictInfo *or_rinfo, RelOptInfo *rel) /* * OK, add subclause(s) to the result OR. If we found more than one, - * we need an AND node. + * we need an AND node. But if we found only one, and it is itself an + * OR node, add its subclauses to the result instead; this is needed + * to preserve AND/OR flatness (ie, no OR directly underneath OR). */ - clauselist = lappend(clauselist, make_ands_explicit(subclauses)); + subclause = (Node *) make_ands_explicit(subclauses); + if (or_clause(subclause)) + clauselist = list_concat(clauselist, + list_copy(((BoolExpr *) subclause)->args)); + else + clauselist = lappend(clauselist, subclause); } /* diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index c62a63f674..70f630b3eb 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2827,6 +2827,33 @@ select * from tenk1 a join tenk1 b on Index Cond: (unique2 = 3) (12 rows) +explain (costs off) +select * from tenk1 a join tenk1 b on + (a.unique1 = 1 and b.unique1 = 2) or + ((a.unique2 = 3 or a.unique2 = 7) and b.hundred = 4); + QUERY PLAN +---------------------------------------------------------------------------------------------------------------------- + Nested Loop + Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4))) + -> Bitmap Heap Scan on tenk1 b + Recheck Cond: ((unique1 = 2) OR (hundred = 4)) + -> BitmapOr + -> Bitmap Index Scan on tenk1_unique1 + Index Cond: (unique1 = 2) + -> Bitmap Index Scan on tenk1_hundred + Index Cond: (hundred = 4) + -> Materialize + -> Bitmap Heap Scan on tenk1 a + Recheck Cond: ((unique1 = 1) OR (unique2 = 3) OR (unique2 = 7)) + -> BitmapOr + -> Bitmap Index Scan on tenk1_unique1 + Index Cond: (unique1 = 1) + -> Bitmap Index Scan on tenk1_unique2 + Index Cond: (unique2 = 3) + -> Bitmap Index Scan on tenk1_unique2 + Index Cond: (unique2 = 7) +(19 rows) + -- -- test placement of movable quals in a parameterized join tree -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 1031f26b31..dbb5be64ad 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -774,6 +774,10 @@ select * from tenk1 a join tenk1 b on explain (costs off) select * from tenk1 a join tenk1 b on (a.unique1 = 1 and b.unique1 = 2) or (a.unique2 = 3 and b.ten = 4); +explain (costs off) +select * from tenk1 a join tenk1 b on + (a.unique1 = 1 and b.unique1 = 2) or + ((a.unique2 = 3 or a.unique2 = 7) and b.hundred = 4); -- -- test placement of movable quals in a parameterized join tree