Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling constant folding errors #12414

Open
pramodsatya opened this issue Feb 20, 2025 · 2 comments · May be fixed by #12376
Open

Handling constant folding errors #12414

pramodsatya opened this issue Feb 20, 2025 · 2 comments · May be fixed by #12376
Assignees
Labels
enhancement New feature or request

Comments

@pramodsatya
Copy link
Collaborator

pramodsatya commented Feb 20, 2025

Description

In the FusionNext (SPI) project, we are trying to make the Presto Native engine more correct by doing constant folding with the native worker code. Right now, the co-ordinator uses Presto Java code for constant folding. This could have compatibility issues with the native engine. When investigating this work we saw an issue for cases where constant folding evaluations could throw exceptions.

Velox constant folding code is implemented by the function tryFoldIfConstant. If constant folding results in a VeloxUserError, such as Divide with zero exception for say divide(0, 0), the expression compiler in Velox suppresses this error and returns the input expression as is. The runtime evaluates the expression to throw the exception thereafter.

Presto has a different behavior. Any exceptions encountered during constant folding are replaced by a FailFunction expression (refer to function processWithExceptionHandling). This FailFunction could be used to trigger further optimizations in the co-ordinator.

For evaluating constant folding with the native worker, we introduced a new Native Worker REST endpoint which takes as input a set of Presto expressions and returns constant folded versions of them to the caller. The NativeExpressionOptimizer prestodb/presto#24126 which does the conversion invokes the Velox ExprCompiler to compile constants. As a result this code returns the original expression if the constant folding can throw an exception. The expression is evaluated during runtime and it throws an exception then. While this still results in correct behavior, we would like to retain the Presto optimization with FailFunction to enable more optimizations.

We propose extending the Velox Expression Compiler to aid this conversion. We want to add an option where the Expression Compiler could substitute a FailFunction for the cases where the constant expression evaluation throws. FailFunction could provide more context about the constant folding error, such as an ErrorCode and ErrorMessage. We will trigger this behavior based on a specific config and retain the default behavior to return the original expression.
#12376 models the code we would like.

This code change really simplifies any expression manipulation in the Prestissimo code for NativeExpressionOptimizer as we would otherwise need to re-implement much of the Velox expression compilation there.

Co-authored by: @aditi-pandit @czentgr

@pramodsatya pramodsatya added the enhancement New feature or request label Feb 20, 2025
@pramodsatya pramodsatya self-assigned this Feb 21, 2025
@aditi-pandit
Copy link
Collaborator

@pedroerp @mbasmanova @bikramSingh91 : We encountered this issue when attempting constant folding with Prestissimo native side-car. Would be great to hear your thoughts.

@mbasmanova
Copy link
Contributor

@pramodsatya Sounds reasonable overall.

While this still results in correct behavior, we would like to retain the Presto optimization with FailFunction to enable more optimizations.

Would you elaborate on the types of optimizations this enables? A few examples would be helpful. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants