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.

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index dd08b19cb8371bbfedc80f36b3dc76fbb36b704d..6cfd2ab4c3877338cd25241d076454d53d9426c1 100644 (file)
--- 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 bd33cb5c12646b7df4d4e54a9f5b045c4f0a8879..d7b66ee1729c211ddfd383021d9d3872f5c16811 100644 (file)
--- 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 4690b711402199bb7c5bf395010a560e10854ebe..2721c3f1cf0a6b5a638e696b3fc08684140f7461 100644 (file)
--- 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 96e15b526c2370a4ac46cb5adc090db59cdc6489..0e5638c7c1b4db0f1649a72d56c899c9a96a9e5e 100644 (file)
--- 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);