nzw921rx commented on PR #13025: URL: https://github.com/apache/seatunnel/pull/13025#issuecomment-6461311177
Thanks for the fix. I reviewed head `dca588e6f81f9a5d649911f7c1d1a13acddfcb88` and repeated local validation after merging dev `2aa67dbd370d1591255da9a51d8ef70f01373010` into an isolated checkout. No blocking code issue was identified. ### Local validation | Validation | Result | |---|---| | Merge with the inspected dev revision | No conflicts | | JDBC unit suite on JDK 11 | 1,141 reported tests; zero failures/errors; six existing skips | | JDBC unit suite on JDK 17 | 1,141 reported tests; zero failures/errors; six existing skips | | Real MySQL 8.0.43 database matrix | 648 cases / 10,368 checked values; zero read mismatches, write mismatches, or SQL errors | | Actual Zeta JDBC Source → JDBC Sink jobs | All 12 scenarios passed, with 16 expected rows per job | | `JobHistoryServiceTest` on JDK 11 | Both tests passed | | Engine/JDBC reactor build | All 41 modules passed | | JDBC reactor verification, including Spotless | Passed | | Whitespace checks | Passed | The JDBC unit runs exclude the Oracle/Kingbase container test packages. The six skips are existing environment-dependent catalog tests. ### Temporal and boundary coverage The database matrix covers: - Independent JVM, source-connection, and sink-connection zones: UTC, Asia/Shanghai, and America/New_York. - Client-side and server-side prepared statements. - Both `preserveInstants` settings. - Individual updates, ordinary batches, and rewritten batches. - JDBC-read values and directly constructed `LocalDateTime` inputs. - NULL, microseconds, pre-epoch values, leap days, year/day boundaries, DST gaps and overlaps, historical dates, and years 1000/9999. The Zeta jobs cover three JVM zones, two connection zones, and both preparation modes. Destination values are checked independently using SQL `DATE_FORMAT` for DATETIME fields and `UNIX_TIMESTAMP` for TIMESTAMP instants. ### Scope and compatibility The implementation remains focused: - MySQL binds ordinary `LocalDateTime` values directly. - The shared JDBC default retains its previous `setTimestamp` behavior. - StarRocks explicitly retains its previous binding. - Source mappings, TIME, TIMESTAMP_TZ, and checkpoint-state classes are unchanged. - The new binding hook introduces no shared mutable state. - SQL binding failures remain visible without falling back to the old conversion. - Both language versions of the documentation explain the behavior change, manual compensation offsets, and the limitation that existing stored values are not repaired automatically. This validation did not include local Flink/Spark jobs or the Docker-based integration harness. ### CI status At the time of this review, the required Build check was failing in `Run / unit-test (11, ubuntu-latest)`. The uploaded report contains one error in `JobHistoryServiceTest`: `IllegalStateException: Node failed to start!`, after approximately 300 seconds during test initialization. The report does not establish the underlying startup failure cause. Both tests passed locally on JDK 11 with the dev-integrated revision. This supports retrying the failed CI job, but does not replace the required CI check. From the source review and independent local validation, I support merging once the required Build check passes. -- 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]
