SEZ9 commented on PR #11746: URL: https://github.com/apache/seatunnel/pull/11746#issuecomment-6159578260
@li3zhi4 thanks for the detailed update. Taking your three items in order: - **Issue 1 (SQL Server version gate):** The shape you describe — major version check, WARN + fall back to the existing single-column split on an older version or on a metadata-read `SQLException`, explicit tests for 10/11/16 and the failure path, and docs that separate the runtime gate from the compatibility-level prerequisite — is exactly what I was after. I'll read it on `0d14a5669f`; nothing further needed from you on this one unless the review turns something up. - **Issue 2 (OceanBase Javadoc / import):** Noted as done, will confirm on the commit. - **Issue 4 (`sqlite.version` pin):** Keeping the matching 3.39.3.0 pin plus the keep-in-sync comment is fine with me; agreed that a POM hierarchy change doesn't belong in this PR. One direct follow-up tied to Issue 1: the same gate-and-fallback pattern is what PR11746-F2 asks for on Oracle — `OracleDialect` currently enables composite splitting unconditionally while the boundary SQL depends on 12c+ `FETCH FIRST`, so 11g composite-PK jobs would hit a syntax error instead of falling back. Is that covered on `0d14a5669f` as well, or still pending? If pending, mirroring the SQL Server approach (version check → WARN → single-column fallback, with the same style of tests) would close it. What I still need a status on from the earlier review, since this comment covers 1/2/4 only: - **F1** — SQLite composite split dropping rows whose PK components are NULL. - **F3 / F4** — STRING key columns in the composite path with no collation guard; DB `ORDER BY` vs `compareArrays` disagreement can collapse or truncate the walk. - **F5** — comma-based `COMPOSITE_KEY_SEPARATOR` encoding with identifiers/values containing commas. - **F6** — `compareArrays` / `Arrays.equals` on JDBC boundary tuples (Integer vs Long from SQLite, scale-sensitive `BigDecimal.equals`). - **F7** — upgrade note and a documented way to opt back into the old single-column split. - **F8** — the removed "only one split key" guard in `ChunkSplitter.generateSplits`, leaving `FixedChunkSplitter` unprotected. A short per-item "done on `0d14a5669f`" / "pending" / "disagree, because…" would be enough for me to finish the pass in one go. <!-- streview-comment:1734 --> -- 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]
