Skip to content

Commit

Permalink
Fix fast path in map Presto function for input arrays with nulls (#7862)
Browse files Browse the repository at this point in the history
Summary:
Fast path in the "map" Presto function could produce invalid MapVector if input
arrays had null rows with out-of-bounds  offsets/sizes.

Fixes facebookincubator/velox#7861

Pull Request resolved: facebookincubator/velox#7862

Reviewed By: xiaoxmeng

Differential Revision: D51814196

Pulled By: mbasmanova

fbshipit-source-id: 791bfa01e4e34c164f83117c23c34f286b3ff881
  • Loading branch information
mbasmanova authored and facebook-github-bot committed Dec 4, 2023
1 parent e4b9def commit add8303
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
10 changes: 9 additions & 1 deletion velox/functions/prestosql/Map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class MapFunction : public exec::VectorFunction {
auto mapVector = std::make_shared<MapVector>(
context.pool(),
outputType,
nullptr,
keysArray->nulls(),
rows.end(),
keysArray->offsets(),
keysArray->sizes(),
Expand Down Expand Up @@ -306,6 +306,14 @@ class MapFunction : public exec::VectorFunction {
return false;
}
for (auto row = 0; row < keys->size(); ++row) {
if (keys->isNullAt(row)) {
continue;
}

if (values->isNullAt(row)) {
return false;
}

if (keys->offsetAt(row) != values->offsetAt(row) ||
keys->sizeAt(row) != values->sizeAt(row)) {
return false;
Expand Down
30 changes: 30 additions & 0 deletions velox/functions/prestosql/tests/MapTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,36 @@ TEST_F(MapTest, someNulls) {
assertEqualVectors(expectedMap, result);
}

TEST_F(MapTest, nullWithNonZeroSizes) {
auto keys = makeArrayVectorFromJson<int32_t>({
"[1, 2, 3]",
"[1, 2]",
"[1, 2, 3]",
});

auto values = makeArrayVectorFromJson<int64_t>({
"[10, 20, 30]",
"[11, 21]",
"[12, 22, 32]",
});

// Set null for one of the rows. Also, set offset and size for the row to
// values that exceed the size of the 'elements' vector.
keys->setNull(1, true);
keys->setOffsetAndSize(1, 100, 10);
values->setNull(1, true);
values->setOffsetAndSize(1, 100, 10);

auto result = evaluate("map(c0, c1)", makeRowVector({keys, values}));

auto expected = makeMapVectorFromJson<int32_t, int64_t>({
"{1: 10, 2: 20, 3: 30}",
"null",
"{1: 12, 2: 22, 3: 32}",
});
assertEqualVectors(expected, result);
}

TEST_F(MapTest, partiallyPopulated) {
auto size = 1'000;

Expand Down

0 comments on commit add8303

Please sign in to comment.