Search Postgresql Archives

Re: Crash in 9.4 Beta when partially collapsing left outer joins

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 9, 2014 at 2:43 PM, Michael Paquier
<michael.paquier@xxxxxxxxx> wrote:
> Some bisecting is showing as well that the commit at the origin of the
> regression is f343a88.
The failure is caused by an assertion not happy since this commit:
    frame #4: 0x0000000101d20670
postgres`generate_bitmap_or_paths(root=0x00007fd61d004d48,
rel=0x00007fd61c033a58, clauses=0x00007fd61d010200,
other_clauses=0x0000000000000000) + 480 at indxpath.c:1213
    frame #5: 0x0000000101d1fc37
postgres`create_index_paths(root=0x00007fd61d004d48,
rel=0x00007fd61c033a58) + 1255 at indxpath.c:314
    frame #6: 0x0000000101d1146b
postgres`set_plain_rel_pathlist(root=0x00007fd61d004d48,
rel=0x00007fd61c033a58, rte=0x00007fd61c033c88) + 75 at allpaths.c:397

While reading the code of this commit, I noticed that
extract_or_clause has added some logic for nested OR clauses: it
extracts their content and adds them directly to the list of
subclauses that are then used by generate_bitmap_or_paths, triggering
the assertion failure reported by the trace above.
The logic for nested OR is correct by reading it, hence why not simply
removing the assertion failing? The attached patch 1 does so.

Another approach would consist in removing the nested OR part and keep
the old assertion logic, like in the patch 2 attached, but this seems
like a no-go as f343a88 has actually improved nested OR tracking.
Thoughts?
Note: I added as well a regression tests in patch 1 as this is IMO the
correct approach, if that's considered as correct of course :)
-- 
Michael
From 78612c5c80d57b297ef93e992874b571e9bf0f75 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@xxxxxxxxxx>
Date: Tue, 9 Sep 2014 16:43:41 +0900
Subject: [PATCH] Fix Assertion failure caused by nested OR at extraction

Commit f343a88 has added some logic in extract_or_clause to extract OR
subclauses, while the index path code in planner is wrongly assuming
that subclauses cannot be OR clauses themselves.

Per report from Benjamin Smith
---
 src/backend/optimizer/path/indxpath.c |  1 -
 src/test/regress/expected/join.out    | 21 +++++++++++++++++++++
 src/test/regress/sql/join.sql         |  3 +++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 42dcb11..88bf946 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1210,7 +1210,6 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
 				List	   *orargs;
 
 				Assert(IsA(orarg, RestrictInfo));
-				Assert(!restriction_is_or_clause((RestrictInfo *) orarg));
 				orargs = list_make1(orarg);
 
 				indlist = build_paths_for_OR(root, rel,
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 1cb1c51..5079ba0 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2827,6 +2827,27 @@ 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 or ((a.unique1 = 2 or a.unique1 = 3) and b.ten = 4);
+                                         QUERY PLAN                                         
+--------------------------------------------------------------------------------------------
+ Nested Loop
+   Join Filter: ((a.unique1 = 1) OR (((a.unique1 = 2) OR (a.unique1 = 3)) AND (b.ten = 4)))
+   ->  Seq Scan on tenk1 b
+   ->  Materialize
+         ->  Bitmap Heap Scan on tenk1 a
+               Recheck Cond: ((unique1 = 1) OR ((unique1 = 2) OR (unique1 = 3)))
+               ->  BitmapOr
+                     ->  Bitmap Index Scan on tenk1_unique1
+                           Index Cond: (unique1 = 1)
+                     ->  BitmapOr
+                           ->  Bitmap Index Scan on tenk1_unique1
+                                 Index Cond: (unique1 = 2)
+                           ->  Bitmap Index Scan on tenk1_unique1
+                                 Index Cond: (unique1 = 3)
+(14 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 fa3e068..c170b09 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -774,6 +774,9 @@ 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 or ((a.unique1 = 2 or a.unique1 = 3) and b.ten = 4);
 
 --
 -- test placement of movable quals in a parameterized join tree
-- 
2.1.0

diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c
index 9e954d0..387a308 100644
--- a/src/backend/optimizer/util/orclauses.c
+++ b/src/backend/optimizer/util/orclauses.c
@@ -190,21 +190,8 @@ extract_or_clause(RestrictInfo *or_rinfo, RelOptInfo *rel)
 				RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc2);
 
 				Assert(IsA(rinfo, RestrictInfo));
-				if (restriction_is_or_clause(rinfo))
-				{
-					/*
-					 * Recurse to deal with nested OR.  Note we *must* recurse
-					 * here, this isn't just overly-tense optimization: we
-					 * have to descend far enough to find and strip all
-					 * RestrictInfos in the expression.
-					 */
-					Expr	   *suborclause;
-
-					suborclause = extract_or_clause(rinfo, rel);
-					if (suborclause)
-						subclauses = lappend(subclauses, suborclause);
-				}
-				else if (is_safe_restriction_clause_for(rinfo, rel))
+				if (!restriction_is_or_clause(rinfo) &&
+					is_safe_restriction_clause_for(rinfo, rel))
 					subclauses = lappend(subclauses, rinfo->clause);
 			}
 		}
-- 
Sent via pgsql-general mailing list (pgsql-general@xxxxxxxxxxxxxx)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Postgresql Jobs]     [Postgresql Admin]     [Postgresql Performance]     [Linux Clusters]     [PHP Home]     [PHP on Windows]     [Kernel Newbies]     [PHP Classes]     [PHP Books]     [PHP Databases]     [Postgresql & PHP]     [Yosemite]
  Powered by Linux