wgzhao opened a new pull request, #12333:
URL: https://github.com/apache/seatunnel/pull/12333
### Purpose of this pull request
Fixes #10451.
The MySQL catalog and the OceanBase MySQL catalog derived the `UNSIGNED`
attribute of a column by
searching the whole `COLUMN_TYPE` string for the word `unsigned`:
```java
boolean unsigned = columnType.toLowerCase(Locale.ROOT).contains("unsigned");
```
`COLUMN_TYPE` is not just the type name. For `SET` and `ENUM` columns it
also carries the value
list, and `mysql.event.sql_mode` is declared like this:
```
set('REAL_AS_FLOAT','PIPES_AS_CONCAT','ANSI_QUOTES','IGNORE_SPACE','NOT_USED',...,
'ONLY_FULL_GROUP_BY','NO_UNSIGNED_SUBTRACTION','NO_DIR_IN_CREATE')
```
So the literal value name `NO_UNSIGNED_SUBTRACTION` inside that list was
read as the attribute. Both
type converters then append `" UNSIGNED"` to the type name, which turns
`SET` into the type
`SET UNSIGNED` - a type that does not exist in MySQL:
```
ErrorCode:[COMMON-17], ErrorDescription:['MySQL' unsupported convert type
'SET UNSIGNED' ...]
```
That happens during catalog discovery, so a multi-table MySQL-CDC job fails
at startup before it
reads a single binlog event.
### What this PR changes
In MySQL the `UNSIGNED` attribute is only valid on numeric types, and it is
always placed after an
optional length / precision part: `int(10) unsigned`, `bigint(20) unsigned
zerofill`, and
`float unsigned` (no length part when declared without precision). So the
derivation now reads the
leading type name and checks it against the numeric types that the
converters actually describe
with a `*_UNSIGNED` constant:
```java
String dataType = columnType.trim().split("[\\s(]",
2)[0].toUpperCase(Locale.ROOT);
return UNSIGNED_TYPE_NAMES.contains(dataType)
&& columnType.toLowerCase(Locale.ROOT).contains("unsigned");
```
The rule lives in `JdbcCatalogUtils.isNumericUnsignedColumnType(...)` and is
now used by both
`MySqlCatalog.buildColumn` and `OceanBaseMySqlCatalog.buildColumn`, which
carried the identical bug.
Their converters are symmetric, so the `ENUM('...unsigned...')` sibling case
failed in exactly the
same way and is fixed by the same change.
Relationship to #10453: that pull request adds defensive handling for an
externally supplied
`SET UNSIGNED` type name. This pull request removes the reason such a name
is ever synthesized in
the first place. The two are complementary and neither depends on the other.
### Does this PR introduce _any_ user-facing change?
Yes - it is a bug fix, and it only widens what works:
- **Before**: reading a `SET` or `ENUM` column whose value list contains the
word "unsigned"
(through MySQL-CDC startup or through a JDBC catalog read) failed the job
with
`COMMON-17 unsupported convert type`.
- **After**: the column is reported as `STRING`, which is the mapping the
connector documentation
already describes for `SET` and `ENUM`.
Every genuinely `unsigned` numeric column is unaffected: the attribute can
only be attached to the
numeric types in the whitelist, and all of those are still detected, in both
the
`int(10) unsigned` and the `float unsigned` shape.
No config option, default value, public API, SPI contract, serialization
format or checkpoint state
is touched. `Column#isUnsigned()` is consumed only by the two dialects' type
converters, and only
to decide whether to append `" UNSIGNED"`.
### How was this patch tested?
Two focused unit test classes, neither of which needs a MySQL server:
- `JdbcCatalogUtilsTest` covers the derivation itself: unsigned numerics in
both shapes
(`int(10) unsigned`, and `float unsigned` which has no length part),
signed numerics, the exact
`mysql.event.sql_mode` shape, the `ENUM` sibling shape, and blank input.
- `MySqlCatalogUnsignedDetectionTest` drives the real
`MySqlCatalog.buildColumn` with a stubbed
`ResultSet` and asserts the resulting SeaTunnel column type for the same
inputs, so the
catalog-side integration is covered too. It needs no server: the catalog
constructor probes the
server version, and the test catalog overrides connection acquisition so
the probe fails and the
default version is used - which is the production fallback path when the
probe cannot reach the
server. `MySqlCatalogTest` is `@Disabled` because it requires a live
server, so these offline
cases are kept in a separate class.
I confirmed the regression tests actually catch the bug by reverting only
the `MySqlCatalog` line to
the old `contains("unsigned")` derivation and re-running the new class. Both
the SET and the ENUM
case fail with the original error and nothing else changed:
```
ErrorCode:[COMMON-17], ErrorDescription:['MySQL' unsupported convert type
'SET UNSIGNED' of 'test' to SeaTunnel data type.]
ErrorCode:[COMMON-17], ErrorDescription:['MySQL' unsupported convert type
'ENUM UNSIGNED' of 'test' to SeaTunnel data type.]
```
Then the full unit test suite of the connector:
```
$ mvn -pl seatunnel-connectors-v2/connector-jdbc test
Tests run: 994, Failures: 0, Errors: 4, Skipped: 6
```
The 4 errors are Testcontainers classes that require Docker (Kingbase and
Oracle catalog / dialect
container tests) failing with `Previous attempts to find a Docker
environment failed`; they do not
touch the MySQL type conversion path and fail identically without this
change.
No E2E case is added. The trigger is a MySQL 5.7 system table shape
(`mysql.event` / `mysql.proc`)
that the current MySQL 8 based e2e fixtures do not reproduce, while the
derivation and its
conversion consequence are both covered directly by the unit tests above.
### 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)
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
* [x] If you are contributing the connector code, please check that the
following files are updated:
1. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties)
and add new connector information in it
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml)
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml)
4. Add e2e testcase in
[seatunnel-e2e](https://github.com/apache/seatunnel/tree/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/)
5. Update connector
[plugin_config](https://github.com/apache/seatunnel/blob/dev/config/plugin_config)
--
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]
