luozihen commented on PR #12015:
URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5582554227
@SEZ9
Thanks for the update, and for the clean checklist — that makes it much
easier to confirm.
Here's the status against F1–F8 on the latest head:
**Addressed:**
- **F1** — I reverted `ReadonlyConfig#toConfig()` back to
`ConfigFactory.parseMap(confData)`, so
the dotted-key path-expansion semantics are unchanged. The fix is now
scoped to
`MultiTableFailureHelper.mergeOptions()`, which merges the option maps
directly instead of
going through `toConfig()`.
- **F4** — `${primary_key}` / `${unique_key}` expansion happens only in the
connector
(`expandPrimaryKeyPlaceholder`). The engine-level
`TablePlaceholderProcessor.replaceTablePlaceholder` only rewrites
top-level String /
single-element-String-List values, so it leaves the nested
`multi-table_config` map untouched —
no double processing.
**Addressed, but please confirm if you'd like it stricter:**
- **F2 / F5** — I kept the order-based "first match wins" behavior and
documented it. This is backed by
the code path: `ReadonlyConfig.fromConfig` uses Jackson's
`TypeReference<Map<String, Object>>`,
which deserializes to a `LinkedHashMap`, preserving declaration order; I did
not add a separate
order-preservation test, so if you'd prefer I pin it down with a test or an
explicit `LinkedHashMap`
type, I can do that.
**Still open — happy to address as follow-ups:**
- **F3** — I haven't added explicit identifier validation for the resolved
key columns; they are
still comma-joined into `PRIMARY_KEYS` and rely on the existing list
conversion.
- **F6** — regex patterns are currently validated lazily at match time
(thrown as `JDBC-12`), not
up-front during config validation.
- **F7** — I haven't added a regression test for the restored `toConfig()`
dotted-key behavior, nor
an engine-level IT/E2E for the new option (only unit tests so far).
- **F8** — the key is `multi-table_config` to match the name we settled on
in the issue. If the
project convention prefers a different form, I'm fine renaming it.
Happy to take the open ones on, or wait for your targeted comments and fix
them accordingly.
Thanks again!
--
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]
