DanielLeens commented on PR #12014:
URL: https://github.com/apache/seatunnel/pull/12014#issuecomment-5846261138
Thanks for chasing these down — good timing, since I'd just posted a fresh
full re-review of the current head (`7926b4b342b`) covering this exact ground
independently. Let me fold in the specifics.
One correction on commit attribution first: neither the doc sentence nor the
XA test you're asking about actually landed in `25baad9e3`. That commit touches
only the four `MySQL-CDC.md`/`Jdbc.md` files (en + zh) and adds the "flush
succeeds, then TRUNCATE fails" duplicate-window paragraph. Both the
`exactly_once = false` requirement statement and the
`JdbcExactlyOnceSinkWriterTest` coverage were added earlier, in `1a4f2d32d`
("Reject JDBC XA exactly-once for TRUNCATE table-operations", 2026-09-09) —
that commit's file list is the same four doc files plus
`TruncateTableEvent.java`, `AbstractJdbcSinkWriter.java`,
`JdbcExactlyOnceSinkWriter.java`, and `JdbcExactlyOnceSinkWriterTest.java`
together, so the code, the test, and the doc statement all shipped in one
commit, eight days before `25baad9e3`.
1. **Docs**: the current `docs/en/connectors/sink/Jdbc.md`, in the "Does
JDBC Sink apply MySQL-CDC `TRUNCATE TABLE`?" entry, reads: "The JDBC sink must
keep `exactly_once = false` (the default); `is_exactly_once = true` is not
supported for table operations and fails fast." I read that directly off the
current head rather than off the diff hunk, so it's accurate as of
`7926b4b342b`. `25baad9e3` appends the duplicate-window paragraph right after
it.
2. **Test**:
`JdbcExactlyOnceSinkWriterTest.applyTableOperationIsRejectedOnXaWriter`
(production override at `JdbcExactlyOnceSinkWriter.java:166-179`) is in
`1a4f2d32d`, the same commit as the doc line above — not `25baad9e3`, which is
docs-only. It asserts both the exception message and `verify(xaFacade,
never()).endAndPrepare(any())`, i.e. it proves no XA side effect happens before
the rejection, not just that some exception is thrown.
3. **F4**: still open, agreed. I've folded it into my fresh full re-review
as Issue 2 (Medium) — a restore/failover test that actually lands between a
committed flush and a failed/pending TRUNCATE, to back up the duplicate-window
claim the docs now make. That review's merge recommendation treats it as
non-blocking ("Ready to merge," no blockers), so I'd frame it as a good fast
follow-up rather than something that has to hold up this PR — but I'll leave
the "this PR vs. follow-up" call to det101.
With that, I think F1 is fully closed on both counts you raised here. Happy
to point to specific line ranges in `1a4f2d32d` for anything above if it'd help.
--
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]
