SEZ9 commented on PR #12592: URL: https://github.com/apache/seatunnel/pull/12592#issuecomment-6378579901
@hutiefang76 thanks for the pointers against `cb6ae01c`, that makes this much easier to follow. **F1** – noted that `reconvertTimestampType` is intended to preserve the declared scale, including `TIMESTAMP_NS`, and that the native-DDL test and sink round trip cover it. I will verify that against the diff before marking it resolved. On other sinks consuming the scale through their own converters: the original concern was the auto-DDL change for every DuckDB-to-X pipeline, so I would still like the upgrade note to call that out explicitly. Your comment appears to have been cut off right at the upgrade-note link, so I only see the F1 part. Could you re-post the rest? Specifically, I still need to confirm: - **F4** – does the upgrade note now state that a plain DuckDB `TIMESTAMP` produces MySQL `DATETIME(6)` instead of `DATETIME`? - **F2 / F6** – is the pre-epoch alias path (JVM-default-zone `getTimestamp`) now guarded, and is there a test pinning the two documented limitations (pre-1970 DST gap, Gregorian-cutover date)? - **F3 / F8** – have the one-time WARN and the Javadoc been reworded so they no longer hard-code the driver version, and does the Javadoc state that the metadata-based detection is version-independent? - **F5 / F7** – has the per-cell `ResultSetMetaData` lookup / `Calendar` allocation been hoisted out of the hot read path (e.g. resolved once per result set)? If any of these are already in `cb6ae01c`, links like the ones you gave for F1 are all I need; if some are intentionally deferred, just say so and I will note it. <!-- streview-comment:1982 --> -- 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]
