li3zhi4 commented on PR #11746: URL: https://github.com/apache/seatunnel/pull/11746#issuecomment-6126291289
Thanks for continuing the full re-review. I have now addressed the remaining mandatory and optional items and pushed the final head `f9f2bd328` (current `upstream/dev` is an ancestor via merge `1a3d1e479`).\n\n**Mandatory Issue 1 \u2014 all unvalidated inheritance paths are now opted out.** In addition to the four derived dialect classes fixed in `34af774c36`, the factory-created paths are covered:\n\n- Greenplum factory now creates a private opt-out Postgres-derived dialect;\n- HighGo factory creates a private opt-out dialect while preserving its `postgresLow` delegation;\n- OceanBase Oracle-compatible mode now uses the upstream standalone `OceanBaseOracleDialect`, with the composite opt-out added there (preserving upstream sampling/connection behavior; the redundant nested factory class was removed).\n\nThe complete inventory is now explicit: only MySQL, PostgreSQL, SQLite, SQL Server and Oracle have composite opt-in; StarRocks, DSQL, OpenGauss, PostgresLow, Greenplum, HighGo an d OceanBase Oracle mode all return false. Loader/factory coverage is included in `CompositeKeyChunkSplitterTest`.\n\n**Optional Issue 2 \u2014 now implemented, not deferred.** Composite metadata probes now use the same filtered relation as the read path when `where_condition` is configured. `ChunkSplitter.applyWhereCondition()` normalizes the source query; `tableFromClause()` wraps it as a portable derived table alias without `AS` (including Oracle). `queryMinMaxComposite`, `queryNextChunkMaxComposite` and `queryMinComposite` all use it while retaining their per-key `IS NOT NULL` guards. The docs now state that boundary metadata and reads use the same filtered row scope.\n\nCoverage includes SQL relation/alias normalization unit tests and an official MySQL test that filters a 300-row numeric composite table, verifies the first boundary comes from the filtered `LIMIT 10` order, and asserts the complete filtered key set is read exactly once.\n\n**Optional Issue 5 \u2014 addressed.** O racle and SQL Server SplitIT setup no longer binds fixed host ports; they use Testcontainers mapped ports. Oracle readiness now only probes connection validity before one-time table initialization, avoiding retries that repeat `CREATE TABLE`/inserts. SQL Server follows the same readiness-before-initialize shape.\n\n**Other requested cleanups:** numeric/date eligibility and STRING fallback wording is consistent in EN/ZH docs; `SqlType` uses a normal import; duplicate-boundary comparison uses numeric-aware `compareArrays(...) == 0`; EN/ZH incompatible-change entries are present.\n\n**Validation on the final source head:**\n\n- `CompositeKeyChunkSplitterTest`: 23/23;\n- `SqlWhereConditionHelperTest`: 118/118;\n- `DynamicChunkSplitterTest`: 12/12;\n- combined targeted unit tests: **153/153**;\n- JDBC + E2E-part-1/3/7 Spotless: BUILD SUCCESS;\n- Official `JdbcMysqlSplitIT`: **8/8**, failures 0, errors 0, skipped 0;\n- Official `JdbcOracleSplitIT`: **3/3**, failures 0, errors 0, skipped 0 ;\n- Official `JdbcPostgresSplitIT`: **1/1**, failures 0, errors 0, skipped 0;\n- Official `JdbcSqlServerSplitIT`: **1/1**, failures 0, errors 0, skipped 0;\n- Official `JdbcSqliteSplitIT`: **3/3**, failures 0, errors 0, skipped 0;\n- all official E2E runs used the real Testcontainers services and completed with BUILD SUCCESS; `git diff --check` is clean.\n\nThe only remaining gate is repository-side: the fork has not created a Build run for the final head, so Apache's required check remains ACTION_REQUIRED rather than green. Please take the final review on `f9f2bd328`; once the Build dispatch is available, the JDBC-relevant matrix is fully covered.\n -- 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]
