wgzhao commented on PR #12333:
URL: https://github.com/apache/seatunnel/pull/12333#issuecomment-5697389735
Thanks. Follow-up pushed as `a8a0963db`, covering both points.
## Issue 1 - E2E coverage: added
You were right and my justification was wrong. I claimed the trigger needs a
MySQL 5.7 system-table shape, but as you point out, an ordinary user-defined
`SET` / `ENUM` column whose value list contains the literal word "unsigned"
reproduces it on any MySQL version - including the `mysql:8.0.43` image this
suite already uses.
`JdbcMysqlIT` now creates a small table alongside the existing fixtures and
drives it through real catalog discovery:
```sql
CREATE TABLE `seatunnel`.`set_unsigned_source` (
`c_int` int DEFAULT NULL,
`c_int_unsigned` int unsigned DEFAULT NULL,
`c_set` set('REAL_AS_FLOAT','NO_UNSIGNED_SUBTRACTION') DEFAULT NULL,
`c_enum` enum('unsigned','other') DEFAULT NULL)
```
and asserts the discovered SeaTunnel types: `STRING` for `c_set` and
`c_enum`, `LONG` for `c_int_unsigned` (INT UNSIGNED widens), `INT` for `c_int`.
The table is dropped again inside the test so it cannot be picked up by the
pattern or multi-table cases.
## Issue 2 - javadoc corrected
Fixed. It no longer claims the set mirrors both converters. It now says the
set is the one `MySqlTypeConverter` handles and is a *superset* of what
`OceanBaseMySqlTypeConverter` handles (`BIT` and `YEAR` have no `*_UNSIGNED`
case there), and states why keeping the superset is deliberate: this guard only
decides whether a column is unsigned, and narrowing it per dialect would
silently drop the attribute for the shared numeric types.
## Independent verification against a real server
This sandbox has no Docker, so I could not run `JdbcMysqlIT` itself. I
validated both its assumptions and the fix separately, against a throwaway
MySQL instance (`mysqld --initialize-insecure` in a temp dir, MySQL 9.7.2)
using the real Connector/J driver and the real `MySqlCatalog`:
With the fix applied:
```
c_int -> INT sourceType=int
c_int_unsigned -> BIGINT sourceType=int unsigned
c_float_unsigned -> FLOAT sourceType=float unsigned
c_bigint_unsigned -> Decimal(20, 0) sourceType=bigint unsigned
c_set -> STRING
sourceType=set('REAL_AS_FLOAT','PIPES_AS_CONCAT','ANSI_QUOTES','NO_UNSIGNED_SUBTRACTION','NO_DIR_IN_CREATE')
c_enum -> STRING
sourceType=enum('unsigned','signed','other')
c_set_plain -> STRING sourceType=set('a','b')
```
With only the `MySqlCatalog` line reverted to the old derivation, same
server, same driver:
```
ErrorCode:[COMMON-20], 'mysql' table 'seatunnel.set_unsigned_source'
unsupported get catalog table
with field data types '{"c_set":"SET UNSIGNED","c_enum":"ENUM UNSIGNED"}'
```
Two facts fell out of that run which are worth recording, because both would
be easy to get wrong:
1. MySQL 8/9 no longer reports the display width for integer types. `int(10)
unsigned` comes back as `int unsigned` and `bigint(20) unsigned` as `bigint
unsigned`. A derivation keyed off the `(` would have silently missed these; the
leading-token form handles both shapes.
2. `DATA_TYPE` comes back lowercase (`set`, `enum`, `int`). That is why
simply dropping the substring check is not enough on its own - the leading
token has to be uppercased for the whitelist lookup.
Thanks for the CI triage on the Postgres / openGauss / Pulsar lanes; that
matches what I see, and the `connector-jdbc` module reports SUCCESS in the same
reactor run.
--
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]
