SEZ9 commented on PR #12015:
URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5611887428
@DanielLeens thanks for re-reviewing the whole feature end to end on
`dcae482241c` rather than only the `608b46ba87` delta on top of `1730aeef` —
much appreciated.
On the two items your comment covers:
- **F8 (`multi-table_config` -> `multi_table_config`)**: thanks for checking
that no stale `multi-table_config` occurrences remain across the code, error
message, docs and tests, and that the name now matches `primary_keys` /
`generate_sink_sql`. I'll confirm against the diff on my side before marking it
resolved.
- **F3 (identifier validation before comma-join)**: thanks for tracing that
`validatePrimaryKeyColumns` is called from the shared `applyPrimaryKeys`
helper, so the pre-existing top-level `primary_keys` path via
`applyFallbackPrimaryKeys` gets the same fail-fast `JDBC-12` behavior — that is
exactly the point I wanted covered. Same here: I'll verify it in the diff
before closing.
Your comment appears to be cut off after the F3 section, so I can't see your
conclusions on the remaining points. Could you add a short status on each?
1. **F2 / F5 (pattern precedence)**: does `608b46ba87` actually guarantee
"first pattern in declaration order wins" for
`multi_table_config.primary_keys`, or were the docs adjusted to describe the
real behavior instead?
2. **F6 (regex validation -> `JDBC-12`)**: where are invalid user-supplied
regexes compiled and rejected, and is there a test for it?
3. **F7 (tests)**: is there now a regression test for the old `toConfig()`
dotted-key expansion, and does any engine-level IT/E2E exercise the new option?
4. **F1 (`toConfig()` dotted-key semantics) and F4
(`${primary_key}`/`${unique_key}` vs engine-level placeholder replacement)**: a
one-line status on each from your re-trace would be enough.
Once those are covered and I've checked the diff, I'm happy to sign off on
the current head.
<!-- streview-comment:939 -->
--
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]
