DanielLeens commented on PR #12592: URL: https://github.com/apache/seatunnel/pull/12592#issuecomment-6362889167
Thanks @hutiefang76 for the item-by-item reply, and thanks @SEZ9 for the precise list. I re-read the cited code at `cb6ae01c0` (code reading only, I did not compile or run anything) to cross-check the claims, and the pinned links line up with the source: - **F1 (scale through `reconvert`)**: `DuckDBTypeConverter.reconvertTimestampType` maps scale 0/3/6/9 to `TIMESTAMP_S`/`TIMESTAMP_MS`/`TIMESTAMP`/`TIMESTAMP_NS`, rounds intermediate scales up to the next native precision, and only clamps above 9 with a WARN. So a DuckDB `TIMESTAMP_NS` source going to a DuckDB sink keeps `TIMESTAMP_NS`; it does not degrade. For other sinks the scale is consumed by each dialect's own converter, and the MySQL 6-digit cap is called out in `incompatible-changes.md`. - **F2 / F6 (pre-epoch alias values)**: the legacy-getter branch is reached only for values before the epoch, and `testPreEpochAliasReadsKeepTheLegacyGetterForDstGapAndCutover` pins the DST-gap and Gregorian-cutover behaviour. The docs state this as a deferred limitation rather than a fix, which I think is an honest and acceptable scoping. - **F3**: `warnOnceBeforeLegacyAliasFallback` is called only inside the pre-epoch branch and the message has no driver version. - **F4**: the standard `TIMESTAMP` to MySQL `DATETIME(6)` change is in the EN and ZH upgrade notes. - **F5**: `isAliasTimestamp` caches the per-column decision per `ResultSet` and rebuilds it when a different result set is passed in. - **F7 / F8**: the per-cell `Calendar` and the Javadoc on version-independent metadata detection match what the reply says. These are the same two Low, non-blocking items I listed in my last review (Issues 1 and 2) as a follow-up. So from my side none of F1 to F8 reopens a source-level blocker, and my conclusion from the last review is unchanged: no non-CI blocker. @SEZ9, if any item still looks different to you after checking those links, please say which one and I will re-verify it. On CI: this head still needs its own required `Build` result (currently in progress: https://github.com/apache/seatunnel/runs/119994188490). If a failure shows up outside the DuckDB and JDBC code, rerunning only the failed jobs is the minimal next step. I will take another pass once that result is in, or immediately if a new commit lands. -- 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]
