SEZ9 commented on PR #12015:
URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5754813722
Thanks for pulling the full log and laying out the evidence — that made this
quick to follow up on.
**`JdbcMysqlMultipleTablesIT` failure (relates to PR12015-F7)**
In fork run 35303656875 (head `c15f9fdc5`) the failure is `All parts of a
PRIMARY KEY must be NOT NULL`: the MySQL DDL builder emits an explicit `NULL`
for nullable columns and then appends `PRIMARY KEY (...)` when `create_index`
is true. That is a pre-existing gap that the top-level `primary_keys` path hits
as well, so it is not introduced by this PR — but it is triggered by this PR's
new test case.
To keep the PR scoped:
1. Please do not change `MysqlCreateTableSqlBuilder` (or the OceanBase
MySQL-mode twin) here. Open a separate issue for the "emit `NOT NULL` for
primary-key columns" fix; a follow-up PR with a unit test would be very welcome.
2. Fix the E2E fixture instead: add two small dedicated source tables for
this scenario whose key columns (e.g. `c_int`, `c_integer`, `c_mediumint`) are
declared `NOT NULL`, and point the patterns in the new `.conf` at them. Please
leave the shared `source.table1` / `source.table2` definitions untouched, since
the other test methods depend on them. Your assertions on resolved primary keys
and row counts can stay as they are.
Once that is in, the IT/E2E half of F7 is covered. The other half — a
regression test for the previous `toConfig()` dotted-key expansion behaviour —
is still open.
**Option name (PR12015-F8)**
The thread now refers to the option as `multi_table_config`, while my
earlier note was on `multi-table_config`. I'll confirm the key in
`JdbcSinkOptions` and in the docs against the diff on the next pass; if both
consistently use `multi_table_config`, F8 is resolved.
**Still open from the previous review**
I haven't seen responses yet on F1 (`ReadonlyConfig.toConfig()` JSON
round-trip changing dotted-key semantics for all callers), F2/F5
(declaration-order guarantee for a `Map<String, Object>`-typed option and the
matching docs claim), F3 (identifier validation before the resolved keys are
joined into `PRIMARY_KEYS`), F4 (interaction with engine-level
`${primary_key}`/`${unique_key}` placeholder replacement) and F6 (compile-time
regex validation mapped to the JDBC-12 error code). A short note on how you
plan to handle each would help me do the next pass in one go.
<!-- streview-comment:1207 -->
--
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]
