DanielLeens commented on PR #10724:
URL: https://github.com/apache/seatunnel/pull/10724#issuecomment-4671867365

   Thanks for flagging these two points. I rechecked the current head again and 
I agree both concerns are valid.
   
   1. The Spark `TIMESTAMP_TZ` serialization path is still unsafe for negative 
offsets. In `seatunnel-translation/.../OffsetDateTimeUtils.java:35-38`, the 
current code builds a `BigDecimal` from `time.toInstant().toEpochMilli() + "." 
+ time.getOffset().getTotalSeconds()`. For offsets like `-08:00`, that can 
produce malformed decimal text such as `123.-28800`, so this still needs a real 
fix.
   2. The new MySQL non-UTC E2E does not verify the actual timestamp value yet. 
`JdbcMysqlTimestampIT.java:180-186` only checks job success, and 
`jdbc_mysql_timestamp_non_utc_to_assert.conf:54-66` only asserts the field type 
/ non-null contract, not the stored instant or offset semantics.
   
   There is still no new code after my last full review on this PR, so I do not 
want to post another full review on the same head. Please address these in the 
next commit, and once you push an update I will re-review the latest head from 
scratch.
   


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