SEZ9 commented on PR #12015:
URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5707320566
@luozihen Thanks for moving the F1–F8 status into the PR description — that
works better than the truncated comments, and I'll use the "Review status
(F1–F8)" section as the source of truth on the next pass.
To make that pass quick, here's what I'll be checking against for each item,
so please make sure the description points me at the concrete change (or states
explicitly that you're pushing back and why):
- **F1 / F7** – whether the `ReadonlyConfig.toConfig()` dotted-key behavior
is preserved for existing callers, and where the regression test for the old
expansion lives (e.g. in `ReadableConfigTest`), plus which IT/E2E exercises the
new JDBC option end to end.
- **F2 / F5** – how "first pattern in declaration order wins" is actually
guaranteed given the `Map<String, Object>` option type, or that the docs in
`docs/en/connectors/sink/Jdbc.md` were reworded to match the real behavior.
- **F3** – where the resolved key columns are validated as identifiers
before they are joined into `PRIMARY_KEYS` and reach generated SQL.
- **F4** – a short description (ideally with a test) of the ordering between
the connector-level `${primary_key}`/`${unique_key}` expansion and the
engine-level `TablePlaceholder` replacement.
- **F6** – where user-supplied regexes are compiled up front and mapped to
the JDBC-12 error code.
- **F8** – the final option key name (the `multi-table_config`
hyphen/underscore mix), or the reasoning if you want to keep it.
One unrelated note on CI: the failures on run `34802808218` for head
`2c5982be8` are `minio/minio` image pulls in Paimon/Databend/S3 tests, none of
which are touched by this PR; a rebase onto `dev` should clear them. Once the
branch is rebased and the description status is complete, ping me and I'll go
through F1–F8 in one pass.
<!-- streview-comment:1096 -->
--
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]
