Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b3358e2

Browse files
tglsfdctglsfdc
committed
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.
1 parent 5094f99 commit b3358e2

File tree

4 files changed

+127
-29
lines changed

4 files changed

+127
-29
lines changed

‎src/backend/executor/nodeMergejoin.c

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeMergejoin.c,v 1.78 2006/03/05 15:58:26 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeMergejoin.c,v 1.79 2006/03/17 19:38:12 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -948,9 +948,6 @@ ExecMergeJoin(MergeJoinState *node)
948948
* now we get the next inner tuple, if any. If there's none,
949949
* advance to next outer tuple (which may be able to join to
950950
* previously marked tuples).
951-
*
952-
* If we find one but it cannot join to anything, stay in
953-
* NEXTINNER state to fetch the next one.
954951
*/
955952
innerTupleSlot = ExecProcNode(innerPlan);
956953
node->mj_InnerTupleSlot = innerTupleSlot;
@@ -963,8 +960,17 @@ ExecMergeJoin(MergeJoinState *node)
963960
break;
964961
}
965962

963+
/*
964+
* Load up the new inner tuple's comparison values. If we
965+
* see that it contains a NULL and hence can't match any
966+
* outer tuple, we can skip the comparison and assume the
967+
* new tuple is greater than current outer.
968+
*/
966969
if (!MJEvalInnerValues(node, innerTupleSlot))
967-
break; /* stay in NEXTINNER state */
970+
{
971+
node->mj_JoinState = EXEC_MJ_NEXTOUTER;
972+
break;
973+
}
968974

969975
/*
970976
* Test the new inner tuple to see if it matches outer.
@@ -1054,15 +1060,15 @@ ExecMergeJoin(MergeJoinState *node)
10541060
}
10551061

10561062
/* Compute join values and check for unmatchability */
1057-
if (!MJEvalOuterValues(node))
1063+
if (MJEvalOuterValues(node))
10581064
{
1059-
/* Stay in same state to fetch next outer tuple */
1060-
node->mj_JoinState = EXEC_MJ_NEXTOUTER;
1065+
/* Go test the new tuple against the marked tuple */
1066+
node->mj_JoinState = EXEC_MJ_TESTOUTER;
10611067
}
10621068
else
10631069
{
1064-
/* Go test the tuple */
1065-
node->mj_JoinState = EXEC_MJ_TESTOUTER;
1070+
/* Can't match, so fetch next outer tuple */
1071+
node->mj_JoinState = EXEC_MJ_NEXTOUTER;
10661072
}
10671073
break;
10681074

@@ -1071,7 +1077,7 @@ ExecMergeJoin(MergeJoinState *node)
10711077
* tuple satisfy the merge clause then we know we have
10721078
* duplicates in the outer scan so we have to restore the
10731079
* inner scan to the marked tuple and proceed to join the
1074-
* new outer tuples with the inner tuples.
1080+
* new outer tuple with the inner tuples.
10751081
*
10761082
* This is the case when
10771083
* outer inner
@@ -1105,8 +1111,9 @@ ExecMergeJoin(MergeJoinState *node)
11051111
MJ_printf("ExecMergeJoin: EXEC_MJ_TESTOUTER\n");
11061112

11071113
/*
1108-
* here we must compare the outer tuple with the marked inner
1109-
* tuple
1114+
* Here we must compare the outer tuple with the marked inner
1115+
* tuple. (We can ignore the result of MJEvalInnerValues,
1116+
* since the marked inner tuple is certainly matchable.)
11101117
*/
11111118
innerTupleSlot = node->mj_MarkedTupleSlot;
11121119
(void) MJEvalInnerValues(node, innerTupleSlot);
@@ -1179,10 +1186,19 @@ ExecMergeJoin(MergeJoinState *node)
11791186
}
11801187

11811188
/* reload comparison data for current inner */
1182-
(void) MJEvalInnerValues(node, innerTupleSlot);
1183-
1184-
/* continue on to skip outer tuples */
1185-
node->mj_JoinState = EXEC_MJ_SKIP_TEST;
1189+
if (MJEvalInnerValues(node, innerTupleSlot))
1190+
{
1191+
/* proceed to compare it to the current outer */
1192+
node->mj_JoinState = EXEC_MJ_SKIP_TEST;
1193+
}
1194+
else
1195+
{
1196+
/*
1197+
* current inner can't possibly match any outer;
1198+
* better to advance the inner scan than the outer.
1199+
*/
1200+
node->mj_JoinState = EXEC_MJ_SKIPINNER_ADVANCE;
1201+
}
11861202
}
11871203
break;
11881204

@@ -1293,15 +1309,16 @@ ExecMergeJoin(MergeJoinState *node)
12931309
}
12941310

12951311
/* Compute join values and check for unmatchability */
1296-
if (!MJEvalOuterValues(node))
1312+
if (MJEvalOuterValues(node))
12971313
{
1298-
/* Stay in same state to fetch next outer tuple */
1314+
/* Go test the new tuple against the current inner */
1315+
node->mj_JoinState = EXEC_MJ_SKIP_TEST;
1316+
}
1317+
else
1318+
{
1319+
/* Can't match, so fetch next outer tuple */
12991320
node->mj_JoinState = EXEC_MJ_SKIPOUTER_ADVANCE;
1300-
break;
13011321
}
1302-
1303-
/* Test the new tuple against the current inner */
1304-
node->mj_JoinState = EXEC_MJ_SKIP_TEST;
13051322
break;
13061323

13071324
/*
@@ -1356,15 +1373,19 @@ ExecMergeJoin(MergeJoinState *node)
13561373
}
13571374

13581375
/* Compute join values and check for unmatchability */
1359-
if (!MJEvalInnerValues(node, innerTupleSlot))
1376+
if (MJEvalInnerValues(node, innerTupleSlot))
13601377
{
1361-
/* Stay in same state to fetch next inner tuple */
1378+
/* proceed to compare it to the current outer */
1379+
node->mj_JoinState = EXEC_MJ_SKIP_TEST;
1380+
}
1381+
else
1382+
{
1383+
/*
1384+
* current inner can't possibly match any outer;
1385+
* better to advance the inner scan than the outer.
1386+
*/
13621387
node->mj_JoinState = EXEC_MJ_SKIPINNER_ADVANCE;
1363-
break;
13641388
}
1365-
1366-
/* Test the new tuple against the current outer */
1367-
node->mj_JoinState = EXEC_MJ_SKIP_TEST;
13681389
break;
13691390

13701391
/*

‎src/test/regress/expected/join.out

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2182,3 +2182,31 @@ SELECT * FROM t3;
21822182
---+---
21832183
(0 rows)
21842184

2185+
--
2186+
-- regression test for 8.1 merge right join bug
2187+
--
2188+
CREATE TEMP TABLE tt1 ( tt1_id int4, joincol int4 );
2189+
INSERT INTO tt1 VALUES (1, 11);
2190+
INSERT INTO tt1 VALUES (2, NULL);
2191+
CREATE TEMP TABLE tt2 ( tt2_id int4, joincol int4 );
2192+
INSERT INTO tt2 VALUES (21, 11);
2193+
INSERT INTO tt2 VALUES (22, 11);
2194+
set enable_hashjoin to off;
2195+
set enable_nestloop to off;
2196+
-- these should give the same results
2197+
select tt1.*, tt2.* from tt1 left join tt2 on tt1.joincol = tt2.joincol;
2198+
tt1_id | joincol | tt2_id | joincol
2199+
--------+---------+--------+---------
2200+
1 | 11 | 21 | 11
2201+
1 | 11 | 22 | 11
2202+
2 | | |
2203+
(3 rows)
2204+
2205+
select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;
2206+
tt1_id | joincol | tt2_id | joincol
2207+
--------+---------+--------+---------
2208+
1 | 11 | 21 | 11
2209+
1 | 11 | 22 | 11
2210+
2 | | |
2211+
(3 rows)
2212+

‎src/test/regress/expected/join_1.out

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2182,3 +2182,31 @@ SELECT * FROM t3;
21822182
---+---
21832183
(0 rows)
21842184

2185+
--
2186+
-- regression test for 8.1 merge right join bug
2187+
--
2188+
CREATE TEMP TABLE tt1 ( tt1_id int4, joincol int4 );
2189+
INSERT INTO tt1 VALUES (1, 11);
2190+
INSERT INTO tt1 VALUES (2, NULL);
2191+
CREATE TEMP TABLE tt2 ( tt2_id int4, joincol int4 );
2192+
INSERT INTO tt2 VALUES (21, 11);
2193+
INSERT INTO tt2 VALUES (22, 11);
2194+
set enable_hashjoin to off;
2195+
set enable_nestloop to off;
2196+
-- these should give the same results
2197+
select tt1.*, tt2.* from tt1 left join tt2 on tt1.joincol = tt2.joincol;
2198+
tt1_id | joincol | tt2_id | joincol
2199+
--------+---------+--------+---------
2200+
1 | 11 | 21 | 11
2201+
1 | 11 | 22 | 11
2202+
2 | | |
2203+
(3 rows)
2204+
2205+
select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;
2206+
tt1_id | joincol | tt2_id | joincol
2207+
--------+---------+--------+---------
2208+
1 | 11 | 21 | 11
2209+
1 | 11 | 22 | 11
2210+
2 | | |
2211+
(3 rows)
2212+

‎src/test/regress/sql/join.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,3 +369,24 @@ DELETE FROM t3 USING t1 JOIN t2 USING (a) WHERE t3.x > t1.a;
369369
SELECT * FROM t3;
370370
DELETE FROM t3 USING t3 t3_other WHERE t3.x = t3_other.x AND t3.y = t3_other.y;
371371
SELECT * FROM t3;
372+
373+
--
374+
-- regression test for 8.1 merge right join bug
375+
--
376+
377+
CREATE TEMP TABLE tt1 ( tt1_id int4, joincol int4 );
378+
INSERT INTO tt1 VALUES (1, 11);
379+
INSERT INTO tt1 VALUES (2, NULL);
380+
381+
CREATE TEMP TABLE tt2 ( tt2_id int4, joincol int4 );
382+
INSERT INTO tt2 VALUES (21, 11);
383+
INSERT INTO tt2 VALUES (22, 11);
384+
385+
set enable_hashjoin to off;
386+
set enable_nestloop to off;
387+
388+
-- these should give the same results
389+
390+
select tt1.*, tt2.* from tt1 left join tt2 on tt1.joincol = tt2.joincol;
391+
392+
select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol;

0 commit comments

Comments
 (0)