SEPURI-SAI-KRISHNA commented on PR #11724:
URL: https://github.com/apache/seatunnel/pull/11724#issuecomment-5367323569

   @DanielLeens closing the loop on the follow-up I owed from your Issue 
Summary.
   
   **Issue 3 — `toBigDecimal` unification** is now up as its own PR: #11918.
   
   It does what was promised and nothing more. The two copies were 
byte-identical in behaviour, so this is a pure de-duplication: 
`NumericFunction.toBigDecimal` goes `private` → `public`, `ZetaSQLFunction`'s 
private copy is deleted, and the DECIMAL branch of `executeBinaryExpr` calls 
the surviving one. Net `+16/-32` across the two files, no behaviour change and 
no new tests, since the existing `SQLDecimalArithmeticTest` and 
`NumericFunctionTest` already cover both call paths and now exercise a single 
implementation.
   
   The merged Javadoc keeps the reasoning from both originals — why integral 
types go through `longValue()` rather than `doubleValue()`, and why 
`Float`/`Double` use `BigDecimal.valueOf` — and adds a line naming both 
callers, so the next person to touch it knows it is shared rather than local to 
`NumericFunction`.
   
   **Issues 1, 2, and 4** remain tracked and unchanged: the 
addition/subtraction scale question, the absence of anything bounding result 
precision (the near-ceiling test plus `MathContext`/explicit overflow check), 
and the divide-by-zero asymmetry where `DOUBLE` silently yields 
`Infinity`/`NaN` while `INT`/`BIGINT` throw a bare `ArithmeticException`. None 
is made worse by #11918.
   
   Separately, while working in `NumericFunction` for this I turned up a few 
defects that are outside both PRs and will get their own issues rather than 
being folded in here — integral overflow in the `ROUND`/`CEIL`/`FLOOR`/`TRUNC` 
family at negative scale, `ABS` at `Integer.MIN_VALUE`/`Long.MIN_VALUE`, and 
TINYINT/SMALLINT being declared supported by `ZetaSQLType` while the function 
bodies either throw or silently return the input unrounded. I will link them 
here once filed, since they sit next to the code you reviewed.
   
   Thanks again for the seven rounds on this one.
   


-- 
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]

Reply via email to