[GLUTEN-12157][VL] Register sin, tan, tanh, radians, ln in Velox sparksql function registry#12158
Open
brijrajk wants to merge 1 commit into
Open
[GLUTEN-12157][VL] Register sin, tan, tanh, radians, ln in Velox sparksql function registry#12158brijrajk wants to merge 1 commit into
brijrajk wants to merge 1 commit into
Conversation
…ksql function registry
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.
What changes are proposed in this pull request?
Five math scalar functions (
sin,tan,tanh,radians,ln) are mapped inExpressionMappings.scalato their Substrait counterparts but were missing fromGluten's Velox C++ function registry (
RegistrationAllFunctions.cc). Queries usingthese functions silently fell back to vanilla Spark instead of running natively in Velox.
This PR registers them in
registerFunctionOverwrite()using the existing Velox prestosqlimplementations from
velox/functions/prestosql/Arithmetic.h, which match Spark's expectedsemantics for these functions.
Also fixes a leftover from PR #11756 (RAS removal):
MathFunctionsValidateSuiteandScalarFunctionsValidateSuitewere left asabstract classafter their only concretesubclasses were deleted, causing all tests in those suites to silently not run. Both are
promoted to
classhere, consistent with the fix already applied toDateFunctionsValidateSuitein that same PR.Fixes #12157
How was this patch tested?
Added tests in
MathFunctionsValidateSuitefor each of the five functions usingrunQueryAndComparewithcheckGlutenPlan[ProjectExecTransformer]to verify nativeexecution in Velox without fallback.
MathFunctionsValidateSuitenow explicitly disables ANSI mode (enabled by default inSpark 4), which wraps arithmetic expressions in ANSI check nodes and shifts the top-level
plan node away from
ProjectExecTransformer. ANSI-specific behavior is covered by theexisting
MathFunctionsValidateSuiteAnsiOn.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude (Anthropic)