From dcc23347365d1958898445ffdcf34c09689b8d55 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 27 Jun 2008 20:54:37 +0000
Subject: [PATCH] Consider a clause to be outerjoin_delayed if it references
 the nullable side of any lower outer join, even if it also references the
 non-nullable side and so could not get pushed below the outer join anyway. 
 We need this in case the clause is an OR clause: if it doesn't get marked
 outerjoin_delayed, create_or_index_quals() could pull an indexable
 restriction for the nullable side out of it, leading to wrong results as
 demonstrated by today's bug report from toruvinn.  (See added regression test
 case for an example.)

In principle this has been wrong for quite a while.  In practice I don't
think any branch before 8.3 can really show the failure, because
create_or_index_quals() will only pull out indexable conditions, and before
8.3 those were always strict.  So though we might have improperly generated
null-extended rows in the outer join, they'd get discarded from the result
anyway.  The gating factor that makes the failure visible is that 8.3
considers "col IS NULL" to be indexable.  Hence I'm not going to risk
back-patching further than 8.3.
---
 src/backend/optimizer/plan/initsplan.c |  7 ++++---
 src/test/regress/expected/join.out     | 12 ++++++++++++
 src/test/regress/expected/join_1.out   | 12 ++++++++++++
 src/test/regress/sql/join.sql          | 10 ++++++++++
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index dd08b19cb83..6cfd2ab4c38 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.139 2008/04/01 00:48:33 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.140 2008/06/27 20:54:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1092,14 +1092,15 @@ check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
 				(ojinfo->is_full_join &&
 				 bms_overlap(relids, ojinfo->min_lefthand)))
 			{
-				/* yes; have we included all its rels in relids? */
+				/* yes, so set the result flag */
+				outerjoin_delayed = true;
+				/* have we included all its rels in relids? */
 				if (!bms_is_subset(ojinfo->min_lefthand, relids) ||
 					!bms_is_subset(ojinfo->min_righthand, relids))
 				{
 					/* no, so add them in */
 					relids = bms_add_members(relids, ojinfo->min_lefthand);
 					relids = bms_add_members(relids, ojinfo->min_righthand);
-					outerjoin_delayed = true;
 					/* we'll need another iteration */
 					found_some = true;
 				}
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index bd33cb5c126..d7b66ee1729 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2321,3 +2321,15 @@ where f2 = 53;
  53 |    |    | 
 (1 row)
 
+--
+-- regression test for improper extraction of OR indexqual conditions
+-- (as seen in early 8.3.x releases)
+--
+select a.unique2, a.ten, b.tenthous, b.unique2, b.hundred
+from tenk1 a left join tenk1 b on a.unique2 = b.tenthous
+where a.unique1 = 42 and
+      ((b.unique2 is null and a.ten = 2) or b.hundred = 3);
+ unique2 | ten | tenthous | unique2 | hundred 
+---------+-----+----------+---------+---------
+(0 rows)
+
diff --git a/src/test/regress/expected/join_1.out b/src/test/regress/expected/join_1.out
index 4690b711402..2721c3f1cf0 100644
--- a/src/test/regress/expected/join_1.out
+++ b/src/test/regress/expected/join_1.out
@@ -2321,3 +2321,15 @@ where f2 = 53;
  53 |    |    | 
 (1 row)
 
+--
+-- regression test for improper extraction of OR indexqual conditions
+-- (as seen in early 8.3.x releases)
+--
+select a.unique2, a.ten, b.tenthous, b.unique2, b.hundred
+from tenk1 a left join tenk1 b on a.unique2 = b.tenthous
+where a.unique1 = 42 and
+      ((b.unique2 is null and a.ten = 2) or b.hundred = 3);
+ unique2 | ten | tenthous | unique2 | hundred 
+---------+-----+----------+---------+---------
+(0 rows)
+
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 96e15b526c2..0e5638c7c1b 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -485,3 +485,13 @@ select * from
   zt2 left join zt3 on (f2 = f3)
       left join zv1 on (f3 = f1)
 where f2 = 53;
+
+--
+-- regression test for improper extraction of OR indexqual conditions
+-- (as seen in early 8.3.x releases)
+--
+
+select a.unique2, a.ten, b.tenthous, b.unique2, b.hundred
+from tenk1 a left join tenk1 b on a.unique2 = b.tenthous
+where a.unique1 = 42 and
+      ((b.unique2 is null and a.ten = 2) or b.hundred = 3);
-- 
GitLab