From bbe56d48ab13425f14895bf24db74605b1d5e016 Mon Sep 17 00:00:00 2001 From: Johannes Kalmbach Date: Wed, 18 Dec 2024 15:00:19 +0100 Subject: [PATCH 1/3] Fix a small bug. Signed-off-by: Johannes Kalmbach --- src/engine/Filter.cpp | 4 +++- src/util/JoinAlgorithms/JoinAlgorithms.h | 3 +++ test/JoinAlgorithmsTest.cpp | 12 ++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/engine/Filter.cpp b/src/engine/Filter.cpp index 9ecdd85f7a..7f20b58108 100644 --- a/src/engine/Filter.cpp +++ b/src/engine/Filter.cpp @@ -75,7 +75,9 @@ ProtoResult Filter::computeResult(bool requestLaziness) { for (auto& [idTable, localVocab] : subRes->idTables()) { IdTable result = self->filterIdTable(subRes->sortedBy(), idTable, localVocab); - co_yield {std::move(result), std::move(localVocab)}; + if (!result.empty()) { + co_yield {std::move(result), std::move(localVocab)}; + } } }(std::move(subRes), this), resultSortedOn()}; diff --git a/src/util/JoinAlgorithms/JoinAlgorithms.h b/src/util/JoinAlgorithms/JoinAlgorithms.h index 89e1b6611f..be1402a966 100644 --- a/src/util/JoinAlgorithms/JoinAlgorithms.h +++ b/src/util/JoinAlgorithms/JoinAlgorithms.h @@ -801,6 +801,9 @@ struct BlockZipperJoinImpl { for (size_t numBlocksRead = 0; it != end && numBlocksRead < 3; ++it, ++numBlocksRead) { if (ql::ranges::empty(*it)) { + // We haven't read a block, so we have to adapt the counter. + // NOTE: The possible underflow for `size_t` is safe. + --numBlocksRead; continue; } if (!eq((*it)[0], currentEl)) { diff --git a/test/JoinAlgorithmsTest.cpp b/test/JoinAlgorithmsTest.cpp index 6996191e59..cac73cf2c1 100644 --- a/test/JoinAlgorithmsTest.cpp +++ b/test/JoinAlgorithmsTest.cpp @@ -182,6 +182,18 @@ TEST(JoinAlgorithms, JoinWithBlocksExactlyFourBlocksPerElement) { testJoin(a, b, expectedResult); } +// Test the handling of many empty blocks. +TEST(JoinAlgorithms, JoinWithEmptyBlocks) { + NestedBlock a{{{42, 1}, {42, 2}}, {{42, 3}}}; + // The join has to handle all the entries with `42` in the first column at + // once. In `b` there are more than 3 empty blocks between blocks with this + // entry. There was previously a bug in this case. + NestedBlock b{{{42, 16}}, {}, {}, {}, {}, {}, {}, {}, {}, {{42, 23}}}; + JoinResult expectedResult{{42, 1, 16}, {42, 1, 23}, {42, 2, 16}, + {42, 2, 23}, {42, 3, 16}, {42, 3, 23}}; + testJoin(a, b, expectedResult); +} + // ________________________________________________________________________________________ TEST(JoinAlgorithms, JoinWithBlocksMultipleBlocksPerElementBothSides) { NestedBlock a{{{42, 0}}, {{42, 1}, {42, 2}}, {{42, 3}, {67, 0}}}; From 3e0d494821285d33b7b8c691a6dcd1d7b0b6c14f Mon Sep 17 00:00:00 2001 From: Johannes Kalmbach Date: Wed, 18 Dec 2024 16:33:08 +0100 Subject: [PATCH 2/3] Fix the unit tests. Signed-off-by: Johannes Kalmbach --- test/FilterTest.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/FilterTest.cpp b/test/FilterTest.cpp index 7df2617c3f..b2201c2e94 100644 --- a/test/FilterTest.cpp +++ b/test/FilterTest.cpp @@ -87,13 +87,11 @@ TEST(Filter, verifyPredicateIsAppliedCorrectlyOnLazyEvaluation) { auto referenceTable1 = makeIdTableFromVector({{true}, {true}, {true}}, asBool); auto referenceTable2 = makeIdTableFromVector({{true}}, asBool); - IdTable referenceTable3{0, ad_utility::makeUnlimitedAllocator()}; auto m = matchesIdTable; EXPECT_THAT( toVector(std::move(generator)), - ElementsAre(m(referenceTable1), m(referenceTable2), m(referenceTable3), - m(referenceTable3), m(referenceTable2))); + ElementsAre(m(referenceTable1), m(referenceTable2), m(referenceTable2))); } // _____________________________________________________________________________ From 06cfda1084bba428b18da56c013bdf802efb059a Mon Sep 17 00:00:00 2001 From: Johannes Kalmbach Date: Fri, 20 Dec 2024 15:23:12 +0100 Subject: [PATCH 3/3] Fix the wording of a comment. Signed-off-by: Johannes Kalmbach --- src/util/JoinAlgorithms/JoinAlgorithms.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/JoinAlgorithms/JoinAlgorithms.h b/src/util/JoinAlgorithms/JoinAlgorithms.h index be1402a966..0b897e1c8e 100644 --- a/src/util/JoinAlgorithms/JoinAlgorithms.h +++ b/src/util/JoinAlgorithms/JoinAlgorithms.h @@ -802,7 +802,8 @@ struct BlockZipperJoinImpl { ++it, ++numBlocksRead) { if (ql::ranges::empty(*it)) { // We haven't read a block, so we have to adapt the counter. - // NOTE: The possible underflow for `size_t` is safe. + // NOTE: If `numBlocksRead == 0`, the underflow is no problem because + // the next operation is `++`. --numBlocksRead; continue; }