brijrajk commented on code in PR #12158:
URL: https://github.com/apache/gluten/pull/12158#discussion_r3326136978
##########
cpp/velox/operators/functions/RegistrationAllFunctions.cc:
##########
@@ -76,6 +77,14 @@ void registerFunctionOverwrite() {
kRowConstructorWithAllNull,
std::make_unique<RowConstructorWithNullCallToSpecialForm>(kRowConstructorWithAllNull));
+ // Register math functions that are present in the prestosql implementation
+ // but not yet in the sparksql registry. These are semantically identical
+ // to Spark's behavior for the same names.
Review Comment:
Yes, that would make sense. As @n0r0shi noted above, they were already
working on registering these in Velox's sparksql::registerMathFunctions
([facebookincubator/velox#17648](https://github.com/facebookincubator/velox/pull/17648))
— including a proper sparksql::LnFunction for Spark's null semantics — when
this PR went up.
Given that upstream work is in flight, I'd propose narrowing this PR to just
the test-framework fixes (abstract class → class, ANSI disable) and the four
Scala integration tests. Those are independently valuable since those suites
are silently not running today, and the tests will validate native execution
once the Velox changes land in Gluten.
Happy to drop the C++ registrations if that approach works for you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
