davidzollo commented on PR #11029:
URL: https://github.com/apache/seatunnel/pull/11029#issuecomment-5555551636

   Thanks @DanielLeens and @SEZ9 — pushed `f48adc80a76` addressing the open 
items from the 2026-08-23 and 2026-08-31 reviews. I re-traced the 
`snapshot-only` question against source before changing anything: 
`SnapshotOnlySplitAssigner` lives in `connector-cdc-base` and 
`IncrementalSource` gates it purely on `StartupMode` (`getBoundedness()` and 
`createEnumerator()`), with no dialect call in that path — so you were both 
right that only `committed-offset` is PostgreSQL-specific.
   
   **Blocking**
   - Issue 2 (2026-08-31) / SEZ9 Issue 1 — `SNAPSHOT_ONLY` restored in 
`OpengaussSourceOptions.STARTUP_MODE`; the choice set is now `initial`, 
`snapshot-only`, `earliest`, `latest`, with only `committed-offset` excluded. 
`exactly_once` is offered under `snapshot-only` as well, mirroring the 
PostgreSQL rule. The Javadoc that claimed both modes were PostgreSQL-specific 
is corrected. `testOptionRuleExposesOnlyOpengaussStartupModes` pins the four 
values and explicitly asserts `COMMITTED_OFFSET` is absent; new 
`testOptionRuleOffersExactlyOnceForBothSnapshotModes` pins the `exactly_once` 
conditional for both `initial` and `snapshot-only`.
   - Issue 1 (2026-08-31) / SEZ9 Issue 6 — the en/zh Opengauss-CDC 
`startup.mode` rows (which the dev sync had taken from #11707's five-value 
wording) now list exactly the four accepted values and state that 
`committed-offset` is PostgreSQL-only and rejected at config validation. The 
`exactly_once` row now says `initial` or `snapshot-only`.
   
   **Non-blocking**
   - Issue 3 / SEZ9 Issue 3 — 
`OpengaussIncrementalSource.getStartupModeOption()` now documents that it is 
invoked from inside `IncrementalSource`'s constructor and must remain a 
stateless constant. I kept it as a Javadoc contract rather than a constructor 
argument, since the latter would change `IncrementalSource` in 
`connector-cdc-base`, which this PR deliberately leaves untouched.
   - Issue 4 / SEZ9 Issue 7 — the raw `(SingleChoiceOption)` cast is removed 
entirely; the single-choice builder already returns 
`SingleChoiceOption<StartupMode>`.
   - Issue 5 / SEZ9 Issue 8 — "Apache OpenGauss" wording fixed.
   - SEZ9 Issues 2 and 5 — `PostgresDialect`'s replica-identity failure now 
names both remediations (`ALTER TABLE ... REPLICA IDENTITY FULL` or 
`require-replica-identity-full = false`), since on enumerator restore the 
config is baked into the persisted DAG and can only be changed by cancel and 
resubmit. `PostgresCDCIT` asserts the new exact wording and 
`PostgresIncrementalSourceTest` asserts both remediations are present. The 
en/zh Opengauss-CDC prerequisite step now names the opt-out like 
PostgreSQL-CDC.md does, and the PR description has a "Restore caveat" paragraph 
covering the cancel-and-resubmit case.
   - SEZ9 Issue 4 — agree with Daniel's trace: the pre-refactor 
`PostgresIncrementalSource` had exactly one instance field, which this PR 
deletes rather than relocates, and `PgBaseIncrementalSource` declares none, so 
there is no moved field for per-declaring-class resolution to mis-assign. Added 
a Compatibility bullet in the description stating this.
   
   CI on the fork for `f48adc80a76` is in flight now. The only remaining human 
step is the write-capable maintainer approval and merge.
   


-- 
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]

Reply via email to