LeonYoah commented on PR #10428: URL: https://github.com/apache/seatunnel/pull/10428#issuecomment-3828404193
> Hi @LeonYoah, thanks for this contribution! The ability to start from a specific timestamp is a very valuable feature for CDC tasks. > > After reviewing the changes, I have a few concerns that need to be addressed, particularly regarding timezone handling in Oracle: > > ## 1. Critical Timezone Issue in `OracleConnectionUtils` > In `seatunnel-connectors-v2/connector-cdc/connector-cdc-oracle/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/oracle/utils/OracleConnectionUtils.java`: > > You are currently converting the timestamp to a string using `timestamp.toString()` and then parsing it in Oracle: > > ```java > String sql = "SELECT TIMESTAMP_TO_SCN(TO_TIMESTAMP(?, 'YYYY-MM-DD HH24:MI:SS.FF3')) AS SCN FROM DUAL"; > // ... > statement.setString(1, timestamp.toString()); > ``` > > **Risk:** `timestamp.toString()` relies on the JVM's default timezone to format the string. When `TO_TIMESTAMP` is executed in Oracle, it interprets the string based on the database session's timezone. If the SeaTunnel JVM timezone (e.g., UTC) is different from the Oracle Database timezone (e.g., Asia/Shanghai), the calculated SCN will be incorrect (potentially skewed by several hours), leading to data loss or duplication on startup. > > **Suggestion:** Use JDBC's `setTimestamp` directly, which is safer and handles the type conversion correctly: > > ```java > String sql = "SELECT TIMESTAMP_TO_SCN(?) AS SCN FROM DUAL"; > // ... > statement.setTimestamp(1, new java.sql.Timestamp(timestampMs)); > ``` > > ## 2. Missing Documentation > In `docs/en/connectors/source/Oracle-CDC.md` (and the cn version), you updated the description for `startup.mode` to include `timestamp`, which is great. However, the `startup.timestamp` configuration key itself is missing from the **选项** table. > > Users will not know that they need to configure `startup.timestamp` or that it requires a `Long` value (milliseconds). Please add this missing configuration item to the documentation. Thank you for your review. I think your suggestions are completely fine. I will try to make the changes. -- 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]
