Skip to content

Commit

Permalink
Fix null value check for kRange frame column
Browse files Browse the repository at this point in the history
  • Loading branch information
pramodsatya committed Sep 24, 2024
1 parent dcaae29 commit 0b9d3d3
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
5 changes: 4 additions & 1 deletion velox/exec/WindowPartition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,11 @@ void WindowPartition::updateKRangeFrameBounds(

vector_size_t start = 0;
vector_size_t end;
// frameColumn is a column index into the original input rows, while
// orderByColumn is a column index into rows in data_ after the columns are
// reordered as per inputMapping_.
RowColumn frameRowColumn = columns_[frameColumn];
RowColumn orderByRowColumn = columns_[inputMapping_[orderByColumn]];
RowColumn orderByRowColumn = data_->columnAt(orderByColumn);
for (auto i = 0; i < numRows; i++) {
auto currentRow = startRow + i;
auto* partitionRow = partition_[currentRow];
Expand Down
36 changes: 36 additions & 0 deletions velox/exec/tests/WindowTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,5 +487,41 @@ TEST_F(WindowTest, nagativeFrameArg) {
}
}

DEBUG_ONLY_TEST_F(WindowTest, frameColumnNullCheck) {
auto makePlan = [&](const RowVectorPtr& input) {
return PlanBuilder()
.values({input})
.window(
{"sum(c0) OVER (PARTITION BY p0 ORDER BY s0 RANGE BETWEEN UNBOUNDED PRECEDING AND off0 FOLLOWING)"})
.planNode();
};

// Null values in order-by column 's0' and frame column 'off0' do not match,
// so exception is expected.
auto inputThrow = makeRowVector(
{"c0", "p0", "s0", "off0"},
{
makeNullableFlatVector<int64_t>({1, std::nullopt, 1, 2, 2}),
makeFlatVector<int64_t>({1, 2, 1, 2, 1}),
makeNullableFlatVector<int64_t>({1, 2, 3, std::nullopt, 5}),
makeNullableFlatVector<int64_t>({2, std::nullopt, 4, 5, 6}),
});
VELOX_ASSERT_THROW(
AssertQueryBuilder(makePlan(inputThrow)).copyResults(pool()), "");

// Null values in order-by column 's0' and frame column 'off0' match, so no
// exception should be thrown.
auto inputNoThrow = makeRowVector(
{"c0", "p0", "s0", "off0"},
{
makeNullableFlatVector<int64_t>({1, 1, 2, std::nullopt, 2}),
makeFlatVector<int64_t>({1, 1, 1, 2, 2}),
makeNullableFlatVector<int64_t>({1, std::nullopt, 2, 3, 5}),
makeNullableFlatVector<int64_t>({2, std::nullopt, 3, 4, 6}),
});
ASSERT_NO_THROW(
AssertQueryBuilder(makePlan(inputNoThrow)).copyResults(pool()));
}

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

0 comments on commit 0b9d3d3

Please sign in to comment.