SEZ9 commented on PR #12014: URL: https://github.com/apache/seatunnel/pull/12014#issuecomment-5788344356
Thanks — noted on the head SHA. I'll re-review against `b9f6da93` rather than `913cc45e`, since `913cc45e` is its first parent and the newer `dev`-merge is what's actually at the tip. On the merge itself: I agree the bulk of `913cc45e..b9f6da93` is unrelated `dev` work and the PR's own diff is unchanged by patch-id, so I won't re-litigate the new classes. The part that does matter for my previous finding PR12014-F1 (flush-then-TRUNCATE atomicity) is that the merge pulls in changes to `JdbcOutputFormat.java` and `AbstractJdbcSinkWriter.java` — specifically the `commitOnFlush` flag and the `flushException`/`commitFailed` latch that makes `JdbcOutputFormat.flush()` short-circuit with a warning once a flush has already failed. The trace of how `applyTableOperation()` → `prepareCommit()` → `JdbcSinkWriter.prepareCommitInternal()` behaves under that latch was cut off mid-sentence in the thread, so I can't take the conclusion as verified yet. Concrete asks to close out F1 on the new tip: 1. Please finish the trace (or point me to it): on `b9f6da93`, if a batch flush has already failed and the latch is set, does `applyTableOperation()` still surface that failure *before* the `TRUNCATE` statement is executed on every path through `prepareCommitInternal()`? A short code walk or a unit test that asserts "prior flush failure ⇒ no TRUNCATE issued" would settle it. 2. The original F1 concerns still stand independent of the latch: `TRUNCATE` is DDL with an implicit commit, it can fail on FK constraints, and the behavior in exactly-once/XA mode is undefined. Please confirm how each is handled (or explicitly rejected) and reflect that in `docs/en/connectors/sink/Jdbc.md`. For the remaining items from my previous review (F2–F8), the merge doesn't appear to introduce any authored changes touching them, so I'll assume they're still open unless you tell me otherwise — a quick status per item on the current tip would help me finish the pass in one go. <!-- streview-comment:1254 --> -- 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]
