Skip to content

Commit f315052

Browse files
marin-mafacebook-github-bot
authored andcommitted
Fix BaseVector::flattenVector for Lazy input (facebookincubator#9282)
Summary: The current implementation in `flattenVector` doesn't modify the loaded vector in `LazyVector`. Because the `loadedVector` is not a reference, the flattening doesn't take effect. https://github.com/facebookincubator/velox/blob/0c86a0aedb90bfa966087def7551a40fc3571a01/velox/vector/BaseVector.cpp#L866-L871 Fixes facebookincubator#8697 Pull Request resolved: facebookincubator#9282 Reviewed By: kgpai Differential Revision: D55597691 Pulled By: bikramSingh91 fbshipit-source-id: 2d1f5f25bc986479f58c7a1b5555ea57d6b0be3b
1 parent 1146fe2 commit f315052

File tree

4 files changed

+39
-23
lines changed

4 files changed

+39
-23
lines changed

velox/vector/BaseVector.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ void BaseVector::flattenVector(VectorPtr& vector) {
864864
return;
865865
}
866866
case VectorEncoding::Simple::LAZY: {
867-
auto loadedVector =
867+
auto& loadedVector =
868868
vector->asUnchecked<LazyVector>()->loadedVectorShared();
869869
BaseVector::flattenVector(loadedVector);
870870
return;

velox/vector/LazyVector.cpp

+24
Original file line numberDiff line numberDiff line change
@@ -271,4 +271,28 @@ void LazyVector::load(RowSet rows, ValueHook* hook) const {
271271
}
272272
}
273273

274+
void LazyVector::loadVectorInternal() const {
275+
if (!allLoaded_) {
276+
if (!vector_) {
277+
vector_ = BaseVector::create(type_, 0, pool_);
278+
}
279+
SelectivityVector allRows(BaseVector::length_);
280+
loader_->load(allRows, nullptr, size(), &vector_);
281+
VELOX_CHECK(vector_);
282+
if (vector_->encoding() == VectorEncoding::Simple::LAZY) {
283+
vector_ = vector_->asUnchecked<LazyVector>()->loadedVectorShared();
284+
} else {
285+
// If the load produced a wrapper, load the wrapped vector.
286+
vector_->loadedVector();
287+
}
288+
allLoaded_ = true;
289+
const_cast<LazyVector*>(this)->BaseVector::nulls_ = vector_->nulls_;
290+
if (BaseVector::nulls_) {
291+
const_cast<LazyVector*>(this)->BaseVector::rawNulls_ =
292+
BaseVector::nulls_->as<uint64_t>();
293+
}
294+
} else {
295+
VELOX_CHECK(vector_);
296+
}
297+
}
274298
} // namespace facebook::velox

velox/vector/LazyVector.h

+8-22
Original file line numberDiff line numberDiff line change
@@ -188,29 +188,13 @@ class LazyVector : public BaseVector {
188188
// Returns a shared_ptr to the vector holding the values. If vector is not
189189
// loaded, loads all the rows, otherwise returns the loaded vector which can
190190
// have partially loaded rows.
191+
VectorPtr& loadedVectorShared() {
192+
loadVectorInternal();
193+
return vector_;
194+
}
195+
191196
const VectorPtr& loadedVectorShared() const {
192-
if (!allLoaded_) {
193-
if (!vector_) {
194-
vector_ = BaseVector::create(type_, 0, pool_);
195-
}
196-
SelectivityVector allRows(BaseVector::length_);
197-
loader_->load(allRows, nullptr, size(), &vector_);
198-
VELOX_CHECK(vector_);
199-
if (vector_->encoding() == VectorEncoding::Simple::LAZY) {
200-
vector_ = vector_->asUnchecked<LazyVector>()->loadedVectorShared();
201-
} else {
202-
// If the load produced a wrapper, load the wrapped vector.
203-
vector_->loadedVector();
204-
}
205-
allLoaded_ = true;
206-
const_cast<LazyVector*>(this)->BaseVector::nulls_ = vector_->nulls_;
207-
if (BaseVector::nulls_) {
208-
const_cast<LazyVector*>(this)->BaseVector::rawNulls_ =
209-
BaseVector::nulls_->as<uint64_t>();
210-
}
211-
} else {
212-
VELOX_CHECK(vector_);
213-
}
197+
loadVectorInternal();
214198
return vector_;
215199
}
216200

@@ -287,6 +271,8 @@ class LazyVector : public BaseVector {
287271
const SelectivityVector& rows,
288272
SelectivityVector& baseRows);
289273

274+
void loadVectorInternal() const;
275+
290276
std::unique_ptr<VectorLoader> loader_;
291277

292278
// True if all values are loaded.

velox/vector/tests/VectorTest.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -2754,6 +2754,12 @@ TEST_F(VectorTest, flattenVector) {
27542754
test(dictionary, false);
27552755
EXPECT_TRUE(dictionary->isFlatEncoding());
27562756

2757+
VectorPtr lazyDictionary =
2758+
wrapInLazyDictionary(makeFlatVector<int32_t>({1, 2, 3}));
2759+
test(lazyDictionary, true);
2760+
EXPECT_TRUE(lazyDictionary->isLazy());
2761+
EXPECT_TRUE(lazyDictionary->loadedVector()->isFlatEncoding());
2762+
27572763
// Array with constant elements.
27582764
auto* arrayVector = array->as<ArrayVector>();
27592765
arrayVector->elements() = BaseVector::wrapInConstant(100, 1, flat);

0 commit comments

Comments
 (0)