Fix bug introduced into mergejoin logic by performance improvement patch of

2005-05-13.  When we find that a new inner tuple can't possibly match any
outer tuple (because it contains a NULL), we can't immediately skip the
tuple when we are in NEXTINNER state.  Doing so can lead to emitting
multiple copies of the tuple in FillInner mode, because we may rescan the
tuple after returning to a previous marked tuple.  Instead, proceed to
NEXTOUTER state the same as we used to do.  After we've found that there's
no need to return to the marked position, we can go to SKIPINNER_ADVANCE
state instead of SKIP_TEST when the inner tuple is unmatchable; this
preserves the performance improvement.  Per bug report from Bruce.
I also made a couple of cosmetic code rearrangements and added a regression
test for the problem.

diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c
index c111b336611364410cf68f85b2dad298cc3d5e4a..0c59e11be60b3c035e1518c6a24e86dc202b8760 100644 (file)
--- a/src/backend/executor/nodeMergejoin.c
+++ b/src/backend/executor/nodeMergejoin.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/nodeMergejoin.c,v 1.78 2006/03/05 15:58:26 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/nodeMergejoin.c,v 1.79 2006/03/17 19:38:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -948,9 +948,6 @@ ExecMergeJoin(MergeJoinState *node)
                 * now we get the next inner tuple, if any.  If there's none,
                 * advance to next outer tuple (which may be able to join to
                 * previously marked tuples).
-                *
-                * If we find one but it cannot join to anything, stay in
-                * NEXTINNER state to fetch the next one.
                 */
                innerTupleSlot = ExecProcNode(innerPlan);
                node->mj_InnerTupleSlot = innerTupleSlot;
@@ -963,8 +960,17 @@ ExecMergeJoin(MergeJoinState *node)
                    break;
                }
 
+               /*
+                * Load up the new inner tuple's comparison values.  If we
+                * see that it contains a NULL and hence can't match any
+                * outer tuple, we can skip the comparison and assume the
+                * new tuple is greater than current outer.
+                */
                if (!MJEvalInnerValues(node, innerTupleSlot))
-                   break;      /* stay in NEXTINNER state */
+               {
+                   node->mj_JoinState = EXEC_MJ_NEXTOUTER;
+                   break;
+               }
 
                /*
                 * Test the new inner tuple to see if it matches outer.
@@ -1054,15 +1060,15 @@ ExecMergeJoin(MergeJoinState *node)
                }
 
                /* Compute join values and check for unmatchability */
-               if (!MJEvalOuterValues(node))
+               if (MJEvalOuterValues(node))
                {
-                   /* Stay in same state to fetch next outer tuple */
-                   node->mj_JoinState = EXEC_MJ_NEXTOUTER;
+                   /* Go test the new tuple against the marked tuple */
+                   node->mj_JoinState = EXEC_MJ_TESTOUTER;
                }
                else
                {
-                   /* Go test the tuple */
-                   node->mj_JoinState = EXEC_MJ_TESTOUTER;
+                   /* Can't match, so fetch next outer tuple */
+                   node->mj_JoinState = EXEC_MJ_NEXTOUTER;
                }
                break;
 
@@ -1071,7 +1077,7 @@ ExecMergeJoin(MergeJoinState *node)
                 * tuple satisfy the merge clause then we know we have
                 * duplicates in the outer scan so we have to restore the
                 * inner scan to the marked tuple and proceed to join the
-                * new outer tuples with the inner tuples.
+                * new outer tuple with the inner tuples.
                 *
                 * This is the case when
                 *                        outer inner
@@ -1105,8 +1111,9 @@ ExecMergeJoin(MergeJoinState *node)
                MJ_printf("ExecMergeJoin: EXEC_MJ_TESTOUTER\n");
 
                /*
-                * here we must compare the outer tuple with the marked inner
-                * tuple
+                * Here we must compare the outer tuple with the marked inner
+                * tuple.  (We can ignore the result of MJEvalInnerValues,
+                * since the marked inner tuple is certainly matchable.)
                 */
                innerTupleSlot = node->mj_MarkedTupleSlot;
                (void) MJEvalInnerValues(node, innerTupleSlot);
@@ -1179,10 +1186,19 @@ ExecMergeJoin(MergeJoinState *node)
                    }
 
                    /* reload comparison data for current inner */
-                   (void) MJEvalInnerValues(node, innerTupleSlot);
-
-                   /* continue on to skip outer tuples */
-                   node->mj_JoinState = EXEC_MJ_SKIP_TEST;
+                   if (MJEvalInnerValues(node, innerTupleSlot))
+                   {
+                       /* proceed to compare it to the current outer */
+                       node->mj_JoinState = EXEC_MJ_SKIP_TEST;
+                   }
+                   else
+                   {
+                       /*
+                        * current inner can't possibly match any outer;
+                        * better to advance the inner scan than the outer.
+                        */
+                       node->mj_JoinState = EXEC_MJ_SKIPINNER_ADVANCE;
+                   }
                }
                break;
 
@@ -1293,15 +1309,16 @@ ExecMergeJoin(MergeJoinState *node)
                }
 
                /* Compute join values and check for unmatchability */
-               if (!MJEvalOuterValues(node))
+               if (MJEvalOuterValues(node))
                {
-                   /* Stay in same state to fetch next outer tuple */
+                   /* Go test the new tuple against the current inner */
+                   node->mj_JoinState = EXEC_MJ_SKIP_TEST;
+               }
+               else
+               {
+                   /* Can't match, so fetch next outer tuple */
                    node->mj_JoinState = EXEC_MJ_SKIPOUTER_ADVANCE;
-                   break;
                }
-
-               /* Test the new tuple against the current inner */
-               node->mj_JoinState = EXEC_MJ_SKIP_TEST;
                break;
 
                /*
@@ -1356,15 +1373,19 @@ ExecMergeJoin(MergeJoinState *node)
                }
 
                /* Compute join values and check for unmatchability */
-               if (!MJEvalInnerValues(node, innerTupleSlot))
+               if (MJEvalInnerValues(node, innerTupleSlot))
                {
-                   /* Stay in same state to fetch next inner tuple */
+                   /* proceed to compare it to the current outer */
+                   node->mj_JoinState = EXEC_MJ_SKIP_TEST;
+               }
+               else
+               {
+                   /*
+                    * current inner can't possibly match any outer;
+                    * better to advance the inner scan than the outer.
+                    */
                    node->mj_JoinState = EXEC_MJ_SKIPINNER_ADVANCE;
-                   break;
                }
-
-               /* Test the new tuple against the current outer */
-               node->mj_JoinState = EXEC_MJ_SKIP_TEST;
                break;
 
                /*

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index b110f08130010e1fc34e59ee49c2eff37e5c4cdb..bd92397b3af7f520b5e7757edc63c996ed9afb50 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2182,3 +2182,31 @@ SELECT * FROM t3;
 ---+---
 (0 rows)
 
+--
+-- regression test for 8.1 merge right join bug
+--
+CREATE TEMP TABLE tt1 ( tt1_id int4, joincol int4 );
+INSERT INTO tt1 VALUES (1, 11);
+INSERT INTO tt1 VALUES (2, NULL);
+CREATE TEMP TABLE tt2 ( tt2_id int4, joincol int4 );
+INSERT INTO tt2 VALUES (21, 11);
+INSERT INTO tt2 VALUES (22, 11);
+set enable_hashjoin to off;
+set enable_nestloop to off;
+-- these should give the same results
+select tt1.*, tt2.* from tt1 left join tt2 on tt1.joincol = tt2.joincol;
+ tt1_id | joincol | tt2_id | joincol 
+--------+---------+--------+---------
+      1 |      11 |     21 |      11
+      1 |      11 |     22 |      11
+      2 |         |        |        
+(3 rows)
+
+select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;
+ tt1_id | joincol | tt2_id | joincol 
+--------+---------+--------+---------
+      1 |      11 |     21 |      11
+      1 |      11 |     22 |      11
+      2 |         |        |        
+(3 rows)
+

diff --git a/src/test/regress/expected/join_1.out b/src/test/regress/expected/join_1.out
index 172d6d6fe8ab3fc1285ae1848d76aa2c1491239f..6a38234c1de384d548ff8f42c135139dd5d7f095 100644 (file)
--- a/src/test/regress/expected/join_1.out
+++ b/src/test/regress/expected/join_1.out
@@ -2182,3 +2182,31 @@ SELECT * FROM t3;
 ---+---
 (0 rows)
 
+--
+-- regression test for 8.1 merge right join bug
+--
+CREATE TEMP TABLE tt1 ( tt1_id int4, joincol int4 );
+INSERT INTO tt1 VALUES (1, 11);
+INSERT INTO tt1 VALUES (2, NULL);
+CREATE TEMP TABLE tt2 ( tt2_id int4, joincol int4 );
+INSERT INTO tt2 VALUES (21, 11);
+INSERT INTO tt2 VALUES (22, 11);
+set enable_hashjoin to off;
+set enable_nestloop to off;
+-- these should give the same results
+select tt1.*, tt2.* from tt1 left join tt2 on tt1.joincol = tt2.joincol;
+ tt1_id | joincol | tt2_id | joincol 
+--------+---------+--------+---------
+      1 |      11 |     21 |      11
+      1 |      11 |     22 |      11
+      2 |         |        |        
+(3 rows)
+
+select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;
+ tt1_id | joincol | tt2_id | joincol 
+--------+---------+--------+---------
+      1 |      11 |     21 |      11
+      1 |      11 |     22 |      11
+      2 |         |        |        
+(3 rows)
+

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index ffc2afe6ed441e2ec996769fd5a4d49b8bee4bcd..149cdadc19084714c4f9ae9129f78198890b69a5 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -369,3 +369,24 @@ DELETE FROM t3 USING t1 JOIN t2 USING (a) WHERE t3.x > t1.a;
 SELECT * FROM t3;
 DELETE FROM t3 USING t3 t3_other WHERE t3.x = t3_other.x AND t3.y = t3_other.y;
 SELECT * FROM t3;
+
+--
+-- regression test for 8.1 merge right join bug
+--
+
+CREATE TEMP TABLE tt1 ( tt1_id int4, joincol int4 );
+INSERT INTO tt1 VALUES (1, 11);
+INSERT INTO tt1 VALUES (2, NULL);
+
+CREATE TEMP TABLE tt2 ( tt2_id int4, joincol int4 );
+INSERT INTO tt2 VALUES (21, 11);
+INSERT INTO tt2 VALUES (22, 11);
+
+set enable_hashjoin to off;
+set enable_nestloop to off;
+
+-- these should give the same results
+
+select tt1.*, tt2.* from tt1 left join tt2 on tt1.joincol = tt2.joincol;
+
+select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;