SEZ9 commented on PR #12015:
URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5578424721
@luozihen Great news, thanks for rerunning — glad the Doris flake cleared
and the run is green now. No need to apologise, you're doing everything right
for a first PR.
Next steps from here:
1. I'll do the fresh confirmation pass I promised on `1730aeef`, going back
through the points from my earlier review. To make that quick and avoid me
guessing, could you post a short checklist in this thread of which of those
items your last push (the HOCON fix + rebase) addressed, and which are still
open or you'd like to push back on? Specifically:
- **F1** — the `ReadonlyConfig.toConfig()` JSON round-trip and its effect
on dotted-key path semantics for all callers.
- **F2 / F5** — the "first pattern in declaration order wins" guarantee
vs. the `Map<String, Object>` option type, and keeping the doc statement in
`docs/en/connectors/sink/Jdbc.md` consistent with whatever ordering the code
actually guarantees.
- **F3** — identifier validation for the resolved key columns before
they're comma-joined into `PRIMARY_KEYS` and reach generated SQL.
- **F4** — how the connector-level `${primary_key}`/`${unique_key}`
expansion interacts with the engine-level `TablePlaceholder` replacement.
- **F6** — explicit compile-time validation of user-supplied regex
patterns mapped to the new JDBC-12 error code.
- **F7** — a regression test for the previous `toConfig()` dotted-key
expansion and an engine-level IT/E2E for the new option.
- **F8** — the `multi-table_config` key naming (hyphen + underscore mix).
2. Once I've confirmed those, if anything is still open I'll leave targeted
comments; if it's all covered I'll approve and we'll get a second maintainer
look before merge, which is standard here.
You don't need to take any further action on CI. Just the checklist above
and I'll take it from there.
<!-- streview-comment:896 -->
--
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]
