SEZ9 commented on PR #11746: URL: https://github.com/apache/seatunnel/pull/11746#issuecomment-6246254121
Thanks for the follow-up push. Comparing `056524ee04` against `e7586c06a8`, the only change is the `.withStartupTimeout(Duration.ofMinutes(6))` added to the Oracle container builders in the two Oracle ITs. That looks like a reasonable test-stability tweak. Since no main code, docs or options changed in this delta, a few points from the previous round still look open to me: - **F5 (`DynamicChunkSplitter`)** – the comma-based `COMPOSITE_KEY_SEPARATOR` encoding is fragile for identifiers/values that contain commas. Could the key metadata be carried as a structured list, or at least escaped/quoted? - **F6 (`DynamicChunkSplitter`)** – `compareArrays`/`Arrays.equals` on raw JDBC values is brittle (e.g. SQLite returning Integer vs Long for the same column, scale-sensitive `BigDecimal.equals`). Normalising numerics before comparing plus a small unit test for mixed Integer/Long and differing-scale BigDecimal tuples would help. - **F7 (`docs/en/connectors/source/Jdbc.md`)** – an upgrade note describing the behavior change for existing composite-PK jobs, and an explicit statement that setting `partition_column` restores the previous single-column split, would be useful. - **F8** – the "only one split key" guard was removed from the shared `ChunkSplitter.generateSplits`; an equivalent guard on the `FixedChunkSplitter` path would let a multi-column key there fail fast with a clear error. On the earlier F1–F4 points (NULL key components, Oracle version gating, STRING key collation): the current code already appears to have NULL handling, a version gate via `supportCompositeKeySplit(DatabaseMetaData)`, and a non-STRING requirement for key columns, so I'm treating those as addressed. If I've misread any of that, a short note is enough. Happy to re-review once the next push touches the main sources. <!-- streview-comment:1852 --> -- 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]
