Skip to content

Commit

Permalink
Allow returning Status from callNullable and callNullFree methods (fa…
Browse files Browse the repository at this point in the history 
…cebookincubator#10274)

Summary:
This pr continues the no-throw-exception optimization for more simple
function methods, following the work of PR facebookincubator#9659.

Fixes facebookincubator#10265

Pull Request resolved: facebookincubator#10274

Reviewed By: xiaoxmeng

Differential Revision: D59012420

Pulled By: bikramSingh91

fbshipit-source-id: 3b409782f3ab4e2e401e4027ae251cf552d4620c
  • Loading branch information
@PHILO-HE @facebook-github-bot
PHILO-HE authored and facebook-github-bot committed Jun 28, 2024
1 parent 258db51 commit bcfc8f8
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 23 deletions.
73 changes: 50 additions & 23 deletions velox/core/SimpleFunctionMetadata.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,15 +666,19 @@ class UDFHolder {
DECLARE_METHOD_RESOLVER(callAscii_method_resolver, callAscii);
DECLARE_METHOD_RESOLVER(initialize_method_resolver, initialize);

// Check which flavor of the call() method is provided by the UDF object. UDFs
// are required to provide at least one of the following methods:
// Check which flavor of the call()/callNullable()/callNullFree() method is
// provided by the UDF object. UDFs are required to provide at least one of
// the following methods:
//
// - bool|void call(...)
// - bool|void callNullable(...)
// - bool|void callNullFree(...)
// - bool|void|Status call(...)
// - bool|void|Status callNullable(...)
// - bool|void|Status callNullFree(...)
//
// Each of these methods can return either bool or void. Returning void means
// that the UDF is assumed never to return null values.
// Each of these methods can return bool, void or Status. Returning void
// means that the UDF is assumed never to return null values. Returning
// Status to hold success or error outcome of the function call, and it
// implies result is not null. If you need to return null as result, please
// use bool return type.
//
// Optionally, UDFs can also provide the following methods:
//
Expand Down Expand Up @@ -707,14 +711,10 @@ class UDFHolder {
udf_has_call_return_void | udf_has_call_return_status;

static_assert(
!(udf_has_call_return_bool && udf_has_call_return_void),
"Provided call() methods need to return either void OR bool OR status.");
static_assert(
!(udf_has_call_return_bool && udf_has_call_return_status),
"Provided call() methods need to return either void OR bool OR status.");
static_assert(
!(udf_has_call_return_void && udf_has_call_return_status),
"Provided call() methods need to return either void OR bool OR status.");
udf_has_call_return_void + udf_has_call_return_bool +
udf_has_call_return_status <=
1,
"Provided call() methods need to only return void, bool OR Status.");

// callNullable():
static constexpr bool udf_has_callNullable_return_bool = util::has_method<
Expand All @@ -729,11 +729,21 @@ class UDFHolder {
void,
exec_return_type,
const exec_arg_type<TArgs>*...>::value;
static constexpr bool udf_has_callNullable_return_status = util::has_method<
Fun,
callNullable_method_resolver,
Status,
exec_return_type,
const exec_arg_type<TArgs>*...>::value;
static constexpr bool udf_has_callNullable =
udf_has_callNullable_return_bool | udf_has_callNullable_return_void;
udf_has_callNullable_return_bool | udf_has_callNullable_return_void |
udf_has_callNullable_return_status;

static_assert(
!(udf_has_callNullable_return_bool && udf_has_callNullable_return_void),
"Provided callNullable() methods need to return either void OR bool.");
udf_has_callNullable_return_void + udf_has_callNullable_return_bool +
udf_has_callNullable_return_status <=
1,
"Provided callNullable() methods need to only return void, bool OR Status.");

// callNullFree():
static constexpr bool udf_has_callNullFree_return_bool = util::has_method<
Expand All @@ -748,11 +758,21 @@ class UDFHolder {
void,
exec_return_type,
const exec_no_nulls_arg_type<TArgs>&...>::value;
static constexpr bool udf_has_callNullFree_return_status = util::has_method<
Fun,
callNullFree_method_resolver,
Status,
exec_return_type,
const exec_no_nulls_arg_type<TArgs>&...>::value;
static constexpr bool udf_has_callNullFree =
udf_has_callNullFree_return_bool | udf_has_callNullFree_return_void;
udf_has_callNullFree_return_bool | udf_has_callNullFree_return_void |
udf_has_callNullFree_return_status;

static_assert(
!(udf_has_callNullFree_return_bool && udf_has_callNullFree_return_void),
"Provided callNullFree() methods need to return either void OR bool.");
udf_has_callNullFree_return_void + udf_has_callNullFree_return_bool +
udf_has_callNullFree_return_status <=
1,
"Provided callNullFree() methods need to only return void, bool OR Status.");

// callAscii():
static constexpr bool udf_has_callAscii_return_bool = util::has_method<
Expand Down Expand Up @@ -966,7 +986,11 @@ class UDFHolder {
bool& notNull,
const typename Exec::template resolver<TArgs>::in_type*... args) {
static_assert(udf_has_callNullable);
if constexpr (udf_has_callNullable_return_bool) {

if constexpr (udf_has_callNullable_return_status) {
notNull = true;
return instance_.callNullable(out, args...);
} else if constexpr (udf_has_callNullable_return_bool) {
notNull = instance_.callNullable(out, args...);
return Status::OK();
} else {
Expand Down Expand Up @@ -995,7 +1019,10 @@ class UDFHolder {
bool& notNull,
const exec_no_nulls_arg_type<TArgs>&... args) {
static_assert(udf_has_callNullFree);
if constexpr (udf_has_callNullFree_return_bool) {
if constexpr (udf_has_callNullFree_return_status) {
notNull = true;
return instance_.callNullFree(out, args...);
} else if constexpr (udf_has_callNullFree_return_bool) {
notNull = instance_.callNullFree(out, args...);
} else {
instance_.callNullFree(out, args...);
Expand Down
67 changes: 67 additions & 0 deletions velox/expression/tests/SimpleFunctionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1542,4 +1542,71 @@ TEST_F(SimpleFunctionTest, noThrow) {
"Input must not be 6");
}

template <typename TExec>
struct CallNullableNoThrowFunction {
VELOX_DEFINE_FUNCTION_TYPES(TExec);

Status callNullable(out_type<int64_t>& out, const arg_type<int64_t>* in) {
if (!in) {
return Status::UserError("Input cannot be NULL");
}
out = *in + 1;
return Status::OK();
}
};

TEST_F(SimpleFunctionTest, callNullableNoThrow) {
registerFunction<CallNullableNoThrowFunction, int64_t, int64_t>(
{"nullable_no_throw"});
// Error reported via Status by callNullable.
VELOX_ASSERT_THROW(
(evaluateOnce<int64_t, int64_t>("nullable_no_throw(c0)", std::nullopt)),
"Input cannot be NULL");
auto result = evaluateOnce<int64_t, int64_t>(
"try(nullable_no_throw(c0))", std::nullopt);
EXPECT_EQ(std::nullopt, result);

// No error.
result = evaluateOnce<int64_t, int64_t>("nullable_no_throw(c0)", 1);
EXPECT_EQ(2, result);
}

template <typename TExec>
struct CallNullFreeNoThrowFunction {
VELOX_DEFINE_FUNCTION_TYPES(TExec);

Status callNullFree(out_type<int64_t>& out, const arg_type<int64_t>& in) {
if (in == 0) {
return Status::UserError("Input cannot be 0");
}
if (in % 2 == 0) {
return Status::UserError("Input cannot be even");
}
out = in + 1;
return Status::OK();
}
};

TEST_F(SimpleFunctionTest, callNullFreeNoThrow) {
registerFunction<CallNullFreeNoThrowFunction, int64_t, int64_t>(
{"null_free_no_throw"});
// Error reported via Status by callNullable.
VELOX_ASSERT_THROW(
(evaluateOnce<int64_t, int64_t>("null_free_no_throw(c0)", 0)),
"Input cannot be 0");
auto result =
evaluateOnce<int64_t, int64_t>("try(null_free_no_throw(c0))", 0);
EXPECT_EQ(std::nullopt, result);

VELOX_ASSERT_THROW(
(evaluateOnce<int64_t, int64_t>("null_free_no_throw(c0)", 4)),
"Input cannot be even");
result = evaluateOnce<int64_t, int64_t>("try(null_free_no_throw(c0))", 4);
EXPECT_EQ(std::nullopt, result);

// No error.
result = evaluateOnce<int64_t, int64_t>("null_free_no_throw(c0)", 1);
EXPECT_EQ(2, result);
}

} // namespace

0 comments on commit bcfc8f8

Please sign in to comment.