wgzhao commented on PR #10453:
URL: https://github.com/apache/seatunnel/pull/10453#issuecomment-5697390221
Thanks @DanielLeens - and thanks for the unusually candid note about the
`columnType` leak. Appreciated.
Two follow-ups, plus one acknowledgement.
## Issue 1 (root cause) - now open as #12333
The catalog-side derivation is no longer deferred. #12333 replaces
`columnType.toLowerCase(Locale.ROOT).contains("unsigned")` with a shared
`JdbcCatalogUtils.isNumericUnsignedColumnType(...)` that only treats a column
as unsigned when its leading type name is one of the numeric types the
converters actually describe with a `*_UNSIGNED` constant. It fixes both
`MySqlCatalog` and `OceanBaseMySqlCatalog`, which carried the identical bug,
and it closes the `ENUM UNSIGNED` sibling case that @SEZ9 and I both flagged.
So the relationship between the two pull requests is now: #12333 removes the
reason a synthetic `SET UNSIGNED` name is ever produced; this pull request
keeps handling one defensively if it is supplied from outside. Neither depends
on the other.
## Issue 2 (E2E) - the end-to-end case lives in #12333
#12333 adds an E2E case that drives real catalog discovery over a `SET` /
`ENUM` column whose value list contains the word "unsigned", which is exactly
the path that synthesizes the name, and it asserts `STRING` plus the
still-detected `INT UNSIGNED` control.
For this pull request's own CDC-side branch I kept the unit test coverage,
for one reason worth stating explicitly: searching this repository, the only
place the exact string `SET UNSIGNED` is produced is
`MySqlTypeConverter.convert()`, when it appends the suffix for `unsigned ==
true`. I could not find a Debezium path that yields `Column.typeName() == "SET
UNSIGNED"`, so that branch reads as defensive rather than as a path with a real
producer. If you would still prefer an IT here that drives the synthetic input
directly (rather than relying on #12333's catalog-level case), say so and I
will add it.
## Issue 3 (bare `SET` column type on the CDC path)
Agreed this is pre-existing and out of scope here: the switch in
`MySqlTypeUtils` has no case that reconstructs the `SET(...)` value list, so a
CDC-sourced `SET` column already ends up with a bare `SET` type for any
downstream auto-create DDL, independent of the `UNSIGNED` suffix. I have not
touched it. Happy to file a tracking issue if you think it is worth one.
--
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]
