n0r0shi commented on PR #12158:
URL: https://github.com/apache/gluten/pull/12158#issuecomment-4562883422
Hi @brijrajk, apologies for the parallel work, I didn't see your PR was
already up when I started. I had an upstream Velox fix in flight at the same
time: facebookincubator/velox#17648.
Heads up on `ln`, independent of which path lands: `prestosql::LnFunction`
calls `std::log` and returns `-Infinity` for `ln(0)` / `NaN` for `ln(-1)`.
Spark's `UnaryLogExpression` returns `null` in both cases (Hive heritage in
`mathExpressions.scala`).
The Velox PR adds a `sparksql::LnFunction` mirroring the existing
`sparksql::Log2Function` for that reason. The other
four(`sin`/`tan`/`tanh`/`radians`) are fine with the prestosql impls — no
domain restriction, same as `sinh`/`cos`/`cosh`/ `degrees` already.
Your test-framework fixes (`abstract class` → `class`, ANSI disable) are
independently valuable and should land regardless — those suites really are
silently not running today.
Happy to coordinate whichever direction maintainers prefer.
--
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]
