Skip to content

Commit

Permalink
adjust ErrorVectorPtr
Browse files Browse the repository at this point in the history
  • Loading branch information
duanmeng committed Mar 17, 2023
1 parent 9e573ab commit edc6ca2
Show file tree
Hide file tree
Showing 13 changed files with 28 additions and 27 deletions.
4 changes: 2 additions & 2 deletions velox/expression/CastExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ VectorPtr CastExpr::applyMap(
mapKeys->size(), rows, input, context.pool());
}

EvalCtx::ErrorVectorPtr oldErrors;
ErrorVectorPtr oldErrors;
context.swapErrors(oldErrors);

// Cast keys
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions velox/expression/ConjunctExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down
9 changes: 1 addition & 8 deletions velox/expression/EvalCtx.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>, this ends up being a
// double pointer in the form of std::shared_ptr<std::exception_ptr>. This is
// fine since we only need to actually follow the pointer in failure cases.
using ErrorVector = FlatVector<std::shared_ptr<void>>;
using ErrorVectorPtr = std::shared_ptr<ErrorVector>;

// Sets the error at 'index' in '*errorPtr' if the value is
// null. Creates and resizes '*errorPtr' as needed and initializes
// new positions to null.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/LambdaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class ExprCallable : public Callable {
const BufferPtr& wrapCapture,
EvalCtx* context,
const std::vector<VectorPtr>& args,
EvalCtx::ErrorVectorPtr& elementErrors,
ErrorVectorPtr& elementErrors,
VectorPtr* result) override {
auto row = createRowVector(context, wrapCapture, args, rows.end());
EvalCtx lambdaCtx = createLambdaCtx(context, row, finalSelection);
Expand Down
6 changes: 2 additions & 4 deletions velox/expression/TryExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<EvalCtx::ErrorVectorPtr> errorsSetter(
context.errorsPtr(), nullptr);
ScopedVarSetter<ErrorVectorPtr> errorsSetter(context.errorsPtr(), nullptr);
inputs_[0]->eval(rows, context, result);

nullOutErrors(rows, context, result);
Expand All @@ -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<EvalCtx::ErrorVectorPtr> errorsSetter(
context.errorsPtr(), nullptr);
ScopedVarSetter<ErrorVectorPtr> errorsSetter(context.errorsPtr(), nullptr);
inputs_[0]->evalSimplified(rows, context, result);

nullOutErrors(rows, context, result);
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/tests/EvalCtxTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/tests/ExprStatsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_++;

Expand Down
3 changes: 1 addition & 2 deletions velox/expression/tests/ExpressionFuzzer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, ExprUsageStats>& exprNameToStats_;
Expand Down
4 changes: 2 additions & 2 deletions velox/functions/prestosql/ArrayAllMatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class AllMatchFunction : public exec::VectorFunction {
auto it = args[1]->asUnchecked<FunctionVector>()->iterator(&rows);

while (auto entry = it.next()) {
exec::EvalCtx::ErrorVectorPtr elementErrors;
ErrorVectorPtr elementErrors;
auto elementRows =
toElementRows<ArrayVector>(numElements, *entry.rows, flatArray.get());
auto wrapCapture = toWrapCapture<ArrayVector>(
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions velox/functions/prestosql/types/JsonType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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(
Expand Down
7 changes: 7 additions & 0 deletions velox/vector/FlatVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,13 @@ void FlatVector<StringView>::prepareForReuse();
template <typename T>
using FlatVectorPtr = std::shared_ptr<FlatVector<T>>;

// Error vector uses an opaque flat vector to store std::exception_ptr.
// Since opaque types are stored as shared_ptr<void>, this ends up being a
// double pointer in the form of std::shared_ptr<std::exception_ptr>. This is
// fine since we only need to actually follow the pointer in failure cases.
using ErrorVector = FlatVector<std::shared_ptr<void>>;
using ErrorVectorPtr = std::shared_ptr<ErrorVector>;

} // namespace facebook::velox

#include "velox/vector/FlatVector-inl.h"
6 changes: 5 additions & 1 deletion velox/vector/FunctionVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

namespace facebook::velox {

namespace exec {
class EvalCtx;
} // namespace exec

// Represents a function with possible captures.
class Callable {
public:
Expand Down Expand Up @@ -61,7 +65,7 @@ class Callable {
const BufferPtr& wrapCapture,
exec::EvalCtx* context,
const std::vector<VectorPtr>& args,
exec::EvalCtx::ErrorVectorPtr& elementErrors,
ErrorVectorPtr& elementErrors,
VectorPtr* result) = 0;
};

Expand Down

0 comments on commit edc6ca2

Please sign in to comment.