LeonYoah commented on PR #10428: URL: https://github.com/apache/seatunnel/pull/10428#issuecomment-3828403466
> 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:在审阅了这些更改之后，我有一些需要解决的问题，特别是关于 Oracle 中的时区处理问题： > > ## 1. Critical Timezone Issue in `OracleConnectionUtils` > 1. `OracleConnectionUtils` 中的关键时区问题 > In `seatunnel-connectors-v2/connector-cdc/connector-cdc-oracle/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/oracle/utils/OracleConnectionUtils.java`: 在 `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:您目前使用 `timestamp.toString()` 将时间戳转换为字符串，然后在 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.**风险：** `timestamp.toString()` 依赖 JVM 的默认时区来格式化字符串。在 Oracle 中执行 `TO_TIMESTAMP` 时，它会根据数据库会话的时区来解释字符串。 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.如果 SeaTunnel JVM 时区（例如 UTC）与 Oracle 数据库时区（例如 Asia/Shanghai）不同，则计算出的 SCN 将不正确（可能偏差几个小时），导致启动时数据丢失或重复。 > > **Suggestion:** Use JDBC's `setTimestamp` directly, which is safer and handles the type conversion correctly:**建议：** 直接使用 JDBC 的 `setTimestamp` ，这样更安全，也能正确处理类型转换： > > ```java > String sql = "SELECT TIMESTAMP_TO_SCN(?) AS SCN FROM DUAL"; > // ... > statement.setTimestamp(1, new java.sql.Timestamp(timestampMs)); > ``` > > ## 2. Missing Documentation > 2. 文件缺失 > 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.在 `docs/en/connectors/source/Oracle-CDC.md` （以及中文版）中，您更新了 `startup.mode` 的描述，使其包含 `timestamp` ，这很好。但是，** 选项**表中缺少 `startup.timestamp` 配置项本身。 > > 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.用户不会知道他们需要配置 `startup.timestamp` ，也不会知道它需要一个 `Long` 值（毫秒）。请将此缺失的配置项添加到文档中。 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]
