From 9ada194a1349716f62c9a417508c81fa106cb8f3 Mon Sep 17 00:00:00 2001 From: Johannes Kalmbach Date: Thu, 19 Dec 2024 10:39:51 +0100 Subject: [PATCH] In the middle of fixing the tests... Signed-off-by: Johannes Kalmbach --- src/engine/IndexScan.cpp | 11 ++++++++-- test/engine/IndexScanTest.cpp | 41 ++++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/engine/IndexScan.cpp b/src/engine/IndexScan.cpp index 279887c72..2b19356e0 100644 --- a/src/engine/IndexScan.cpp +++ b/src/engine/IndexScan.cpp @@ -209,7 +209,7 @@ std::shared_ptr IndexScan::makeCopyWithAddedPrefilters( Result::Generator IndexScan::chunkedIndexScan() { auto optBlocks = [this]() -> std::optional> { - if (prefilteredBlocks_.has_value()) { + if (getLimit().isUnconstrained() && prefilteredBlocks_.has_value()) { return prefilteredBlocks_; } auto optAllBlocks = getBlockMetadata(); @@ -366,7 +366,8 @@ IndexScan::getBlockMetadataOptionallyPrefiltered() const { // The code after this is expensive because it always copies the complete // block metadata, so we do an early return of `nullopt` (which means "use // all the blocks") if no prefilter is specified. - if (!prefilter_.has_value() && !prefilteredBlocks_.has_value()) { + if ((!prefilter_.has_value() && !prefilteredBlocks_.has_value()) || + !getLimit().isUnconstrained()) { return std::nullopt; } @@ -721,7 +722,13 @@ void IndexScan::setBlocksForJoinOfIndexScans(Operation* left, auto metaBlocks1 = getBlocks(leftScan); auto metaBlocks2 = getBlocks(rightScan); + + // If one of the relations doesn't even exist and therefore has no blocks, + // we know that the join result is empty and can thus inform the other scan; + if (!metaBlocks1.has_value() || !metaBlocks2.has_value()) { + leftScan.prefilteredBlocks_.emplace(); + rightScan.prefilteredBlocks_.emplace(); return; } LOG(INFO) << "Original num blocks: " << metaBlocks1->blockMetadata_.size() diff --git a/test/engine/IndexScanTest.cpp b/test/engine/IndexScanTest.cpp index b067e1b51..7d366bd0d 100644 --- a/test/engine/IndexScanTest.cpp +++ b/test/engine/IndexScanTest.cpp @@ -57,18 +57,11 @@ void testLazyScan(Permutation::IdTableGenerator partialLazyScanResult, ++numBlocks; } - if (limitOffset.isUnconstrained()) { - EXPECT_EQ(numBlocks, partialLazyScanResult.details().numBlocksRead_); - // The number of read elements might be a bit larger than the final result - // size, because the first and/or last block might be incomplete, meaning - // that they have to be completely read, but only partially contribute to - // the result. - EXPECT_LE(lazyScanRes.size(), - partialLazyScanResult.details().numElementsRead_); - } - auto resFullScan = fullScan.getResult()->idTable().clone(); IdTable expected{resFullScan.numColumns(), alloc}; + std::cout << "Result of full scan" << IdTable(resFullScan.clone()) + << std::endl; + std::cout << fullScan.getDescriptor() << std::endl; if (limitOffset.isUnconstrained()) { for (auto [lower, upper] : expectedRows) { @@ -84,12 +77,14 @@ void testLazyScan(Permutation::IdTableGenerator partialLazyScanResult, } if (limitOffset.isUnconstrained()) { - EXPECT_EQ(lazyScanRes, expected); + EXPECT_EQ(lazyScanRes, expected) << IdTable{resFullScan.clone()}; } else { // If the join on blocks could already determine that there are no matching // blocks, then the lazy scan will be empty even with a limit present. + // TODO Handle the limit EXPECT_TRUE((lazyScanRes.empty() && expectedRows.empty()) || - lazyScanRes == expected); + lazyScanRes == expected) + << "actual:" << lazyScanRes << "expected:" << expected; } } @@ -116,15 +111,25 @@ void testLazyScanForJoinOfTwoScans( IndexScan s1Copy{qec, Permutation::PSO, tripleLeft}; s1.setLimit(limit); IndexScan s2{qec, Permutation::PSO, tripleRight}; - IndexScan s2Copy{qec, Permutation::PSO, tripleLeft}; + IndexScan s2Copy{qec, Permutation::PSO, tripleRight}; IndexScan::setBlocksForJoinOfIndexScans(&s1, &s2); + s1.disableStoringInCache(); + s2.disableStoringInCache(); + + if (!limit.isUnconstrained()) { + std::cout << "has limit\n"; + } // TODO also switch the left and right inputs for the test auto implForSwitch = - [](IndexScan& l, IndexScan& l2, IndexScan& r, IndexScan& r2, - const auto& expectedL, const auto& expectedR, - const LimitOffsetClause& limitL, const LimitOffsetClause& limitR) { + [&qec](IndexScan& l, IndexScan& l2, IndexScan& r, IndexScan& r2, + const auto& expectedL, const auto& expectedR, + const LimitOffsetClause& limitL, const LimitOffsetClause& limitR, + ad_utility::source_location location = + ad_utility::source_location::current()) { + auto tr = generateLocationTrace(location); + qec->getQueryTreeCache().clearAll(); auto res1 = l.computeResultOnlyForTesting(true); auto res2 = r.computeResultOnlyForTesting(true); testLazyScan(convertGenerator(std::move(res1.idTables())), l2, @@ -235,6 +240,7 @@ const auto testSetAndMakeScanWithPrefilterExpr = TEST(IndexScan, lazyScanForJoinOfTwoScans) { SparqlTriple xpy{Tc{Var{"?x"}}, "

", Tc{Var{"?y"}}}; SparqlTriple xqz{Tc{Var{"?x"}}, "", Tc{Var{"?z"}}}; + /* { // In the tests we have a blocksize of two triples per block, and a new // block is started for a new relation. That explains the spacing of the @@ -259,8 +265,9 @@ TEST(IndexScan, lazyScanForJoinOfTwoScans) { // graph), so both lazy scans are empty. testLazyScanForJoinOfTwoScans(kg, xpy, xqz, {}, {}); } + */ { - // No triple for relation (which does appear in the knowledge graph, but + // No triple for relation (which does appear in the knowledge graph, but // not as a predicate), so both lazy scans are empty. std::string kg = "

.

. "