SEPURI-SAI-KRISHNA opened a new pull request, #13092:
URL: https://github.com/apache/seatunnel/pull/13092
Yes, glad to contribute to Apache SeaTunnel.
### Purpose of this pull request
Fixes #13091.
The JDBC source skips every row whose split column is NULL once a table is
read by more than one split, and the job finishes without an error. Each split
reads a range or a hash bucket of the split column, and none of these
conditions matches NULL. The split column can be NULL without any
configuration: for a table without a primary key, the split column is taken
from a unique index, which may hold NULLs.
This PR adds one split that reads the rows whose split column is NULL:
- `ChunkSplitter#generateSplits` adds it after the splitter has built its
splits, when the split column is nullable in the catalog table and a probe
finds a NULL in it: `SELECT 1 FROM <table> WHERE col IS NULL` (or over the
`query`), at most one row, on the same `where_condition` scope as the existing
MIN/MAX query. Nothing is added when the column is `NOT NULL` (a primary key,
for example), when it has no NULL values, or when one of the splits already
reads the whole table, so those jobs get the same splits as before.
- The split is a `JdbcSourceSplit` with the new `nullSplitKey` flag. Its
statement is built like a range split: `SELECT * FROM <table> WHERE col IS
NULL`, or `SELECT * FROM (<query>) st_jdbc_splitter WHERE col IS NULL`, with
`where_condition` applied to the inner query once, so a narrow `query` still
gets the columns the condition needs.
- Hash splits of a nullable column also get `AND col IS NOT NULL`. On MySQL
`CRC32(NULL)` is NULL, but DuckDB's `hash(NULL)` is not, so without this the
NULL rows would be read twice there.
This is the JDBC source counterpart of #12597, which fixed the same gap for
the CDC sources by not splitting on a nullable column. I did not copy that
approach here: the CDC snapshot needs ordered key ranges for its backfill, and
the JDBC source does not, while a JDBC user who sets `partition_column` on a
large table has asked for parallel reads, which a fallback to one split would
silently remove.
Compatibility:
- `JdbcSourceSplit` keeps its `serialVersionUID` and its seven-argument
constructor. A split serialized by the `dev` class deserializes with
`nullSplitKey = false` and its other fields unchanged (checked by writing a
split with the `dev` connector jar and reading it with this class). A restored
job keeps its pending splits, so the fix applies when the table is split again.
- No option is added or changed.
Overlap: #12292 restructures `ChunkSplitter#generateSplits`. Whichever lands
second needs to keep the `addNullSplitKeySplit` call. #11746 (composite keys)
handles NULL key components in its own composite path and leaves single-column
splitting unchanged, and #12991 stops NULL sample values from becoming chunk
boundaries but does not read the NULL rows, so neither overlaps in behavior.
### Does this PR introduce _any_ user-facing change?
Yes. A job whose split column has NULL values now reads the rows it used to
skip. Jobs whose split column is `NOT NULL` or has no NULL values get the same
splits as before; for a nullable split column, split planning runs the one
probe query. The `Jdbc` source docs (EN and ZH) explain the extra split, and
`incompatible-changes.md` (EN and ZH) records the change.
### How was this patch tested?
**Unit tests** in `FixedChunkSplitterTest`, on an embedded DuckDB table
where every tenth row has a NULL split column:
- `testNullSplitKeyRowsAreReadExactlyOnce` reads every split of a range
split on a numeric column and of a hash split on a string column, and checks
that all 100 ids are read exactly once and that one split reads the NULL rows.
Without the extra split it fails with 90 of 100 rows; without the `IS NOT NULL`
guard it fails with 110, because DuckDB hashes NULL to a value.
- `testNotNullSplitKeyAddsNoNullSplitKeySplit` checks that a `NOT NULL`
split column gets no extra split.
- `testNullableSplitKeyWithoutNullValuesAddsNoNullSplitKeySplit` checks that
a nullable split column without NULL values gets no extra split either. If the
probe always reported a NULL, it would fail with 5 splits instead of 4.
**E2E.** `JdbcMysqlSplitIT#testNullSplitKeyRowsAreReadOnce` creates a MySQL
table without a primary key and with a nullable unique key, then reads it with
the dynamic splitter and no split settings (the unique key becomes the split
column) and with a fixed hash split on a nullable string column. Each read must
return all 100 ids exactly once. The existing tests of the class keep their
exact split counts, because the nullable columns of their table hold no NULL
values. I ran the whole `JdbcMysqlSplitIT` (all 6 tests) against a local
`mysql:8.0.43`: all pass with this change. On `dev` (without the
`isNullSplitKey` assertion, which needs this change) the new test fails with 90
of 100 ids.
**Local jobs** on the Zeta engine (local mode), 1000 rows, 100 or 142 with a
NULL split column, `dev` against this change:
| Source settings | `dev` | This PR |
| --- | --- | --- |
| MySQL, `table_path`, no split settings (nullable unique key) | 900 | 1000 |
| MySQL, `table_path`, `partition_column` (int) | 900 | 1000 |
| MySQL, `query`, `partition_column` (string, hash split) | 858 | 1000 |
| MySQL, `query`, `partition_column` (string, `charset_based`) | 858 | 1000 |
| MySQL, `table_path`, `partition_column` (string, `split.string-strategy =
"hash"`) | 858 | 1000 |
| MySQL, narrow `query` + `where_condition` on a column it does not select |
772 | 858 (all matching rows) |
| MySQL, primary key table | 1000 | 1000, no extra split |
| DuckDB, fixed int, dynamic int, dynamic sampled string | 900 / 900 / 858 |
1000 |
| DuckDB, fixed hash split (string) | 1000 | 1000, no duplicates |
No duplicate ids in any run with this change.
Checks, JDK 17:
- `./mvnw test -pl seatunnel-connectors-v2/connector-jdbc`: 1200 tests; the
only errors are the 4 Oracle and Kingbase container tests, which cannot start a
container on this machine.
- `./mvnw spotless:check` on `connector-jdbc` and
`connector-jdbc-e2e-part-7`: clean.
### Check list
* [X] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md):
none added.
* [X] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs: `Jdbc` source docs,
EN and ZH.
* [X] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR: added, EN and ZH.
* [X] If you are contributing the connector code, please check that the
following files are updated: not applicable, no new connector.
--
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]
