SEPURI-SAI-KRISHNA opened a new pull request, #13076:
URL: https://github.com/apache/seatunnel/pull/13076
Yes, glad to contribute to Apache SeaTunnel.
### Purpose of this pull request
Fixes #13073.
`MySqlCatalog.buildColumn` upper cased `DATA_TYPE` with the JVM default
locale. MySQL reports `DATA_TYPE` in lower case (`int`, `datetime`), and under
`tr-TR` and `az-AZ` `i` upper cases to `İ`. So `int` became `İNT`, matched no
type, and reading the table failed with `COMMON-17 unsupported convert type
'İNT'`. That breaks a JDBC source with `table_path` for almost any MySQL table
on such a worker. `TiDBCatalog` extends `MySqlCatalog`, and
`OceanBaseMySqlCatalog` has the same code.
The case conversions on this path now use `Locale.ROOT`:
- `MySqlCatalog:225` and `OceanBaseMySqlCatalog:146`, `DATA_TYPE`. These are
the calls that broke real jobs.
- `MySqlTypeConverter:199` and `OceanBaseMySqlTypeConverter:126`. The
callers in this repository pass these converters an already upper cased name,
but they are public `TypeConverter`s and their own tests call them with
lowercase names. In a Turkish JVM, 15 of the existing tests in
`MySqlTypeConverterTest` fail on `dev`.
- `OceanBaseMySqlTypeConverter:304`, which parses `vector(N)`. It contains
no `i`, so this one never failed. It is changed so that no default locale
conversion is left on the path.
- `OceanBaseMySqlCatalog:151` upper cased `dataType` a second time. That
call is removed, since the value is already upper case.
Not changed here:
- The `query` path. MySQL Connector/J reports `getColumnTypeName` in upper
case (`MysqlType.getName()`), so it was not affected.
- MySQL CDC. Under a Turkish locale the column types arrive as `İNT` from
Debezium's own DDL parser (`ColumnDefinitionParserListener`), before SeaTunnel
code sees them. That needs a separate change.
### Does this PR introduce any user facing change?
Yes, only on workers whose default locale is Turkish or Azerbaijani: MySQL,
TiDB and OceanBase (MySQL mode) tables can be read through the catalog there. I
compared `dev`'s converters with these on 270 inputs (every MySQL and OceanBase
type name in lower, upper and capitalised form, through both converters):
- `tr-TR` and `az-AZ`: 106 inputs that failed now convert. No input that
converted before gives a different result, and every result now equals the
`ROOT` one.
- `ROOT`: identical results before and after.
So no working job changes, and no entry in `incompatible-changes.md` is
needed. No option, default or API changed.
### How was this patch tested?
Tests, each running under `tr-TR` and `az-AZ` and restoring the base locale
and the `FORMAT` and `DISPLAY` defaults:
-
`MySqlCatalogUnsignedDetectionTest.shouldReadLowerCaseDataTypeUnderAnyDefaultLocale`:
`buildColumn` with `DATA_TYPE` in lower case, as the server reports it (`int`,
`int` with `int(10) unsigned`, `datetime`). The existing tests in this class
mock `DATA_TYPE` as `"INT"`, which is why they pass in a Turkish JVM.
- `MySqlTypeConverterTest.testConvertDoesNotDependOnTheDefaultLocale`.
-
`OceanBaseMySqlTypeConverterTest.testConvertDoesNotDependOnTheDefaultLocale`.
-
`OceanBaseMySqlCatalogTest.shouldReadLowerCaseDataTypeUnderAnyDefaultLocale`.
This class is new, because no test covered `OceanBaseMySqlCatalog`. Its
constructor does not connect, so it runs offline.
Reverting the four `DATA_TYPE` calls one at a time, each fails its own test.
I also ran the whole `connector-jdbc` suite in a JVM started with
`-Duser.language=tr -Duser.country=TR`. With this change, 20 existing tests
that failed there pass, all in the MySQL and OceanBase classes, and none starts
failing. The ones left are other dialects (DM, SQL Server, YashanDB, IRIS,
Xugu, Oracle and others), which I would handle separately, and the `field_ide`
lookup that #13063 fixes.
There is no E2E change. The failure depends on the default locale of the
engine JVM, and the E2E suites run the engine in containers with their own
default locale.
Checks, JDK 17:
- `./mvnw clean test -pl seatunnel-connectors-v2/connector-jdbc`: 1200
tests, 0 failures, 4 errors. The 4 are the Oracle and Kingbase container tests,
which need Docker.
- `./mvnw spotless:check -pl seatunnel-connectors-v2/connector-jdbc`: clean.
- `./mvnw -B -T 1 clean test -D"license.skipAddThirdParty"=true -pl
seatunnel-ci-tools`: 34 tests, 0 failures, 2 skipped by the module.
Open PRs #13070, #12853, #12153, #11971 and #11848 also edit these files. A
three-way merge with each is conflict free.
### Check list
* [X] Code changed are covered with tests, or it does not need tests for
reason
* [X] If any new Jar binary package adding in your PR, please add License
Notice according to
[License
Notice](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md):
none added.
* [X] If necessary, please update the documentation to describe the new
feature: not needed, no documented behaviour changes.
* [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):
not needed, no new connector.
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml):
not needed, no new module.
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml):
not needed, connector-jdbc is already labelled.
4. Add e2e test for your connector: not needed, see above.
5. Update [connector-v2
doc](https://github.com/apache/seatunnel/tree/dev/docs/en/connectors): not
needed, no option or mapping changes.
* [X] Update the `release-note`: not applicable, the file was removed from
`dev` in #9304.
--
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]
