Skip to content

Commit

Permalink
Revert "FIX: use rows.end() instead of rows.size() in vector operatio…
Browse files Browse the repository at this point in the history
…ns and expression eval. (facebookincubator#4458)"

This reverts commit dd529b5.
  • Loading branch information
zhejiangxiaomai committed Apr 23, 2023
1 parent ba4a625 commit 5a5ee0b
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 22 deletions.
10 changes: 5 additions & 5 deletions velox/expression/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ void Expr::evalSimplifiedImpl(
if (!remainingRows.hasSelections()) {
releaseInputValues(context);
result =
BaseVector::createNullConstant(type(), rows.end(), context.pool());
BaseVector::createNullConstant(type(), rows.size(), context.pool());
return;
}

Expand All @@ -338,7 +338,7 @@ void Expr::evalSimplifiedImpl(
if (!remainingRows.hasSelections()) {
releaseInputValues(context);
result = BaseVector::createNullConstant(
type(), rows.end(), context.pool());
type(), rows.size(), context.pool());
return;
}
}
Expand Down Expand Up @@ -535,7 +535,7 @@ void Expr::evalFlatNoNullsImpl(
// No need to re-evaluate constant expression. Simply move constant values
// from constantInputs_.
inputValues_[i] = std::move(constantInputs_[i]);
inputValues_[i]->resize(rows.end());
inputValues_[i]->resize(rows.size());
} else {
inputs_[i]->evalFlatNoNulls(rows, context, inputValues_[i]);
}
Expand Down Expand Up @@ -643,7 +643,7 @@ void Expr::evaluateSharedSubexpr(
eval(rows, context, result);

if (!sharedSubexprRows) {
sharedSubexprRows = context.execCtx()->getSelectivityVector(rows.end());
sharedSubexprRows = context.execCtx()->getSelectivityVector(rows.size());
}

*sharedSubexprRows = rows;
Expand Down Expand Up @@ -1035,7 +1035,7 @@ void Expr::setAllNulls(
result->addNulls(notNulls.get()->asRange().bits(), rows);
return;
}
result = BaseVector::createNullConstant(type(), rows.end(), context.pool());
result = BaseVector::createNullConstant(type(), rows.size(), context.pool());
}

namespace {
Expand Down
6 changes: 3 additions & 3 deletions velox/expression/FieldReference.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ void FieldReference::evalSpecialForm(
}
// The caller relies on vectors having a meaningful size. If we
// have a constant that is not wrapped in anything we set its size
// to correspond to rows.end(). This is in place for unique ones
// to correspond to rows.size(). This is in place for unique ones
// and a copy otherwise.
if (!useDecode && child->isConstantEncoding()) {
if (isUniqueChild) {
child->resize(rows.end());
child->resize(rows.size());
} else {
child = BaseVector::wrapInConstant(rows.end(), 0, child);
child = BaseVector::wrapInConstant(rows.size(), 0, child);
}
}
result = useDecode ? std::move(decoded.wrap(child, *input, rows.end()))
Expand Down
8 changes: 4 additions & 4 deletions velox/vector/BaseVector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ std::string BaseVector::toString(
}

void BaseVector::ensureWritable(const SelectivityVector& rows) {
auto newSize = std::max<vector_size_t>(rows.end(), length_);
auto newSize = std::max<vector_size_t>(rows.size(), length_);
if (nulls_ && !(nulls_->unique() && nulls_->isMutable())) {
BufferPtr newNulls = AlignedBuffer::allocate<bool>(newSize, pool_);
auto rawNewNulls = newNulls->asMutable<uint64_t>();
Expand All @@ -511,9 +511,9 @@ void BaseVector::ensureWritable(
VectorPool* vectorPool) {
if (!result) {
if (vectorPool) {
result = vectorPool->get(type, rows.end());
result = vectorPool->get(type, rows.size());
} else {
result = BaseVector::create(type, rows.end(), pool);
result = BaseVector::create(type, rows.size(), pool);
}
return;
}
Expand Down Expand Up @@ -542,7 +542,7 @@ void BaseVector::ensureWritable(

// The copy-on-write size is the max of the writable row set and the
// vector.
auto targetSize = std::max<vector_size_t>(rows.end(), result->size());
auto targetSize = std::max<vector_size_t>(rows.size(), result->size());

VectorPtr copy;
if (vectorPool) {
Expand Down
7 changes: 4 additions & 3 deletions velox/vector/ComplexVector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ void RowVector::copy(
BufferPtr mappedIndices;
vector_size_t* rawMappedIndices = nullptr;
if (toSourceRow) {
mappedIndices = AlignedBuffer::allocate<vector_size_t>(rows.end(), pool_);
mappedIndices =
AlignedBuffer::allocate<vector_size_t>(rows.size(), pool_);
rawMappedIndices = mappedIndices->asMutable<vector_size_t>();
nonNullRows.applyToSelected(
[&](auto row) { rawMappedIndices[row] = indices[toSourceRow[row]]; });
Expand Down Expand Up @@ -687,7 +688,7 @@ std::string ArrayVector::toString(vector_size_t index) const {
}

void ArrayVector::ensureWritable(const SelectivityVector& rows) {
auto newSize = std::max<vector_size_t>(rows.end(), BaseVector::length_);
auto newSize = std::max<vector_size_t>(rows.size(), BaseVector::length_);
if (offsets_ && !offsets_->unique()) {
BufferPtr newOffsets =
AlignedBuffer::allocate<vector_size_t>(newSize, BaseVector::pool_);
Expand Down Expand Up @@ -949,7 +950,7 @@ std::string MapVector::toString(vector_size_t index) const {
}

void MapVector::ensureWritable(const SelectivityVector& rows) {
auto newSize = std::max<vector_size_t>(rows.end(), BaseVector::length_);
auto newSize = std::max<vector_size_t>(rows.size(), BaseVector::length_);
if (offsets_ && !offsets_->unique()) {
BufferPtr newOffsets =
AlignedBuffer::allocate<vector_size_t>(newSize, BaseVector::pool_);
Expand Down
2 changes: 1 addition & 1 deletion velox/vector/FlatVector-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ void FlatVector<T>::resize(vector_size_t newSize, bool setNotNull) {

template <typename T>
void FlatVector<T>::ensureWritable(const SelectivityVector& rows) {
auto newSize = std::max<vector_size_t>(rows.end(), BaseVector::length_);
auto newSize = std::max<vector_size_t>(rows.size(), BaseVector::length_);
if (values_ && !(values_->unique() && values_->isMutable())) {
BufferPtr newValues;
if constexpr (std::is_same_v<T, StringView>) {
Expand Down
13 changes: 7 additions & 6 deletions velox/vector/tests/EnsureWritableVectorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,17 +421,18 @@ TEST_F(EnsureWritableVectorTest, constant) {
}

// If constant has smaller size, check that we follow the selectivity vector
// max seleced row size.
// size.
{
const vector_size_t selectivityVectorSize = 100;
auto constant = BaseVector::createConstant(
BIGINT(), variant::create<TypeKind::BIGINT>(123), 1, pool_.get());
SelectivityVector rows(selectivityVectorSize);
rows.setValid(99, false);
rows.updateBounds();
BaseVector::ensureWritable(rows, BIGINT(), pool_.get(), constant);
BaseVector::ensureWritable(
SelectivityVector::empty(selectivityVectorSize),
BIGINT(),
pool_.get(),
constant);
EXPECT_EQ(VectorEncoding::Simple::FLAT, constant->encoding());
EXPECT_EQ(99, constant->size());
EXPECT_EQ(selectivityVectorSize, constant->size());
}

// If constant has larger size, check that we follow the constant vector
Expand Down

0 comments on commit 5a5ee0b

Please sign in to comment.