DanielLeens commented on PR #12561:
URL: https://github.com/apache/seatunnel/pull/12561#issuecomment-6411587558
Thanks @nzw921rx for the careful P2, and thanks @goutamadwant for your
patience with this one. I re-checked the finding against the unchanged head
`17aa4e8e6` (code reading only, nothing executed locally), and it holds, so
please treat my earlier "no outstanding source blocker" line as superseded for
this point. Sorry for missing this in my last review.
Evidence from source:
- `PostgresIncrementalSourceFactory.restoreSource` (around `:129-137`) gates
the filter on the top-level `decoding.plugin.name` option only, which defaults
to `pgoutput`.
- `PostgresSourceConfigFactory.configureConnectorProperties` (`:70-72`) sets
`plugin.name` from that option, but `PgBaseSourceConfigFactory.create()` then
runs `props.putAll(dbzProperties)` after it (right after the
`table.include.list` block), so a `debezium { "plugin.name" = "decoderbufs" }`
passthrough wins for what Debezium actually uses.
- `PostgresPublishedColumns` also reads `publication.name` from the same
`debezium` map, so for such a job it would look at a publication that does not
govern decoderbufs output. With an explicit column list it would drop ordinary
columns, and with an INSERT-only publication that omits the key it could hit
the new primary-key fail-fast. The base version never filtered these jobs, so
this is a behavior change for an existing configuration.
Suggested direction (the contributor's call on the exact shape): resolve the
effective plugin once as "debezium `plugin.name` if present, otherwise the
top-level option", use that value for the `PGOUTPUT` guard, and add a
regression test for the precedence (top-level default `pgoutput` plus
`debezium.plugin.name = decoderbufs` must leave the schema untouched). If the
same effective-plugin question applies to `validateSchemaEvolutionOptions` in
`PostgresIncrementalSource.java:206-220`, it is worth deciding consciously, but
that is outside this PR.
This is a Medium compatibility point, narrow in reach (it needs the debezium
override of `plugin.name`), so I would fix it in this PR rather than defer it.
The rest of my earlier assessment is unchanged, and the new commit will get a
fresh look from me once it is pushed.
--
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]
