Skip to content

Commit

Permalink
Fix RowContainer::hash and equals for inputs of type UNKNOWN (faceboo…
Browse files Browse the repository at this point in the history
…kincubator#6619)

Summary:
Fixes facebookincubator#6616

Pull Request resolved: facebookincubator#6619

Reviewed By: bikramSingh91

Differential Revision: D49385313

Pulled By: mbasmanova

fbshipit-source-id: fb16a50ced469791cfab088568829969c2302243
  • Loading branch information
mbasmanova authored and ericyuliu committed Oct 12, 2023
1 parent 2d485d3 commit 3808e0c
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 14 deletions.
8 changes: 8 additions & 0 deletions velox/exec/RowContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,14 @@ void RowContainer::hash(
folly::Range<char**> rows,
bool mix,
uint64_t* result) {
if (typeKinds_[column] == TypeKind::UNKNOWN) {
for (auto i = 0; i < rows.size(); ++i) {
result[i] = mix ? bits::hashMix(result[i], BaseVector::kNullHash)
: BaseVector::kNullHash;
}
return;
}

bool nullable = column >= keyTypes_.size() || nullableKeys_;
VELOX_DYNAMIC_TYPE_DISPATCH(
hashTyped,
Expand Down
14 changes: 7 additions & 7 deletions velox/exec/RowContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -1338,18 +1338,18 @@ inline bool RowContainer::equals(
RowColumn column,
const DecodedVector& decoded,
vector_size_t index) {
auto typeKind = decoded.base()->typeKind();
if (typeKind == TypeKind::UNKNOWN) {
return isNullAt(row, column.nullByte(), column.nullMask());
}

if (!mayHaveNulls) {
return VELOX_DYNAMIC_TYPE_DISPATCH(
equalsNoNulls,
decoded.base()->typeKind(),
row,
column.offset(),
decoded,
index);
equalsNoNulls, typeKind, row, column.offset(), decoded, index);
} else {
return VELOX_DYNAMIC_TYPE_DISPATCH(
equalsWithNulls,
decoded.base()->typeKind(),
typeKind,
row,
column.offset(),
column.nullByte(),
Expand Down
54 changes: 47 additions & 7 deletions velox/exec/tests/RowContainerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,18 @@ class RowContainerTest : public exec::test::RowContainerTestBase {
EXPECT_EQ(usage, sum);
}

std::vector<char*> store(
RowContainer& rowContainer,
DecodedVector& decodedVector,
vector_size_t size) {
std::vector<char*> rows(size);
for (size_t row = 0; row < size; ++row) {
rows[row] = rowContainer.newRow();
rowContainer.store(decodedVector, row, rows[row], 0);
}
return rows;
}

// Stores the input vector in Row Container, extracts it and compares. Returns
// the container.
std::unique_ptr<RowContainer> roundTrip(const VectorPtr& input) {
Expand All @@ -265,13 +277,8 @@ class RowContainerTest : public exec::test::RowContainerTestBase {
// Store the vector in the rowContainer.
auto rowContainer = std::make_unique<RowContainer>(types, pool_.get());
auto size = input->size();
SelectivityVector allRows(size);
std::vector<char*> rows(size);
DecodedVector decoded(*input, allRows);
for (size_t row = 0; row < size; ++row) {
rows[row] = rowContainer->newRow();
rowContainer->store(decoded, row, rows[row], 0);
}
DecodedVector decoded(*input);
auto rows = store(*rowContainer, decoded, size);

testExtractColumn(*rowContainer, rows, 0, input);
return rowContainer;
Expand Down Expand Up @@ -1227,3 +1234,36 @@ TEST_F(RowContainerTest, mixedFree) {
data1->checkConsistency();
data2->checkConsistency();
}

TEST_F(RowContainerTest, unknown) {
std::vector<TypePtr> types = {UNKNOWN()};
auto rowContainer = std::make_unique<RowContainer>(types, pool_.get());

auto data = makeRowVector({
makeAllNullFlatVector<UnknownValue>(5),
});

auto size = data->size();
DecodedVector decoded(*data->childAt(0));
auto rows = store(*rowContainer, decoded, size);

std::vector<uint64_t> hashes(size, 0);
rowContainer->hash(
0, folly::Range(rows.data(), rows.size()), false /*mix*/, hashes.data());
for (auto hash : hashes) {
ASSERT_EQ(BaseVector::kNullHash, hash);
}

// Fill in hashes with sequential numbers: 0, 1, 2,..
std::iota(hashes.begin(), hashes.end(), 0);
rowContainer->hash(
0, folly::Range(rows.data(), rows.size()), true /*mix*/, hashes.data());
for (auto i = 0; i < size; ++i) {
ASSERT_EQ(bits::hashMix(i, BaseVector::kNullHash), hashes[i]);
}

for (size_t row = 0; row < size; ++row) {
ASSERT_TRUE(rowContainer->equals<false>(
rows[row], rowContainer->columnAt(0), decoded, row));
}
}

0 comments on commit 3808e0c

Please sign in to comment.