SEPURI-SAI-KRISHNA commented on PR #13092: URL: https://github.com/apache/seatunnel/pull/13092#issuecomment-6820550190
@SEZ9 thanks for the thorough review. Changes are in `96594e52d5`: 1. Query tables: `isNullableSplitKey` now returns true whenever `query` is set, so a query always gets the NULL check and the hash `IS NOT NULL`; a table path keeps trusting its schema. Test: the schema says NOT NULL, the query returns NULL rows, range and hash, every id read once. 2. Explicit bounds: with both bounds on a numeric key, the fixed splitter reads closed ranges inside them, so NULL is outside the range like any other value outside it, and I skip the NULL split there (`readsOnlyConfiguredRange`, overridden in `FixedChunkSplitter`). String keys still get it, because their hash splits ignore the bounds and their range splits have open ends. Documented in Jdbc.md (EN and ZH), and the troubleshooting sentence now says NULL rows are outside the range too. 3. A failing NULL check now logs a warning and adds the NULL split anyway. Test with a connection whose `createStatement` fails. 4. I kept the separate check. Adding `SUM(CASE WHEN c IS NULL ...)` to the MIN/MAX query turns it into a full scan, while today MIN/MAX on an indexed column is answered from the index, and the column picked automatically is a unique key, so it is indexed. The check stops at the first NULL and can use that index where the database indexes NULL (MySQL, PostgreSQL). For an unindexed `partition_column` both are full scans; if you prefer folding for that case, I can add it. 5. incompatible-changes (EN and ZH) now says not to restore a checkpoint of this version with an older connector. 6. Corrected there and in Jdbc.md: the rows read are the same, and each hash split of a nullable column gets `AND <col> IS NOT NULL`. 7. Javadoc added, including the query and missing-key cases. 8. New test: a split serialized by the previous seven-field class (checked-in base64) deserializes with `isNullSplitKey() == false` and reads its range. The PR description now covers 1, 2, 3, 5 and 8 as well. `connector-jdbc`: 1293 tests pass. I broke each of 1 to 3 on purpose and its test failed. `JdbcMysqlSplitIT` on MySQL 8.0: 7 tests pass, including `testNullSplitKeyRowsAreReadOnce`. -- 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]
