Skip to content

Commit bc07966

Browse files
xiaoxmengfacebook-github-bot
authored andcommitted
fix: Add BaseVector::reusable() to recursively check vector reusability (facebookincubator#502)
Summary: X-link: facebookincubator/velox#16471 This change adds a new static method BaseVector::reusable() that recursively checks if a vector and all its nested children are reusable (have use_count == 1). The existing verifyVectorState() check in nimble FieldReader only verified the top-level vector's use_count, but complex types like ARRAY<ROW<BIGINT, REAL>> can have nested children that are shared across different parent vectors. When prepareForReuse() is called on a vector whose nested children are still referenced elsewhere, it corrupts those shared children by resetting them to size 0. Adds BaseVector::reusable() which recursively checks use_count for ROW children, ARRAY elements, and MAP keys/values Updates verifyVectorState() in nimble FieldReader to use the new recursive check Adds unit tests for the new method Reviewed By: Yuhta Differential Revision: D93790972
1 parent 74f4fbc commit bc07966

File tree

2 files changed

+70
-67
lines changed

2 files changed

+70
-67
lines changed

dwio/nimble/velox/FieldReader.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,15 @@ void zeroNulls(velox::BaseVector* vector, uint32_t rowCount) {
5555
memset(ensureNulls(vector, rowCount), 0, nullBytes(rowCount));
5656
}
5757

58+
// TODO: consider to use prepareForReuse in velox.
5859
template <typename T>
5960
T* verifyVectorState(velox::VectorPtr& vector) {
60-
// we want vector to not be referenced by anyone else (e.g. ref count of 1)
61+
// we want vector AND all its nested children to not be referenced by anyone
62+
// else (e.g. ref count of 1 recursively). Use BaseVector::reusable
63+
// from Velox to check this.
6164
if (vector != nullptr) {
6265
auto* casted = vector->as<T>();
63-
if ((casted != nullptr) && (vector.use_count() == 1)) {
66+
if ((casted != nullptr) && velox::BaseVector::recursivelyReusable(vector)) {
6467
return casted;
6568
}
6669
vector.reset();

0 commit comments

Comments
 (0)