SEZ9 commented on PR #11069: URL: https://github.com/apache/seatunnel/pull/11069#issuecomment-5381236074
@DanielLeens Thanks for the detailed follow-up and the extra evidence — answering it directly, since it slipped through without a reply. Agreed on the +1 to Issue 1, and your framing of the asymmetry is exactly right: the type-mismatch branch names the value and its class, while the far more common format-mismatch branch leaks a bare `DateTimeParseException` from `java.time` with no column, table, or offending value. Given `OffsetDateTime.parse` uses `ISO_OFFSET_DATE_TIME` strictly, `Z`-less values, `+00`-style two-digit offsets, or a different `time.precision.mode` would all land there unactionably. I'm treating this as blocking too: the fix should wrap the parse and rethrow with the column name plus the raw value in the message. Also +1 on Issue 3 — this is user-visible type support, so the Postgres CDC connector docs need to state `TIMESTAMP_TZ` support in both `docs/en` and `docs/zh`. Concrete remaining asks before a full re-review: 1. Sync the branch with the latest `dev` and resolve the reported merge conflicts, so CI signal is attributable to this diff rather than the stale base. 2. Wrap the `OffsetDateTime.parse` call in `convertToTimestampTz()` and rethrow with column name + raw value (Issue 1). 3. Add the `TIMESTAMP_TZ` documentation updates in `docs/en` and `docs/zh` (Issue 3). Issues 2 and 4 I also agree are valid but lower priority; they can follow in the same push if convenient. @DanielLeens happy to do a full pass over the refreshed head as soon as it's pushed. Thanks again for the careful trace — it made the case for blocking on Issue 1 much easier to justify. <!-- streview-comment:444 --> -- 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]
