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.

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 memory leak issue when running window fuzzer test #10847

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions velox/exec/Window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,14 @@ Window::WindowFrame Window::createWindowFrame(
}
};

auto startFrameArg = createFrameChannelArg(frame.startValue);
auto endFrameArg = createFrameChannelArg(frame.endValue);
return WindowFrame(
{frame.type,
frame.startType,
frame.endType,
createFrameChannelArg(frame.startValue),
createFrameChannelArg(frame.endValue)});
std::move(startFrameArg),
std::move(endFrameArg)});
}

void Window::createWindowFunctions() {
Expand Down
64 changes: 64 additions & 0 deletions velox/exec/tests/WindowTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,5 +439,69 @@ TEST_F(WindowTest, duplicateOrOverlappingKeys) {
"Sorting keys must be unique and not overlap with partitioning keys. Found duplicate key: b");
}

TEST_F(WindowTest, nagativeFrameArg) {
const vector_size_t size = 1'000;

auto sizeAt = [](vector_size_t row) { return row % 5; };
auto keyAt = [](vector_size_t row) { return row % 11; };
auto valueAt = [](vector_size_t row) { return row % 13; };
auto keys = makeArrayVector<float>(size, sizeAt, keyAt);
auto data = makeRowVector(
{"c0", "c1", "p0", "p1", "k0", "row_number"},
{
// Payload.
makeFlatVector<float>(size, [](auto row) { return row; }),
makeFlatVector<float>(size, [](auto row) { return row; }),
// Partition key.
keys,
makeFlatVector<std::string>(
size, [](auto row) { return fmt::format("{}", row + 20); }),
makeFlatVector<int32_t>(size, [](auto row) { return row; }),
// Sorting key.
makeFlatVector<int64_t>(size, [](auto row) { return row; }),
});

createDuckDbTable({data});

struct {
std::string fragmentStart;
std::string fragmentEnd;

std::string debugString() const {
if (fragmentStart[0] == '-') {
return fmt::format(
"Window frame {} offset must not be negative", fragmentStart);
} else {
return fmt::format(
"Window frame {} offset must not be negative", fragmentEnd);
}
}
} testSettings[] = {
{"k0", "-1"}, // Negative end
{"-1", "k0"}, // Negative start
{"-1", "-3"} // Negative start, negative end
};
for (const auto& testData : testSettings) {
SCOPED_TRACE(testData.debugString());
const auto& startOffset = testData.fragmentStart;
const auto& endOffset = testData.fragmentEnd;
auto plan =
PlanBuilder()
.values(split(data, 10))
.window({fmt::format(
"regr_count(c0, c1) over (partition by p0, p1 order by row_number ROWS between {} PRECEDING and {} FOLLOWING)",
startOffset,
endOffset)})
.planNode();
VELOX_ASSERT_USER_THROW(
AssertQueryBuilder(plan, duckDbQueryRunner_)
.assertResults(fmt::format(
"SELECT *, regr_count(c0, c1) over (partition by p0, p1 order by row_number ROWS between {} PRECEDING and {} FOLLOWING) from tmp",
startOffset,
endOffset)),
testData.debugString());
}
}

} // namespace
} // namespace facebook::velox::exec
Loading