Re: [PR] [GLUTEN-12157][VL] Register sin, tan, tanh, radians in Velox sparksql function registry [gluten]

["brijrajk (via GitHub)" <gi...@apache.org>]

brijrajk commented on PR #12158:
URL: https://github.com/apache/gluten/pull/12158#issuecomment-4563294438

   > 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](https://github.com/facebookincubator/velox/pull/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, check `yAsymptote:` in `mathExpressions.scala`).
   > 
   > So a change on Velox is indeed inevitable, 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.
   
   Thanks for catching that — you're right, prestosql::LnFunction returns -Infinity/NaN instead of null for out-of-domain inputs. I've dropped ln from this PR. The remaining four (sin, tan, tanh, radians) are clean. Happy to add ln in a follow-up once the Velox sparksql implementation lands upstream.


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org