Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
zhli1142015 committed Sep 22, 2024
1 parent 385b8e7 commit c3e25e7
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 15 deletions.
4 changes: 1 addition & 3 deletions velox/buffer/Buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ BufferPtr Buffer::slice<bool>(
size_t offset,
size_t length,
memory::MemoryPool* pool) {
if (!buffer) {
return nullptr;
}
VELOX_CHECK_NOT_NULL(buffer, "Buffer must not be null.");

if (offset % 8 == 0) {
return sliceBufferZeroCopy(
Expand Down
8 changes: 3 additions & 5 deletions velox/buffer/Buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ class Buffer {
/// Otherwise return a BufferView into the original buffer (with shared
/// ownership of original buffer).
///
/// @param buffer A pointer to the buffer to be sliced. If this is null, the
/// function returns a null pointer.
/// @param buffer A pointer to the buffer to be sliced. Must not be null.
/// @param offset The element position in the buffer where the slice begins.
/// Must be less or equal than the buffer size.
/// @param length The number of elements to include in the slice. Must be
Expand All @@ -195,9 +194,7 @@ class Buffer {
size_t offset,
size_t length,
memory::MemoryPool* pool) {
if (!buffer) {
return nullptr;
}
VELOX_CHECK_NOT_NULL(buffer, "Buffer must not be null.");
return sliceBufferZeroCopy(
sizeof(T), is_pod_like_v<T>, buffer, offset, length);
}
Expand Down Expand Up @@ -698,5 +695,6 @@ class BufferView : public Buffer {

Releaser const releaser_;
};

} // namespace velox
} // namespace facebook
3 changes: 3 additions & 0 deletions velox/buffer/tests/BufferTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,9 @@ TEST_F(BufferTest, sliceBigintBuffer) {
VELOX_ASSERT_THROW(
Buffer::slice<int64_t>(bufferPtr, 5, 6, pool_.get()),
"Length must be less than or equal to 5.");
VELOX_ASSERT_THROW(
Buffer::slice<int64_t>(nullptr, 5, 6, pool_.get()),
"Buffer must not be null.");
}

TEST_F(BufferTest, sliceBooleanBuffer) {
Expand Down
2 changes: 1 addition & 1 deletion velox/vector/BaseVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ class BaseVector {
}

BufferPtr sliceNulls(vector_size_t offset, vector_size_t length) const {
return Buffer::slice<bool>(nulls_, offset, length, pool_);
return nulls_ ? Buffer::slice<bool>(nulls_, offset, length, pool_) : nulls_;
}

TypePtr type_;
Expand Down
12 changes: 8 additions & 4 deletions velox/vector/ComplexVector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1186,8 +1186,10 @@ VectorPtr ArrayVector::slice(vector_size_t offset, vector_size_t length) const {
type_,
sliceNulls(offset, length),
length,
Buffer::slice<vector_size_t>(offsets_, offset, length, pool_),
Buffer::slice<vector_size_t>(sizes_, offset, length, pool_),
offsets_ ? Buffer::slice<vector_size_t>(offsets_, offset, length, pool_)
: offsets_,
sizes_ ? Buffer::slice<vector_size_t>(sizes_, offset, length, pool_)
: sizes_,
elements_);
}

Expand Down Expand Up @@ -1485,8 +1487,10 @@ VectorPtr MapVector::slice(vector_size_t offset, vector_size_t length) const {
type_,
sliceNulls(offset, length),
length,
Buffer::slice<vector_size_t>(offsets_, offset, length, pool_),
Buffer::slice<vector_size_t>(sizes_, offset, length, pool_),
offsets_ ? Buffer::slice<vector_size_t>(offsets_, offset, length, pool_)
: offsets_,
sizes_ ? Buffer::slice<vector_size_t>(sizes_, offset, length, pool_)
: sizes_,
keys_,
values_);
}
Expand Down
4 changes: 3 additions & 1 deletion velox/vector/DictionaryVector-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,9 @@ VectorPtr DictionaryVector<T>::slice(vector_size_t offset, vector_size_t length)
this->sliceNulls(offset, length),
length,
valueVector(),
Buffer::slice<vector_size_t>(indices_, offset, length, this->pool_));
indices_
? Buffer::slice<vector_size_t>(indices_, offset, length, this->pool_)
: indices_);
}

template <typename T>
Expand Down
3 changes: 2 additions & 1 deletion velox/vector/FlatVector-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ VectorPtr FlatVector<T>::slice(vector_size_t offset, vector_size_t length)
this->type_,
this->sliceNulls(offset, length),
length,
Buffer::slice<T>(values_, offset, length, this->pool_),
values_ ? Buffer::slice<T>(values_, offset, length, this->pool_)
: values_,
std::vector<BufferPtr>(stringBuffers_));
}

Expand Down

0 comments on commit c3e25e7

Please sign in to comment.