diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp index ea616d8f58d9..cec4df1bef5e 100644 --- a/velox/expression/CastExpr.cpp +++ b/velox/expression/CastExpr.cpp @@ -300,7 +300,7 @@ VectorPtr CastExpr::applyMap( mapKeys->size(), rows, input, context.pool()); } - EvalCtx::ErrorVectorPtr oldErrors; + ErrorVectorPtr oldErrors; context.swapErrors(oldErrors); // Cast keys @@ -362,7 +362,7 @@ VectorPtr CastExpr::applyArray( auto elementToTopLevelRows = functions::getElementToTopLevelRows( arrayElements->size(), rows, input, context.pool()); - EvalCtx::ErrorVectorPtr oldErrors; + ErrorVectorPtr oldErrors; context.swapErrors(oldErrors); VectorPtr newElements; diff --git a/velox/expression/ConjunctExpr.cpp b/velox/expression/ConjunctExpr.cpp index 20e2e45f5e1d..e8dcadc33a25 100644 --- a/velox/expression/ConjunctExpr.cpp +++ b/velox/expression/ConjunctExpr.cpp @@ -25,7 +25,7 @@ uint64_t* rowsWithError( const SelectivityVector& rows, const SelectivityVector& activeRows, EvalCtx& context, - EvalCtx::ErrorVectorPtr& previousErrors, + ErrorVectorPtr& previousErrors, LocalSelectivityVector& errorRowsHolder) { auto errors = context.errors(); if (!errors) { @@ -126,7 +126,7 @@ void ConjunctExpr::evalSpecialForm( for (int32_t i = 0; i < inputs_.size(); ++i) { VectorPtr inputResult; VectorRecycler inputResultRecycler(inputResult, context.vectorPool()); - EvalCtx::ErrorVectorPtr errors; + ErrorVectorPtr errors; if (handleErrors) { context.swapErrors(errors); } diff --git a/velox/expression/EvalCtx.h b/velox/expression/EvalCtx.h index 4164ad36ae06..bd25ebcc63f5 100644 --- a/velox/expression/EvalCtx.h +++ b/velox/expression/EvalCtx.h @@ -101,13 +101,6 @@ class EvalCtx { }); } - // Error vector uses an opaque flat vector to store std::exception_ptr. - // Since opaque types are stored as shared_ptr, this ends up being a - // double pointer in the form of std::shared_ptr. This is - // fine since we only need to actually follow the pointer in failure cases. - using ErrorVector = FlatVector>; - using ErrorVectorPtr = std::shared_ptr; - // Sets the error at 'index' in '*errorPtr' if the value is // null. Creates and resizes '*errorPtr' as needed and initializes // new positions to null. @@ -320,7 +313,7 @@ struct ScopedContextSaver { // The selection of the context being saved. const SelectivityVector* FOLLY_NONNULL rows; const SelectivityVector* FOLLY_NULLABLE finalSelection; - EvalCtx::ErrorVectorPtr errors; + ErrorVectorPtr errors; }; /// Produces a SelectivityVector with a single row selected using a pool of diff --git a/velox/expression/Expr.h b/velox/expression/Expr.h index 1d7e486f9723..53b0757c0a9b 100644 --- a/velox/expression/Expr.h +++ b/velox/expression/Expr.h @@ -649,7 +649,7 @@ class ExprSetListener { /// @param errors Error vector produced inside the try expression. virtual void onError( const SelectivityVector& rows, - const EvalCtx::ErrorVector& errors) = 0; + const ErrorVector& errors) = 0; }; /// Return the ExprSetListeners having been registered. diff --git a/velox/expression/LambdaExpr.cpp b/velox/expression/LambdaExpr.cpp index 2b9358dc02c7..1a88f59972fe 100644 --- a/velox/expression/LambdaExpr.cpp +++ b/velox/expression/LambdaExpr.cpp @@ -63,7 +63,7 @@ class ExprCallable : public Callable { const BufferPtr& wrapCapture, EvalCtx* context, const std::vector& args, - EvalCtx::ErrorVectorPtr& elementErrors, + ErrorVectorPtr& elementErrors, VectorPtr* result) override { auto row = createRowVector(context, wrapCapture, args, rows.end()); EvalCtx lambdaCtx = createLambdaCtx(context, row, finalSelection); diff --git a/velox/expression/TryExpr.cpp b/velox/expression/TryExpr.cpp index f187c56af7f1..a6662054d882 100644 --- a/velox/expression/TryExpr.cpp +++ b/velox/expression/TryExpr.cpp @@ -31,8 +31,7 @@ void TryExpr::evalSpecialForm( // This also prevents this TRY expression from leaking exceptions to the // parent TRY expression, so the parent won't incorrectly null out rows that // threw exceptions which this expression already handled. - ScopedVarSetter errorsSetter( - context.errorsPtr(), nullptr); + ScopedVarSetter errorsSetter(context.errorsPtr(), nullptr); inputs_[0]->eval(rows, context, result); nullOutErrors(rows, context, result); @@ -51,8 +50,7 @@ void TryExpr::evalSpecialFormSimplified( // This also prevents this TRY expression from leaking exceptions to the // parent TRY expression, so the parent won't incorrectly null out rows that // threw exceptions which this expression already handled. - ScopedVarSetter errorsSetter( - context.errorsPtr(), nullptr); + ScopedVarSetter errorsSetter(context.errorsPtr(), nullptr); inputs_[0]->evalSimplified(rows, context, result); nullOutErrors(rows, context, result); diff --git a/velox/expression/tests/EvalCtxTest.cpp b/velox/expression/tests/EvalCtxTest.cpp index d2c0cf748170..d641da1b163e 100644 --- a/velox/expression/tests/EvalCtxTest.cpp +++ b/velox/expression/tests/EvalCtxTest.cpp @@ -161,7 +161,7 @@ TEST_F(EvalCtxTest, addErrorsPreserveOldErrors) { ASSERT_EQ(BaseVector::countNulls(context.errors()->nulls(), 4), 2); // Add two out_of_range to anotherErrors. - EvalCtx::ErrorVectorPtr anotherErrors; + ErrorVectorPtr anotherErrors; std::out_of_range rangeError{"out of range"}; context.addError(0, std::make_exception_ptr(rangeError), anotherErrors); context.addError(4, std::make_exception_ptr(rangeError), anotherErrors); diff --git a/velox/expression/tests/ExprStatsTest.cpp b/velox/expression/tests/ExprStatsTest.cpp index 00ebf186489a..51c79a9c1abb 100644 --- a/velox/expression/tests/ExprStatsTest.cpp +++ b/velox/expression/tests/ExprStatsTest.cpp @@ -174,7 +174,7 @@ class TestListener : public exec::ExprSetListener { void onError( const SelectivityVector& rows, - const ::facebook::velox::exec::EvalCtx::ErrorVector& errors) override { + const ::facebook::velox::ErrorVector& errors) override { rows.applyToSelected([&](auto row) { exceptionCount_++; diff --git a/velox/expression/tests/ExpressionFuzzer.h b/velox/expression/tests/ExpressionFuzzer.h index bda1fb9b943d..875152c63c7f 100644 --- a/velox/expression/tests/ExpressionFuzzer.h +++ b/velox/expression/tests/ExpressionFuzzer.h @@ -132,8 +132,7 @@ class ExpressionFuzzer { // occurred. void onError( const SelectivityVector& /*rows*/, - const ::facebook::velox::exec::EvalCtx::ErrorVector& /*errors*/) - override {} + const ::facebook::velox::ErrorVector& /*errors*/) override {} private: std::unordered_map& exprNameToStats_; diff --git a/velox/functions/prestosql/ArrayAllMatch.cpp b/velox/functions/prestosql/ArrayAllMatch.cpp index 392e5c8ff480..397e8db2bc0d 100644 --- a/velox/functions/prestosql/ArrayAllMatch.cpp +++ b/velox/functions/prestosql/ArrayAllMatch.cpp @@ -60,7 +60,7 @@ class AllMatchFunction : public exec::VectorFunction { auto it = args[1]->asUnchecked()->iterator(&rows); while (auto entry = it.next()) { - exec::EvalCtx::ErrorVectorPtr elementErrors; + ErrorVectorPtr elementErrors; auto elementRows = toElementRows(numElements, *entry.rows, flatArray.get()); auto wrapCapture = toWrapCapture( @@ -125,7 +125,7 @@ class AllMatchFunction : public exec::VectorFunction { private: FOLLY_ALWAYS_INLINE bool hasError( - const exec::EvalCtx::ErrorVectorPtr& elementErrors, + const ErrorVectorPtr& elementErrors, int idx) const { return elementErrors && idx < elementErrors->size() && !elementErrors->isNullAt(idx); diff --git a/velox/functions/prestosql/types/JsonType.cpp b/velox/functions/prestosql/types/JsonType.cpp index 65227a7b9ce0..6b10cae1d4bc 100644 --- a/velox/functions/prestosql/types/JsonType.cpp +++ b/velox/functions/prestosql/types/JsonType.cpp @@ -169,7 +169,7 @@ struct AsJson { const BufferPtr& elementToTopLevelRows, bool isMapKey = false) : decoded_(context, *input, rows) { - exec::EvalCtx::ErrorVectorPtr oldErrors; + ErrorVectorPtr oldErrors; context.swapErrors(oldErrors); if (isMapKey && decoded_->mayHaveNulls()) { @@ -272,7 +272,7 @@ struct AsJson { exec::EvalCtx& context, const SelectivityVector& rows, const BufferPtr& elementToTopLevelRows, - exec::EvalCtx::ErrorVectorPtr& oldErrors) { + ErrorVectorPtr& oldErrors) { if (context.errors()) { if (elementToTopLevelRows) { context.addElementErrorsToTopLevel( diff --git a/velox/vector/FlatVector.h b/velox/vector/FlatVector.h index 75335119fe42..0ea0475c9f8f 100644 --- a/velox/vector/FlatVector.h +++ b/velox/vector/FlatVector.h @@ -509,6 +509,13 @@ void FlatVector::prepareForReuse(); template using FlatVectorPtr = std::shared_ptr>; +// Error vector uses an opaque flat vector to store std::exception_ptr. +// Since opaque types are stored as shared_ptr, this ends up being a +// double pointer in the form of std::shared_ptr. This is +// fine since we only need to actually follow the pointer in failure cases. +using ErrorVector = FlatVector>; +using ErrorVectorPtr = std::shared_ptr; + } // namespace facebook::velox #include "velox/vector/FlatVector-inl.h" diff --git a/velox/vector/FunctionVector.h b/velox/vector/FunctionVector.h index d098237704f5..59a05f95c475 100644 --- a/velox/vector/FunctionVector.h +++ b/velox/vector/FunctionVector.h @@ -20,6 +20,10 @@ namespace facebook::velox { +namespace exec { +class EvalCtx; +} // namespace exec + // Represents a function with possible captures. class Callable { public: @@ -61,7 +65,7 @@ class Callable { const BufferPtr& wrapCapture, exec::EvalCtx* context, const std::vector& args, - exec::EvalCtx::ErrorVectorPtr& elementErrors, + ErrorVectorPtr& elementErrors, VectorPtr* result) = 0; };