Skip to content

Commit

Permalink
Fix error handling in least and greatest Presto functions (#3818)
Browse files Browse the repository at this point in the history
Summary:
Report errors via EvalCtx::setError instead of throwing.

Fixes #3794

Pull Request resolved: #3818

Reviewed By: mbasmanova

Differential Revision: D42756552

Pulled By: kgpai

fbshipit-source-id: ad7bbe2653a60ed6180dd6dcba43251d205f61a1
  • Loading branch information
kgpai authored and facebook-github-bot committed Jan 26, 2023
1 parent a6bb02a commit 045d5df
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
2 changes: 1 addition & 1 deletion velox/functions/prestosql/GreatestLeast.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class ExtremeValueFunction : public exec::VectorFunction {
exec::DecodedArgs decodedArgs(rows, args, context);

std::set<size_t> usedInputs;
rows.applyToSelected([&](int row) {
context.applyToSelectedNoThrow(rows, [&](int row) {
size_t valueIndex = 0;

T currentValue = decodedArgs.at(0)->valueAt<T>(row);
Expand Down
11 changes: 9 additions & 2 deletions velox/functions/prestosql/tests/GreatestLeastTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class GreatestLeastTest : public functions::test::FunctionBaseTest {
void runTest(
const std::string& query,
const std::vector<std::vector<T>>& inputs,
const std::vector<T>& output,
const std::vector<std::optional<T>>& output,
std::optional<size_t> stringBuffersExpectedCount = std::nullopt) {
// Create input vectors
auto vectorSize = inputs[0].size();
Expand All @@ -40,7 +40,11 @@ class GreatestLeastTest : public functions::test::FunctionBaseTest {
// Call evaluate to run the query on the created input
auto result = evaluate<SimpleVector<T>>(query, makeRowVector(inputColumns));
for (int32_t i = 0; i < vectorSize; ++i) {
ASSERT_EQ(result->valueAt(i), output[i]);
if (output[i].has_value()) {
ASSERT_EQ(result->valueAt(i), output[i]);
} else {
ASSERT_TRUE(result->isNullAt(i));
}
}

if (stringBuffersExpectedCount.has_value()) {
Expand All @@ -59,15 +63,18 @@ TEST_F(GreatestLeastTest, leastDouble) {
}

TEST_F(GreatestLeastTest, nanInput) {
std::vector<double> input{0, 1.1, std::nan("1")};
assertUserInvalidArgument(
[&]() { runTest<double>("least(c0)", {{0.0 / 0.0}}, {0}); },
"Invalid argument to least(): NaN");
runTest<double>("try(least(c0, 1.0))", {input}, {0, 1.0, std::nullopt});

assertUserInvalidArgument(
[&]() {
runTest<double>("greatest(c0)", {1, {0.0 / 0.0}}, {1, 0});
},
"Invalid argument to greatest(): NaN");
runTest<double>("try(greatest(c0, 1.0))", {input}, {1.0, 1.1, std::nullopt});
}

TEST_F(GreatestLeastTest, greatestDouble) {
Expand Down

0 comments on commit 045d5df

Please sign in to comment.