Skip to content

Commit

Permalink
Clarify currently buggy behaviour
Browse files Browse the repository at this point in the history
  • Loading branch information
RobinTF committed Jul 11, 2024
1 parent 373f009 commit 27e451e
Showing 1 changed file with 21 additions and 10 deletions.
31 changes: 21 additions & 10 deletions src/engine/Operation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,23 +152,19 @@ CacheValue Operation::runComputationAndTransformToCache(
Result Operation::extractFromCache(
std::shared_ptr<const CacheableResult> result, bool freshlyInserted,
bool isRoot, ComputationMode computationMode) {
if (result->isDataEvaluated()) {
auto resultNumRows = result->idTable().size();
auto resultNumCols = result->idTable().numColumns();
LOG(DEBUG) << "Computed result of size " << resultNumRows << " x "
<< resultNumCols << std::endl;
}

// Keep backwards compatible for operations that don't support this
if (!result->isDataEvaluated() &&
computationMode == ComputationMode::FULLY_MATERIALIZED) {
auto& cache = _executionContext->getQueryTreeCache();
auto cacheKey = getCacheKey();
try {
cache.transformValue(getCacheKey(), [](const CacheValue& oldValue) {
cache.transformValue(getCacheKey(), [&result](
const CacheValue& oldValue) {
const auto& oldResult = oldValue.resultTable();
return CacheValue{CacheableResult{oldResult.aggregateTable().value()},
oldValue.runtimeInfo()};
CacheValue value{CacheableResult{oldResult.aggregateTable().value()},
oldValue.runtimeInfo()};
result = value.resultTablePtr();
return value;
});
} catch (const std::bad_optional_access&) {
ad_utility::Timer timer{ad_utility::Timer::Started};
Expand All @@ -180,10 +176,25 @@ Result Operation::extractFromCache(
return value;
});
}

Check warning on line 178 in src/engine/Operation.cpp

View check run for this annotation

Codecov / codecov/patch

src/engine/Operation.cpp#L158-L178

Added lines #L158 - L178 were not covered by tests
// TODO<RobinTF> In rare cases this assertion might be violated when
// the cache is cleared while we are transforming. The solution would be to
// replace transformValue with a solution that is part of computeOnce that
// calls some sort of "cache entry needs recomputing check" and replaces the
// value while blocking the individual entry, but not the whole cache.
AD_CORRECTNESS_CHECK(result->isDataEvaluated());
}

Check warning on line 185 in src/engine/Operation.cpp

View check run for this annotation

Codecov / codecov/patch

src/engine/Operation.cpp#L184-L185

Added lines #L184 - L185 were not covered by tests

if (result->isDataEvaluated()) {
auto resultNumRows = result->idTable().size();
auto resultNumCols = result->idTable().numColumns();
LOG(DEBUG) << "Computed result of size " << resultNumRows << " x "
<< resultNumCols << std::endl;
}

if (result->isDataEvaluated()) {
return Result::createResultWithFullyEvaluatedIdTable(std::move(result));
}

if (freshlyInserted) {
return Result::createResultAsMasterConsumer(
std::move(result),
Expand Down

0 comments on commit 27e451e

Please sign in to comment.