resolve comments

velox/docs/functions/presto/array.rst

@@ -9,7 +9,7 @@ Array Functions
         Returns true if all the elements match the predicate (a special case is when the array is empty);
         Returns false if one or more elements don’t match;
         Returns NULL if the predicate function returns NULL for one or more elements and true for all other elements.
-        Throw exceptions if the predicate function has errors on some elements and returns true or NULL on others.
+        Throws an exception if the predicate fails for one or more elements and returns true or NULL for the rest.
 
 .. function:: array_average(array(double)) -> double
 

velox/expression/EvalCtx.cpp

@@ -273,6 +273,14 @@ void EvalCtx::addElementErrorsToTopLevel(
   });
 }
 
+void EvalCtx::throwIfHasError(vector_size_t row) {
+  if (hasError(row)) {
+    auto err =
+        std::static_pointer_cast<std::exception_ptr>(errors_->valueAt(row));
+    throwError(*err);
+  }
+}
+
 const VectorPtr& EvalCtx::getField(int32_t index) const {
   const VectorPtr* field;
   if (!peeledFields_.empty()) {

velox/expression/EvalCtx.h

@@ -270,6 +270,12 @@ class EvalCtx {
   /// new elements to null.
   void ensureErrorsVectorSize(ErrorVectorPtr& vector, vector_size_t size) const;
 
+  bool hasError(vector_size_t row) const {
+    return errors_ && row < errors_->size() && !errors_->isNullAt(row);
+  }
+
+  void throwIfHasError(vector_size_t row);
+
  private:
   core::ExecCtx* const FOLLY_NONNULL execCtx_;
   ExprSet* FOLLY_NULLABLE const exprSet_;

velox/functions/prestosql/ArrayAllMatch.cpp

@@ -60,11 +60,10 @@ class AllMatchFunction : public exec::VectorFunction {
     exec::LocalDecodedVector bitsDecoder(context);
     auto it = args[1]->asUnchecked<FunctionVector>()->iterator(&rows);
 
-    // Turns off the ThrowOnError flag and temporarily resets errors in the
+    // Turn off the ThrowOnError flag and reset errors in the
     // context to nullptr, so we only handle errors in this function itself.
+    auto topLevelThrowOnError = context.throwOnError();
     ScopedVarSetter throwOnError(context.mutableThrowOnError(), false);
-    ScopedVarSetter<exec::EvalCtx::ErrorVectorPtr> errorsSetter(
-        context.errorsPtr(), nullptr);
     while (auto entry = it.next()) {
       auto elementRows =
           toElementRows<ArrayVector>(numElements, *entry.rows, flatArray.get());
@@ -86,8 +85,7 @@ class AllMatchFunction : public exec::VectorFunction {
         auto offset = offsets[row];
         auto allMatch = true;
         auto hasNull = false;
-        auto hasError =
-            errors && row < errors->size() && !errors->isNullAt(row);
+
         for (auto i = 0; i < size; ++i) {
           auto idx = offset + i;
           if (bitsDecoder->isNullAt(idx)) {
@@ -102,10 +100,8 @@ class AllMatchFunction : public exec::VectorFunction {
         // outcome can be decided by some other array element, e.g. if there is
         // another element that returns 'false' for the predicate.
         if (allMatch) {
-          if (hasError) {
-            auto err = std::static_pointer_cast<std::exception_ptr>(
-                context.errors()->valueAt(row));
-            std::rethrow_exception(*err);
+          if (topLevelThrowOnError) {
+            context.throwIfHasError(row);
           }
 
           if (hasNull) {

velox/functions/prestosql/tests/ArrayAllMatchTest.cpp

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include "velox/common/base/tests/GTestUtils.h"
 #include "velox/functions/prestosql/tests/utils/FunctionBaseTest.h"
 
 using namespace facebook::velox;
@@ -129,30 +130,36 @@ TEST_F(ArrayAllMatchTest, errorSuppress) {
   auto result = evaluate<SimpleVector<bool>>(
       "all_match(c0, x -> ((10 / x) > 2))", makeRowVector({input}));
 
-  auto expectedResult = makeNullableFlatVector<bool>({false, false});
+  auto expectedResult = makeFlatVector<bool>({false, false});
   assertEqualVectors(expectedResult, result);
 }
 
 TEST_F(ArrayAllMatchTest, errorReThrow) {
-  static constexpr std::string_view errorMessage{"division by zero"};
-  static constexpr std::string_view errorCode{"ARITHMETIC_ERROR"};
-
-  try {
-    evaluate<SimpleVector<bool>>(
-        "all_match(c0, x -> ((10 / x) > 2))",
-        makeRowVector({makeNullableArrayVector<int8_t>({{1, 0}})}));
-  } catch (const VeloxUserError& ve) {
-    EXPECT_EQ(errorMessage, ve.message());
-    EXPECT_EQ(errorCode, ve.errorCode());
-  }
-
-  try {
-    evaluate<SimpleVector<bool>>(
-        "all_match(c0, x -> ((10 / x) > 2))",
-        makeRowVector(
-            {makeNullableArrayVector<int8_t>({{1, 0, std::nullopt}})}));
-  } catch (const VeloxUserError& ve) {
-    EXPECT_EQ(errorMessage, ve.message());
-    EXPECT_EQ(errorCode, ve.errorCode());
-  }
+  static constexpr std::string_view kErrorMessage{"division by zero"};
+
+  VELOX_ASSERT_THROW(
+      evaluate<SimpleVector<bool>>(
+          "all_match(c0, x -> ((10 / x) > 2))",
+          makeRowVector({makeArrayVector<int8_t>({{1, 0}})})),
+      kErrorMessage);
+  VELOX_ASSERT_THROW(
+      evaluate<SimpleVector<bool>>(
+          "all_match(c0, x -> ((10 / x) > 2))",
+          makeRowVector(
+              {makeNullableArrayVector<int8_t>({{1, 0, std::nullopt}})})),
+      kErrorMessage);
+}
+
+TEST_F(ArrayAllMatchTest, withTrys) {
+  auto result = evaluate<SimpleVector<bool>>(
+      "TRY(all_match(c0, x -> ((10 / x) > 2)))",
+      makeRowVector({makeNullableArrayVector<int8_t>({{1, 0}})}));
+  auto expectedResult = makeNullableFlatVector<bool>({std::nullopt});
+  assertEqualVectors(expectedResult, result);
+
+  result = evaluate<SimpleVector<bool>>(
+      "all_match(c0, x -> (TRY((10 / x) > 2)))",
+      makeRowVector({makeArrayVector<int8_t>({{1, 0}})}));
+  expectedResult = makeNullableFlatVector<bool>({std::nullopt});
+  assertEqualVectors(expectedResult, result);
 }