From a2abf600457ff4002bdbb5105149e5bbce717a7a Mon Sep 17 00:00:00 2001 From: Gerke Geurts Date: Thu, 12 Dec 2013 20:39:02 +0100 Subject: [PATCH] Issue #13: Test and fix for loss of final rows by SortMergeJoinOperation. --- .../Operations/SortMergeJoinOperation.cs | 12 +++-- Rhino.Etl.Tests/Joins/BaseJoinFixture.cs | 1 - Rhino.Etl.Tests/Joins/JoinFixture.cs | 48 ++++++++++++++++++- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/Rhino.Etl.Core/Operations/SortMergeJoinOperation.cs b/Rhino.Etl.Core/Operations/SortMergeJoinOperation.cs index 8c9d3ed..e670112 100644 --- a/Rhino.Etl.Core/Operations/SortMergeJoinOperation.cs +++ b/Rhino.Etl.Core/Operations/SortMergeJoinOperation.cs @@ -60,11 +60,11 @@ public override IEnumerable Execute(IEnumerable rows) rightRows.MoveNext(); Row rightRow = (Row) rightRows.Current; - while (leftRow != null && rightRow != null) + while (leftRow != null || rightRow != null) { - var match = MatchJoinCondition(leftRow, rightRow); Row mergedRow = null; + var match = CompareRows(leftRow, rightRow); if (match == 0) { mergedRow = MergeRows(leftRow, rightRow); @@ -101,9 +101,13 @@ public override IEnumerable Execute(IEnumerable rows) if (mergedRow != null) yield return mergedRow; } + } - if (leftRow == null && rightRow != null && (JoinType & JoinType.Right) != 0) - yield return MergeRows(new Row(), rightRow); + private int CompareRows(Row leftRow, Row rightRow) + { + return leftRow == null + ? (rightRow == null ? 0 : 1) + : (rightRow == null ? -1 : MatchJoinCondition(leftRow, rightRow)); } /// diff --git a/Rhino.Etl.Tests/Joins/BaseJoinFixture.cs b/Rhino.Etl.Tests/Joins/BaseJoinFixture.cs index 1b90767..9e68e7c 100644 --- a/Rhino.Etl.Tests/Joins/BaseJoinFixture.cs +++ b/Rhino.Etl.Tests/Joins/BaseJoinFixture.cs @@ -19,7 +19,6 @@ public BaseJoinFixture() AddPerson(3, "foo@example.org"); AddPerson(5, "silver@exaple.org"); - } protected void AddPerson(int id, string email) diff --git a/Rhino.Etl.Tests/Joins/JoinFixture.cs b/Rhino.Etl.Tests/Joins/JoinFixture.cs index f68bc11..0b1a836 100644 --- a/Rhino.Etl.Tests/Joins/JoinFixture.cs +++ b/Rhino.Etl.Tests/Joins/JoinFixture.cs @@ -157,6 +157,52 @@ public void SortMergeFullJoin() Assert.Null(items[2]["name"]); Assert.Equal(5, items[2]["person_id"]); } - } + } + + [Fact] + public void SortMergeFullJoinMergesMultipleNonMatchingLeftRowsAtEnd() + { + AddUser("toby", "toby@test.org"); + AddUser("tom", "tom@test.org"); + + using (FullMergeJoinUsersToPeopleByEmail join = new FullMergeJoinUsersToPeopleByEmail()) + { + join.Left(new GenericEnumerableOperation(left)) + .Right(new GenericEnumerableOperation(right)); + join.PrepareForExecution(new SingleThreadedPipelineExecuter()); + IEnumerable result = join.Execute(null); + List items = new List(result); + + Assert.Equal(5, items.Count); + Assert.Null(items[3]["person_id"]); + Assert.Equal("toby", items[3]["name"]); + + Assert.Null(items[4]["person_id"]); + Assert.Equal("tom", items[4]["name"]); + } + } + + [Fact] + public void SortMergeFullJoinMergesMultipleNonMatchingRightRowsAtEnd() + { + AddPerson(8, "toby@test.org"); + AddPerson(10, "tom@test.org"); + + using (FullMergeJoinUsersToPeopleByEmail join = new FullMergeJoinUsersToPeopleByEmail()) + { + join.Left(new GenericEnumerableOperation(left)) + .Right(new GenericEnumerableOperation(right)); + join.PrepareForExecution(new SingleThreadedPipelineExecuter()); + IEnumerable result = join.Execute(null); + List items = new List(result); + + Assert.Equal(5, items.Count); + Assert.Equal(8, items[3]["person_id"]); + Assert.Null(items[3]["name"]); + + Assert.Equal(10, items[4]["person_id"]); + Assert.Null(items[4]["name"]); + } + } } } \ No newline at end of file