misc: Add a new error code for remote function to be caught by try#26928
Closed
feilong-liu wants to merge 1 commit intoprestodb:masterfrom
Closed
misc: Add a new error code for remote function to be caught by try#26928feilong-liu wants to merge 1 commit intoprestodb:masterfrom
feilong-liu wants to merge 1 commit intoprestodb:masterfrom
Conversation
Differential Revision: D90356706
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a new REMOTE_FUNCTION_ERROR_CAUGHT_BY_TRY standard error code and ensures TryFunction treats this error code as handled (i.e., swallowed) similarly to other user-level data errors. Class diagram for updated StandardErrorCode and TryFunction handlingclassDiagram
class StandardErrorCode {
<<enum>>
+DIVISION_BY_ZERO
+INVALID_CAST_ARGUMENT
+INVALID_FUNCTION_ARGUMENT
+NUMERIC_VALUE_OUT_OF_RANGE
+REMOTE_FUNCTION_ERROR_CAUGHT_BY_TRY
+toErrorCode() ErrorCode
}
class ErrorCode {
+getCode() int
}
class PrestoException {
-ErrorCode errorCode
+getErrorCode() ErrorCode
}
class TryFunction {
-propagateIfUnhandled(e PrestoException) void
}
StandardErrorCode --> ErrorCode : creates
PrestoException --> ErrorCode : has
TryFunction ..> PrestoException : uses
TryFunction ..> StandardErrorCode : compares codes
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The growing chain of
||checks inpropagateIfUnhandledis getting harder to maintain; consider refactoring these error-code checks into a staticSetor helper method so adding new handled error codes is less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The growing chain of `||` checks in `propagateIfUnhandled` is getting harder to maintain; consider refactoring these error-code checks into a static `Set` or helper method so adding new handled error codes is less error-prone.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Does this also allow for wrapping the full exception trace from remote? |
kaikalur
reviewed
Jan 9, 2026
| import static com.facebook.presto.spi.StandardErrorCode.INVALID_CAST_ARGUMENT; | ||
| import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT; | ||
| import static com.facebook.presto.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; | ||
| import static com.facebook.presto.spi.StandardErrorCode.REMOTE_FUNCTION_ERROR_CAUGHT_BY_TRY; |
Contributor
There was a problem hiding this comment.
Do we need the CAUGHT_BY_TRY suffix? Maybe better would be a general:
CATCHABLE_EXCEPTION
and have a class that can be subclassed so we can add more later if needed?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When running remote functions, error in one single function call can fail the whole query. Consider that remote functions is less stable than local functions, sometimes it makes sense to not fail the query if a few of the function calls fail.
This PR adds a new error code, which can be used in remote functions, so that users can opt in to not fail a query with the TRY function.
Motivation and Context
Give user the flexibility to not fail a query if a few remote function calls fail
Impact
As in description
Test Plan
simple change
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.