Enable no-throw behavior for simple functions #9659
Conversation
✅ Deploy Preview for meta-velox canceled.
|
5a64a6a to
fce47c7
Compare
fce47c7 to
49bb20b
Compare
| if constexpr (udf_has_call_return_bool) { | ||
|
|
||
| if constexpr (udf_has_call_return_status) { | ||
| notNull = true; |
There was a problem hiding this comment.
so the new call function cannot return nulls for now, and support for this will be added in a future PR?
velox/expression/EvalCtx.cpp
Outdated
| @@ -172,6 +172,36 @@ auto throwError(const std::exception_ptr& exceptionPtr) { | |||
| } | |||
| } // namespace | |||
|
|
|||
| void EvalCtx::setStatus(vector_size_t index, Status status) { | |||
| VELOX_DCHECK(!status.ok()); | |||
There was a problem hiding this comment.
wonder if we should have an actual check. Otherwise in non-debug code an "ok" will be wrapped as a runtime error.
| struct NoThrowFunction { | ||
| VELOX_DEFINE_FUNCTION_TYPES(TExec); | ||
|
|
||
| Status call(out_type<int64_t>& out, const arg_type<int64_t>& in) { |
There was a problem hiding this comment.
maybe tests what happens if you have a runtime error? Should it always "throw" right away, or does "try" also swallow it?
There was a problem hiding this comment.
We don't have consistency here at the moment. TRY should suppress only USER errors. EvalCtx::applyToSelectedNoThrow throws non-USER errors and stores USER errors. However. EvalCtx::setError stores all errors. I think it makes sense for EvalCtx::setStatus to match EvalCtx::applyToSelectedNoThrow and store USER errors while throwing other errors. Let me know if you think otherwise.
| @@ -1303,14 +1303,17 @@ struct DateParseFunction { | |||
| } | |||
|
|
|||
| auto dateTimeResult = | |||
| format_->parse(std::string_view(input.data(), input.size()), true) | |||
| .value(); | |||
| format_->parse(std::string_view(input.data(), input.size())); | |||
There was a problem hiding this comment.
nit: can't you do implicit cast from StringView into string_view? If not, explicit should work:
format_->parse(input);
or
format_->parse((std::string_view)input);
Summary: Throwing exceptions is expensive. When many rows throw exceptions that are being suppressed by TRY, query performance degrades a lot. Vector functions may avoid throwing exceptions by calling EvalCtx::setError, but Simple functions can only throw. This change introduces new 'call' method to be used by Simple function to avoid throwing: ``` Status call(result, arg1, arg2,...); ``` This method can report errors via Status without throwing. This change allows default-null-behavior functions to avoid throwing. More changes are needed to allow all simple functions to avoid throwing. Use the new API to avoid throwing in date_parse Presto function. Reviewed By: pedroerp Differential Revision: D56705407
49bb20b to
7a6e6aa
Compare
|
@pedroerp Pedro, thank you for review. I updated the PR to address your comments. |
…0274) Summary: This pr continues the no-throw-exception optimization for more simple function methods, following the work of PR #9659. Fixes #10265 Pull Request resolved: #10274 Reviewed By: xiaoxmeng Differential Revision: D59012420 Pulled By: bikramSingh91 fbshipit-source-id: 3b409782f3ab4e2e401e4027ae251cf552d4620c
Summary:
Throwing exceptions is expensive. When many rows throw exceptions that are being suppressed by TRY, query performance degrades a lot.
Vector functions may avoid throwing exceptions by calling EvalCtx::setError, but Simple functions can only throw.
This change introduces new 'call' method to be used by Simple function to avoid throwing:
This method can report errors via Status without throwing.
This change allows default-null-behavior functions to avoid throwing. More changes are needed to allow all simple functions to avoid throwing.
Differential Revision: D56705407