HyukjinKwon commented on PR #59408:
URL: https://github.com/apache/spark/pull/59408#issuecomment-6167589384
Thanks for scoping this explicitly. On overflow: having result overflow in
two-argument floor/ceil raise an error in ANSI mode and return NULL otherwise,
for Float, Double and Decimal inputs, matches how Spark already handles decimal
overflow (Cast.changePrecision picks the same two CastUtils helpers), and the
migration note covers the Decimal error-subclass change, so that boundary looks
reasonable to me. On Float conversion: widening to Double is the convention
used by CAST(float AS DECIMAL) and round/bround, but under directional rounding
it changes results for most short-decimal FLOAT values, not just boundary ones.
ceil(CAST('0.1' AS FLOAT), 1) and ceil(CAST('1.1' AS FLOAT), 1) become 0.2 and
1.2, floor(CAST('3.1415' AS FLOAT), 4) becomes 3.1414, and ceil(CAST('0.1' AS
FLOAT), 7) becomes 0.1000001. The Double path rounds the value's own shortest
representation, so DOUBLE 0.1 stays 0.1. Converting a Float through its own
shortest representation (java.lang.Float.toString) would
still give the expected results for the nextDown(0.1f)/nextUp(0.9f) boundary
tests while keeping those short-decimal values unchanged. Could you weigh that
option against staying consistent with the CAST/round widening convention?
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"conversation:6131540574","thread_id":"conversation:6131540574","verdict_sha256":"4e9ff9946d74d4455670835b3fb3fcdcec079d73f2c595b80325571d51a032c5"}
-->
--
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]
