From 5a5ee0bdbb08c5e37a5966df0fcd8c84a1dad82d Mon Sep 17 00:00:00 2001 From: zhejiangxiaomai Date: Sun, 23 Apr 2023 15:05:31 +0800 Subject: [PATCH] Revert "FIX: use rows.end() instead of rows.size() in vector operations and expression eval. (#4458)" This reverts commit dd529b50f753910acaed387f92c8ea5063bf14b8. --- velox/expression/Expr.cpp | 10 +++++----- velox/expression/FieldReference.cpp | 6 +++--- velox/vector/BaseVector.cpp | 8 ++++---- velox/vector/ComplexVector.cpp | 7 ++++--- velox/vector/FlatVector-inl.h | 2 +- velox/vector/tests/EnsureWritableVectorTest.cpp | 13 +++++++------ 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/velox/expression/Expr.cpp b/velox/expression/Expr.cpp index 912b1d293be1..9d24d24a5686 100644 --- a/velox/expression/Expr.cpp +++ b/velox/expression/Expr.cpp @@ -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; } @@ -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; } } @@ -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]); } @@ -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; @@ -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 { diff --git a/velox/expression/FieldReference.cpp b/velox/expression/FieldReference.cpp index 0272b1cae4cf..ac12d7a10b50 100644 --- a/velox/expression/FieldReference.cpp +++ b/velox/expression/FieldReference.cpp @@ -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())) diff --git a/velox/vector/BaseVector.cpp b/velox/vector/BaseVector.cpp index af8cb1274377..ed40b4ce4028 100644 --- a/velox/vector/BaseVector.cpp +++ b/velox/vector/BaseVector.cpp @@ -489,7 +489,7 @@ std::string BaseVector::toString( } void BaseVector::ensureWritable(const SelectivityVector& rows) { - auto newSize = std::max(rows.end(), length_); + auto newSize = std::max(rows.size(), length_); if (nulls_ && !(nulls_->unique() && nulls_->isMutable())) { BufferPtr newNulls = AlignedBuffer::allocate(newSize, pool_); auto rawNewNulls = newNulls->asMutable(); @@ -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; } @@ -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(rows.end(), result->size()); + auto targetSize = std::max(rows.size(), result->size()); VectorPtr copy; if (vectorPool) { diff --git a/velox/vector/ComplexVector.cpp b/velox/vector/ComplexVector.cpp index ea51903563ea..b87e39c07080 100644 --- a/velox/vector/ComplexVector.cpp +++ b/velox/vector/ComplexVector.cpp @@ -209,7 +209,8 @@ void RowVector::copy( BufferPtr mappedIndices; vector_size_t* rawMappedIndices = nullptr; if (toSourceRow) { - mappedIndices = AlignedBuffer::allocate(rows.end(), pool_); + mappedIndices = + AlignedBuffer::allocate(rows.size(), pool_); rawMappedIndices = mappedIndices->asMutable(); nonNullRows.applyToSelected( [&](auto row) { rawMappedIndices[row] = indices[toSourceRow[row]]; }); @@ -687,7 +688,7 @@ std::string ArrayVector::toString(vector_size_t index) const { } void ArrayVector::ensureWritable(const SelectivityVector& rows) { - auto newSize = std::max(rows.end(), BaseVector::length_); + auto newSize = std::max(rows.size(), BaseVector::length_); if (offsets_ && !offsets_->unique()) { BufferPtr newOffsets = AlignedBuffer::allocate(newSize, BaseVector::pool_); @@ -949,7 +950,7 @@ std::string MapVector::toString(vector_size_t index) const { } void MapVector::ensureWritable(const SelectivityVector& rows) { - auto newSize = std::max(rows.end(), BaseVector::length_); + auto newSize = std::max(rows.size(), BaseVector::length_); if (offsets_ && !offsets_->unique()) { BufferPtr newOffsets = AlignedBuffer::allocate(newSize, BaseVector::pool_); diff --git a/velox/vector/FlatVector-inl.h b/velox/vector/FlatVector-inl.h index 6762c641ed92..4eebad6b23e4 100644 --- a/velox/vector/FlatVector-inl.h +++ b/velox/vector/FlatVector-inl.h @@ -349,7 +349,7 @@ void FlatVector::resize(vector_size_t newSize, bool setNotNull) { template void FlatVector::ensureWritable(const SelectivityVector& rows) { - auto newSize = std::max(rows.end(), BaseVector::length_); + auto newSize = std::max(rows.size(), BaseVector::length_); if (values_ && !(values_->unique() && values_->isMutable())) { BufferPtr newValues; if constexpr (std::is_same_v) { diff --git a/velox/vector/tests/EnsureWritableVectorTest.cpp b/velox/vector/tests/EnsureWritableVectorTest.cpp index 670c82323cd0..33783da85482 100644 --- a/velox/vector/tests/EnsureWritableVectorTest.cpp +++ b/velox/vector/tests/EnsureWritableVectorTest.cpp @@ -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(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