DanielLeens commented on PR #11746: URL: https://github.com/apache/seatunnel/pull/11746#issuecomment-6338484156
Thanks @SEZ9. I checked the requested paths on `ea4e9cada227`; here are the committed answers rather than another summary-only approval: 1. **F1:** `DynamicChunkSplitter.queryMinMaxComposite` filters NULL components at 218-225. `buildCompositeCondition` includes them in the first chunk (1423-1431) and excludes them from later chunks (1432-1448). `JdbcSqliteSplitIT.testCompositeKeyWithNullPrimaryKeyComponent` starts at 260 and asserts 93 total / 3 NULL-key rows exactly once at 344-352, with NULL secondary keys inside a middle chunk's leading-column range. 2. **F2:** `OracleDialect.supportCompositeKeySplit` at 353-374 reads the live major version, returns false before 12, and also falls back on metadata-query failure. `CompositeKeyChunkSplitterTest.testOracleCompositeSplitGatedOnDatabaseVersion` starts at 477. 3. **F3/F4:** `ChunkSplitter.java:445-457` explicitly excludes STRING components from the composite path. The fallback is documented in EN Jdbc.md at 303 and 313; `testFindSplitKeyFallsBackToSingleColumnForStringCompositeKey` starts at 207. 4. **F5:** the comma constant still exists at `DynamicChunkSplitter.java:117-124`, with its display-only limitation documented there. It is not parsed to recover columns: predicates read field names from SeaTunnelRowType (1421), and `isCompositeSplit` checks that type (1547-1549). Thus the joined display name can be ambiguous, but it does not split a comma-bearing identifier into a different boundary key. I would keep an unambiguous display encoding as a follow-up rather than equating this with runtime boundary corruption. 5. **F6:** boundary/dedup checks use `compareArrays`, including at 166, and delegate to `compareCompositeElement` / `compareNumeric` (432-475). Numeric pairs compare through BigDecimal, which handles Integer/Long and scale differences; non-finite floats have the explicit Double.compare fallback. The mixed-number test starts at `CompositeKeyChunkSplitterTest:584`. 6. **F7:** EN Jdbc.md has the explicit Upgrading paragraph at 311 and both opt-outs at 309. 7. **F8:** the dynamic-splitter gate is at `ChunkSplitter.java:445-447`; `FixedChunkSplitter.createSplits` also rejects multi-column keys at 65-76, with tests at `CompositeKeyChunkSplitterTest:758,788`. Build is green on this head. This is a targeted reply, not a new full review or a claim that I ran the tests locally. My source-level conclusion remains clear; I will leave your direct final verification and the write-capable maintainer approval/merge to you. Current status evidence: head `ea4e9cada2272e8258b1e2a80da3a77436892de2`, base `dev`, compare `diverged` (ahead 31, behind 82), mergeable state `blocked`. -- 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]
