LeonYoah commented on PR #12736: URL: https://github.com/apache/seatunnel/pull/12736#issuecomment-6139576794
Thanks @DanielLeens for the detailed review — addressed the doc/clarity items and a few small robustness fixes in the latest push. ### Issue 1 (snapshot / incompatible-changes / docs) - **Docs (en/zh):** Reworded the LOB section so it is clear that **snapshot does not depend on `lob.enabled`** — it is a normal JDBC `SELECT` and LOB locators on that row are materialized (no per-column re-query). Added notes on **downstream types unchanged** (STRING/BYTES), **target DDL** (use LOB-capable columns, not short VARCHAR), and **reader heap** for large LOBs. - **`incompatible-changes.md`:** Not adding an entry. Snapshot previously dropping CLOB/NCLOB/BLOB on JDBC reads was incorrect behavior (Debezium 1.9.8 treated locators as empty when `lob.enabled` was false). The fix restores values the query already returned; SeaTunnel wire types did not change. Failures after upgrade usually mean the target column or write path could not hold large values before (often masked by nulls), not a new breaking contract. ### Issue 3 (`mergeEvents` null overwrite) - **No code change.** We intentionally allow a later SQL `NULL` to overwrite during same-transaction merge (LOB `SET NULL` after insert/update). With **`SUPPLEMENTAL LOG DATA (ALL) COLUMNS`** (documented requirement), non-LOB columns absent from `SET` are copied from the before image in `LogMinerDmlParser`, not left as bare `null`. Narrowing the null guard to LOB/XML only would regress that LOB case and diverges from newer Debezium `TransactionCommitConsumer` behavior. Unit tests cover merge-with-null for both LOB and non-LOB tables. ### Issue 4 (re-select / flashback logging) - **Fixed:** On flashback-unavailable errors, log **once per table** (no stack per row) and **skip** further `AS OF SCN` attempts for that table, reading the current row directly. ### Issue 5 (`StatementBoundLobs` / failed batch) - **Fixed:** `executeBatch` / `executeUpdate` now **`free` temporary LOBs in `finally`**, so a failed execute does not leave locators registered until `close`/`clearBatch`. ### Issue 7 (SAX factory / CI) - **Fixed:** `connector-cdc-oracle` surefire **`argLine`** appends `surefire.module.args` (`javax.xml.parsers.SAXParserFactory=…`) so CI’s `-Dsurefire.jvm.args=…` does not drop the JDK parser override when `xmlparserv2` is on the test classpath. ### Follow-ups (not in this push) - **Issue 2:** Documented heap/DDL guidance; configurable max LOB size can be a follow-up if needed. - **Issue 6:** XMLTYPE E2E still limited by test image / `xdb` setup (noted in `column_type_test.sql`); parser/assembly unit tests cover the XML path. Happy to re-review when convenient. -- 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]
