jiangxt2 commented on PR #58028:
URL: https://github.com/apache/spark/pull/58028#issuecomment-5659728182

   > Reviewed at 
[0c53916](https://github.com/apache/spark/commit/0c53916c52c729b2e8d13c5996945f8d88e86a60).
 I found two issues that should be addressed before merge:
   > 
   > 1. The new ANSI overflow branch reports the internal scaled-microsecond 
conversion as DOUBLE -> BIGINT. The user operation is FLOAT/DOUBLE -> 
TIMESTAMP, so the structured CAST_OVERFLOW parameters expose the wrong value 
and target type; for FLOAT they also expose the wrong source type. Please 
preserve the original input/source type and report TIMESTAMP rather than 
cementing the intermediate conversion in the new tests.
   > 2. The migration guide documents only the non-ANSI change. This patch also 
changes the exact positive boundary in ANSI CAST from a clamped timestamp to 
CAST_OVERFLOW, and in TRY_CAST from a clamped timestamp to NULL. Please 
document those user-facing changes as well.
   > 
   > Non-blocking follow-up: timestamp_seconds still silently clamps 
overflowing Float/Double inputs in both interpreted and codegen paths, although 
its integral and decimal paths throw DATETIME_OVERFLOW.
   
   Thanks for the review. I’ve addressed the two blocking items in commit 
73b72f34d2c:
   
   - CAST_OVERFLOW now reports the original input value and source type 
(DOUBLE/FLOAT), with TIMESTAMP as the target type, consistently across 
interpreted and codegen paths.
   - The migration guide now documents the non-ANSI behavior change and the 
exact positive-boundary behavior for ANSI CAST and TRY_CAST.
   
   I also verified CastWithAnsiOffSuite, CastWithAnsiOnSuite, and TryCastSuite 
locally against this commit; 414 tests passed.


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

Reply via email to