From 99e9bd1331eb96ef51756b7725c7018d4508e763 Mon Sep 17 00:00:00 2001 From: RobinTF <83676088+RobinTF@users.noreply.github.com> Date: Sun, 17 Nov 2024 21:22:01 +0100 Subject: [PATCH] Don't ignore vocab from side table --- src/engine/TransitivePathImpl.h | 42 ++++++++++++++------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/engine/TransitivePathImpl.h b/src/engine/TransitivePathImpl.h index 638f1784b9..01d9119857 100644 --- a/src/engine/TransitivePathImpl.h +++ b/src/engine/TransitivePathImpl.h @@ -78,26 +78,21 @@ class TransitivePathImpl : public TransitivePathBase { // constant overhead, which should be safe to ignore. runtimeInfo().addDetail("Initialization time", timer.msecs().count()); - NodeGenerator hull = transitiveHull( - edges, yieldOnce ? LocalVocab{} : sub->getCopyOfLocalVocab(), - std::move(nodes), - targetSide.isVariable() - ? std::nullopt - : std::optional{std::get(targetSide.value_)}); + NodeGenerator hull = + transitiveHull(edges, sub->getCopyOfLocalVocab(), std::move(nodes), + targetSide.isVariable() + ? std::nullopt + : std::optional{std::get(targetSide.value_)}, + yieldOnce); auto result = fillTableWithHull( std::move(hull), startSide.outputCol_, targetSide.outputCol_, startSide.treeAndCol_.value().second, yieldOnce, startSide.treeAndCol_.value().first->getResultWidth()); + // Iterate over generator to prevent lifetime issues for (auto& pair : result) { - // Skip merging of the same vocab for this case. - if (yieldOnce) { - co_yield Result::IdTableVocabPair{std::move(pair.first), - sub->getCopyOfLocalVocab()}; - } else { - co_yield pair; - } + co_yield pair; } }; @@ -134,23 +129,18 @@ class TransitivePathImpl : public TransitivePathBase { nullptr, nodesWithoutDuplicates, LocalVocab{}}; NodeGenerator hull = transitiveHull( - edges, yieldOnce ? LocalVocab{} : sub->getCopyOfLocalVocab(), - std::span{&tableInfo, 1}, + edges, sub->getCopyOfLocalVocab(), std::span{&tableInfo, 1}, targetSide.isVariable() ? std::nullopt - : std::optional{std::get(targetSide.value_)}); + : std::optional{std::get(targetSide.value_)}, + yieldOnce); auto result = fillTableWithHull(std::move(hull), startSide.outputCol_, targetSide.outputCol_, yieldOnce); + // Iterate over generator to prevent lifetime issues for (auto& pair : result) { - // Skip merging of the same vocab for this case. - if (yieldOnce) { - co_yield Result::IdTableVocabPair{std::move(pair.first), - sub->getCopyOfLocalVocab()}; - } else { - co_yield pair; - } + co_yield pair; } }; @@ -256,7 +246,7 @@ class TransitivePathImpl : public TransitivePathBase { */ NodeGenerator transitiveHull(const T& edges, LocalVocab edgesVocab, std::ranges::range auto startNodes, - std::optional target) const { + std::optional target, bool yieldOnce) const { ad_utility::Timer timer{ad_utility::Timer::Stopped}; for (auto&& tableColumn : startNodes) { timer.cont(); @@ -272,6 +262,10 @@ class TransitivePathImpl : public TransitivePathBase { mergedVocab.clone(), tableColumn.table_, currentRow}; timer.cont(); + // Reset vocab to prevent merging the same vocab over and over again. + if (yieldOnce) { + mergedVocab = LocalVocab{}; + } } currentRow++; }