SEZ9 commented on PR #12592: URL: https://github.com/apache/seatunnel/pull/12592#issuecomment-6406643914
@hutiefang76 thanks for the re-check on `cb6ae01c`, and apologies for the slow reply. - **F1:** Thanks for pointing me to the `reconvert` scale mapping (9 → `TIMESTAMP_NS`, 0/3/6 → `TIMESTAMP_S`/`TIMESTAMP_MS`/`TIMESTAMP`, intermediate scales widened) and to `testReconvertTimestampFollowsDeclaredPrecision` / `testReconvertTimestampWidensIntermediatePrecision` in `DuckDBTypeConverterTest`. That is exactly the behaviour I was asking for; I'll confirm it against the diff and close F1 once I've done so. - **F3/F8:** The once-per-converter WARN gated on a non-null alias value actually reaching the pre-epoch legacy fallback sounds like the right fix for the noise concern, and metadata-based, driver-version-independent detection in the Javadoc is what F8 asked for. Your comment looks truncated after "will not automat…", so if there was more, please repost it; I'll verify the Javadoc wording in the diff as well. - **F5/F7:** Fine to keep the `Calendar` allocation hoisting as a non-blocking follow-up. Two items from my original review I don't see addressed in this thread yet: 1. **F2/F6:** Is the pre-epoch legacy-getter branch now pinned by tests for the two documented limitations (pre-1970 DST gap and a Gregorian-cutover alias date)? If they exist, please point me to the test names; if not, I'd like them added so the documented behaviour doesn't silently drift. 2. **F4:** Does the upgrade note in `docs/en/introduction/concepts/incompatible-changes.md` now call out that a standard DuckDB `TIMESTAMP` generates MySQL `DATETIME(6)` instead of `DATETIME`? That is the most common user-visible DDL change and should be stated explicitly. Once I have answers on those two, I'm happy to move this forward. <!-- streview-comment:2030 --> -- 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]
