resolve comments

facebookincubator · Mar 17, 2023 · bbfddfc · bbfddfc
1 parent 118cc5c
commit bbfddfc
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 13 deletions.
diff --git a/velox/functions/prestosql/tests/ArrayAllMatchTest.cpp b/velox/functions/prestosql/tests/ArrayAllMatchTest.cpp
@@ -137,34 +137,32 @@ TEST_F(ArrayAllMatchTest, doubles) {
 
 TEST_F(ArrayAllMatchTest, errors) {
   // No throw and return false if there are unmatched elements except nulls
-  auto input =
-      makeNullableArrayVector<int8_t>({{0, 2, 0, 5, 0}, {5, std::nullopt, 0}});
-  auto result = evaluate<SimpleVector<bool>>(
-      "all_match(c0, x -> ((10 / x) > 2))", makeRowVector({input}));
+  auto input = makeRowVector({makeNullableArrayVector<int8_t>(
+      {{0, 2, 0, 5, 0}, {5, std::nullopt, 0}})});
+  auto result =
+      evaluate<SimpleVector<bool>>("all_match(c0, x -> ((10 / x) > 2))", input);
   auto expectedResult = makeFlatVector<bool>({false, false});
   assertEqualVectors(expectedResult, result);
 
   // Throw error if others are matched or null
   static constexpr std::string_view kErrorMessage{"division by zero"};
-  auto errorInput = makeNullableArrayVector<int8_t>(
-      {{1, 0}, {2}, {6}, {1, 0, std::nullopt}, {10, std::nullopt}});
+  auto errorInput = makeRowVector({makeNullableArrayVector<int8_t>(
+      {{1, 0}, {2}, {6}, {1, 0, std::nullopt}, {10, std::nullopt}})});
   VELOX_ASSERT_THROW(
-      evaluate<SimpleVector<bool>>(
-          "all_match(c0, x -> ((10 / x) > 2))", makeRowVector({errorInput})),
+      evaluate("all_match(c0, x -> ((10 / x) > 2))", errorInput),
       kErrorMessage);
   VELOX_ASSERT_THROW(
-      evaluate<SimpleVector<bool>>(
-          "all_match(c0, x -> ((10 / x) > 2))", makeRowVector({errorInput})),
+      evaluate("all_match(c0, x -> ((10 / x) > 2))", errorInput),
       kErrorMessage);
   // Rerun using TRY to get right results
   expectedResult = makeNullableFlatVector<bool>(
       {std::nullopt, true, false, std::nullopt, false});
   result = evaluate<SimpleVector<bool>>(
-      "TRY(all_match(c0, x -> ((10 / x) > 2)))", makeRowVector({errorInput}));
+      "TRY(all_match(c0, x -> ((10 / x) > 2)))", errorInput);
   assertEqualVectors(expectedResult, result);
 
   result = evaluate<SimpleVector<bool>>(
-      "all_match(c0, x -> (TRY((10 / x) > 2)))", makeRowVector({errorInput}));
+      "all_match(c0, x -> (TRY((10 / x) > 2)))", errorInput);
   assertEqualVectors(expectedResult, result);
 }
 

diff --git a/velox/vector/FunctionVector.h b/velox/vector/FunctionVector.h
@@ -53,7 +53,8 @@ class Callable {
       const BufferPtr& elementToTopLevelRows,
       VectorPtr* result) = 0;
 
-  /// Same as 'apply', but suppress the eval error.
+  /// Same as 'apply', but errors are suppressed and returned in
+  /// 'elementErrors', and errors in 'context' are not updated.
   virtual void applyNoThrow(
       const SelectivityVector& rows,
       const SelectivityVector& finalSelection,