Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug for UNION with SUBQUERY with invisible variables #1682

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Fix bug for UNION with SUBQUERY with invisible variables #1682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/engine/Union.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,10 @@ std::vector<ColumnIndex> Union::computePermutation() const {
// _____________________________________________________________________________
IdTable Union::transformToCorrectColumnFormat(
IdTable idTable, const std::vector<ColumnIndex>& permutation) const {
while (idTable.numColumns() < getResultWidth()) {
// NOTE: previously the check was for `getResultWidth()`, but that is wrong if
// some variables in the subtree are invisible because of a subquery.
auto maxNumRequiredColumns = ql::ranges::max(permutation) + 1;
while (idTable.numColumns() < maxNumRequiredColumns) {
idTable.addEmptyColumn();
ad_utility::chunkedFill(idTable.getColumn(idTable.numColumns() - 1),
Id::makeUndefined(), chunkSize,
Expand Down
35 changes: 27 additions & 8 deletions test/UnionTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,34 @@ TEST(Union, computeUnionLarge) {

// _____________________________________________________________________________
TEST(Union, computeUnionLazy) {
auto runTest = [](bool nonLazyChildren,
auto runTest = [](bool nonLazyChildren, bool invisibleSubtreeColumns,
ad_utility::source_location loc =
ad_utility::source_location::current()) {
auto l = generateLocationTrace(loc);
auto* qec = ad_utility::testing::getQec();
qec->getQueryTreeCache().clearAll();
IdTable left = makeIdTableFromVector({{V(1)}, {V(2)}, {V(3)}});
auto leftT = ad_utility::makeExecutionTree<ValuesForTesting>(
qec, std::move(left), Vars{Variable{"?x"}}, false,
std::vector<ColumnIndex>{}, LocalVocab{}, std::nullopt,
nonLazyChildren);
auto leftT = [&]() {
if (!invisibleSubtreeColumns) {
IdTable left = makeIdTableFromVector({{V(1)}, {V(2)}, {V(3)}});
return ad_utility::makeExecutionTree<ValuesForTesting>(
qec, std::move(left), Vars{Variable{"?x"}}, false,
std::vector<ColumnIndex>{}, LocalVocab{}, std::nullopt,
nonLazyChildren);
} else {
// With the `invisibleSubtreeColumns==true` case we test the case, that
// the input contains variables that are not visible because of a
// subquery. This case was previously buggy and triggered an assertion.
IdTable left = makeIdTableFromVector(
{{V(1), V(3)}, {V(2), V(27)}, {V(3), V(123)}});
auto tree = ad_utility::makeExecutionTree<ValuesForTesting>(
qec, std::move(left), Vars{Variable{"?x"}, Variable{"?invisible"}},
false, std::vector<ColumnIndex>{}, LocalVocab{}, std::nullopt,
nonLazyChildren);
tree->getRootOperation()->setSelectedVariablesForSubquery(
{Variable{"?x"}});
return tree;
}
}();

IdTable right = makeIdTableFromVector({{V(4), V(5)}, {V(6), V(7)}});
auto rightT = ad_utility::makeExecutionTree<ValuesForTesting>(
Expand Down Expand Up @@ -112,8 +129,10 @@ TEST(Union, computeUnionLazy) {
ASSERT_EQ(++iterator, result.end());
};

runTest(false);
runTest(true);
runTest(false, false);
runTest(false, true);
runTest(true, false);
runTest(true, true);
}

// _____________________________________________________________________________
Expand Down
Loading