Repository navigation
[Fix][Connector-V2][JDBC] Read MySQL and OceanBase column types independently of the default locale - #13076
[Fix][Connector-V2][JDBC] Read MySQL and OceanBase column types independently of the default locale#13076SEPURI-SAI-KRISHNA wants to merge 2 commits into
Conversation
…endently of the default locale
The JDBC dialect type converters convert the source type name with String#toUpperCase() / String#toLowerCase() without a locale before matching it against the dialect type constants. Under a Turkish or Azerbaijani default locale a lower-case "integer" upper-cases to "INTEGER" with a dotted capital I and an upper-case "INT4" lower-cases to "int4" with a dotless i, so the type no longer matches any constant and the converter falls through to its unsupported-type branch. Pass Locale.ROOT to every remaining case conversion in connector-jdbc so that the same source type maps to the same SeaTunnel type on any worker. The MySQL, OceanBase and SQL Server converters, JdbcDialect and CatalogUtils are already covered by apache#13063, apache#13076 and apache#13077 and are not touched here.
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for this focused fix! I read the complete eight-file diff and the catalog/dialect callers at 5c29f67137db482a52f8ddbef237e7630aeb4098. The production normalization is correct; one integration-coverage requirement remains.
What Problem Does This PR Solve?
User pain point: MySQL/OceanBase report lowercase type names. Under tr-TR/az-AZ, default-locale uppercasing turns int/datetime into tokens with dotted I, so normal catalog discovery rejects an otherwise supported column.
Fix approach: use Locale.ROOT in both catalogs and both type converters, including OceanBase's vector column-type normalization; remove its now-redundant second uppercase call.
One-sentence summary and example: the same MySQL int unsigned column maps to LONG under Turkish as it already does under English, instead of failing schema discovery.
1. Code Change Review
1.1 Core Logic Analysis
Production locations, all under seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/: catalog/mysql/MySqlCatalog.java:225, catalog/oceanbase/OceanBaseMySqlCatalog.java:146-151, internal/dialect/mysql/MySqlTypeConverter.java:199, and internal/dialect/oceanbase/OceanBaseMySqlTypeConverter.java:126,304.
Before/after:
// before, in catalogs and converters
String dataType = rawType.toUpperCase();
// after
String dataType = rawType.toUpperCase(Locale.ROOT);
// OceanBase: already normalized above, so startsWith("VECTOR") needs no second conversionAffected complete chain:
JDBC source/sink factory -> selected MySQL/OceanBase catalog
-> AbstractJdbcCatalog table/column discovery
-> buildColumn(ResultSet)
-> read DATA_TYPE, original COLUMN_TYPE, precision/scale/default/nullability
-> ROOT normalize DATA_TYPE
-> isNumericUnsignedColumnType(DATA_TYPE, COLUMN_TYPE)
-> BasicTypeDefine -> corresponding TypeConverter.convert
-> ROOT normalize -> unsigned/zerofill handling -> existing type switch
-> PhysicalColumn -> CatalogTable -> source schema / sink preparation
Dialect/TypeMapper and MysqlDataTypeConvertor callers also use the corrected converter.
Reverse DDL conversion and the row writer/checkpoint lifecycle are unchanged.
Key findings: normal discovery reaches the fix; affected locale-specific machine tokens are now stable; raw sourceType/COLUMN_TYPE values remain preserved; vector dimensions and numeric mappings are not redesigned. This is a precise boundary correction, not a global locale workaround. The offline catalogs exercise real buildColumn logic, although their ResultSet values are mocked.
1.2 Compatibility Impact
Fully compatible with the intended supported-type contract. No API, configuration key/default, protocol, serialized schema or checkpoint format changes. Unsigned detection still examines the original column declaration through the existing helper. No migration is required.
1.3 Performance / Side-Effect Analysis
Normalization already occurred during metadata preparation, so CPU, memory and GC complexity do not grow; one redundant conversion is removed. No new network operation, lock, queue, cache, retry, shared production state or resource owner. Per-record idempotence and writer release are unaffected.
1.4 Error Handling and Logging
Unsupported types retain existing contextual errors. There is no added logging or credential exposure. The MySQL test deliberately fails connection acquisition in its override; the new OceanBase catalog constructor only initializes the converter and does not open a connection.
Issue 1: Extend the existing MySQL/OceanBase integration coverage for locale-independent discovery
- Location:
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/catalog/mysql/MySqlCatalogUnsignedDetectionTest.java:123;catalog/oceanbase/OceanBaseMySqlCatalogTest.java:45in the same test source root. - Problem description: All new coverage is offline. The connector already has JdbcMysqlIT and JdbcOceanBaseMysqlIT, but neither fixture is updated. Mocked DATA_TYPE/COLUMN_TYPE strings do not pin the driver's real metadata-to-job boundary under the affected default locale.
- Potential risk: A future integration-path regression can escape these direct catalog/converter assertions. This is a coverage requirement, not a demonstrated defect in the ROOT changes.
- Best improvement: Add one concise locale-sensitive real catalog/schema assertion to the existing relevant fixtures, preserving unsigned behavior. Apply the locale in the discovery JVM and restore it; do not create a broad duplicate matrix or a new database harness.
- Severity: Medium; blocking coverage requirement.
- Raised by another reviewer: No; retrieved review/inline history contains no existing findings to duplicate.
2. Code Quality Assessment
2.1 Coding Standards
The changes are minimal and use existing Locale imports where available. The new OceanBase test has ASF header/class/method explanation. No new production core method/state field requires a missing contract comment. Existing ENUM/SET/unsigned regression cases remain intact.
2.2 Test Coverage and Test Stability
Stability: Stable in the present sequential JUnit setup. All four additions save/restore base, FORMAT and DISPLAY locales in finally. Fixed input/result assertions cover int, unsigned int and datetime; no sleep, clock, random port, external service or order-sensitive collection assertion. The MySQL test probes no live database, and OceanBase construction does not connect. Enabling parallel JVM-locale tests would require resource coordination. Issue 1 covers the missing IT extension.
The exact-head fork Build and Apache Build are successful; all executed job conclusions are successful. This is CI evidence for the submitted selection, not proof of a non-English live database scenario absent from the diff.
Local evidence: fetched/checked out independent branch seatunnel-review-13076, read all eight changed files plus buildColumn/unsigned/dialect/type-mapper callers with git show/grep; git diff --check passed against merge base f3e1c2ee3b83efd4a1e7fb4cc4fe40b44060c7d0. No local build, test, database, container or runtime execution.
2.3 Documentation Updates
Supported type names, options and defaults remain unchanged. Existing EN/ZH JDBC configuration is still accurate; no new setup/migration recipe is necessary. A brief bug-fix note can mention locale independence without advertising additional types.
3. Architectural Soundness
3.1 Elegance of the Solution
Precise fix. Metadata catalogs and converters own machine-token parsing. Changing JVM-wide defaults would affect unrelated connectors and is not an acceptable substitute.
3.2 Maintainability
Keep the existing switches/unsigned helper and close the small IT gap; no broader catalog refactor is needed.
3.3 Extensibility
The pattern applies to other machine-defined token normalizers; user names and values must retain their own semantics.
3.4 Historical-Version Compatibility
No stored state, option or public method migration. This review does not certify a live rolling-upgrade execution.
4. Issue Summary
| Number | Issue | Location | Severity |
|---|---|---|---|
| 1 | Existing MySQL/OceanBase IT fixtures do not pin the affected locale path | MySqlCatalogUnsignedDetectionTest.java:123; OceanBaseMySqlCatalogTest.java:45 |
Medium |
5. Merge Recommendation
Conclusion: Ready to merge after fixes
- Blockers — must be fixed
- Issue 1: add minimal integration coverage, then retain green required CI on the new head.
- Recommended fixes — non-blocking
- No unrelated refactor, dependency change or branch-age-only synchronization is requested.
Overall: the production fix is sound and correctly located. Existing fixtures are the smallest safe way to close the coverage requirement. No maintainer approval/change request exists in the retrieved history to reconcile. Daniel has READ permission; this COMMENT is the technical conclusion, while effective approval and merge gates remain with a write-capable maintainer.
Snapshot 2026-11-01T11:47:24.438719+00:00: head 5c29f67137db482a52f8ddbef237e7630aeb4098, base dev; compare diverged, ahead 1 / behind 4; merge blocked / BLOCKED, review decision REVIEW_REQUIRED. Drift is recorded separately; it does not itself request synchronization.
Drop MySqlCatalog, OceanBaseMySqlCatalog and SqlServerURLParser: those files are fixed by apache#13076 and apache#13077, and the OceanBase edit conflicts with the removal of the second toUpperCase() there, so leave them to those PRs. Add RedshiftCatalogLocaleTest, which drives RedshiftCatalog.buildColumn with a mocked result set under tr-TR, so catalog schema discovery is covered and not only the converters. Extend CharsetBasedSplitterTest with an I/i case that asserts the case insensitive fold of upper case I is i and that the encoded range decodes back to i. Restore the base, DISPLAY and FORMAT locale defaults and mark the locale mutating tests with @ResourceLock, so they cannot leak state into other tests.
…e catalog under a Turkish locale in the ITs
|
@DanielLeens thanks for the review. Issue 1 is done in the latest commit. I checked that both detect the bug, against MySQL 8.0.43 and OceanBase CE 4.4.2.1. With this PR both pass. With the four changed classes as they are on |
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. I re-reviewed the current head 4f34fad2f1943c29f60f03da2f1d074d694e6b9c. The only change since my previous review (head 5c29f67137) is two new integration tests, JdbcMysqlIT.testCatalogReadsColumnTypesUnderTurkishLocale and JdbcOceanBaseMysqlIT.testCatalogReadsColumnTypesUnderTurkishLocale, which close the integration-coverage request I raised as Issue 1. The production changes are unchanged and I still find no functional defect in them.
What Problem Does This PR Solve?
User pain point: MySQL and OceanBase report DATA_TYPE in lower case. Under tr-TR / az-AZ the default-locale toUpperCase() turns int into a token with a dotted capital I, so catalog discovery fails with COMMON-17 unsupported convert type and a JDBC source using table_path cannot read almost any MySQL table.
Fix approach: use Locale.ROOT in MySqlCatalog, OceanBaseMySqlCatalog, MySqlTypeConverter and OceanBaseMySqlTypeConverter, and drop a redundant second upper-casing in the OceanBase catalog.
One-sentence summary: the same MySQL int unsigned column maps to LONG under a Turkish JVM exactly as it does under English.
1. Code Change Review
1.1 Core Logic Analysis
Before / after:
resultSet.getString("DATA_TYPE").toUpperCase() -> .toUpperCase(Locale.ROOT) // MySqlCatalog:225, OceanBaseMySqlCatalog:146
typeDefine.getDataType().toUpperCase() -> .toUpperCase(Locale.ROOT) // MySqlTypeConverter:199, OceanBaseMySqlTypeConverter:126
dataType.toUpperCase().startsWith("VECTOR") -> dataType.startsWith("VECTOR") // value already upper caseRuntime path (read from the callers in this checkout):
JDBC source/sink with table_path
-> MySqlCatalog / TiDBCatalog / OceanBaseMySqlCatalog.getTable
-> buildColumn(ResultSet): DATA_TYPE normalized with ROOT -> unsigned detection (uses original COLUMN_TYPE)
-> BasicTypeDefine -> MySqlTypeConverter / OceanBaseMySqlTypeConverter.convert (ROOT again) -> existing switch
-> CatalogTable -> source schema / sink DDL
Key findings:
- The normal catalog-discovery path reaches the fix, not only a recovery path.
TiDBCataloginheritsMySqlCatalog.buildColumn, so TiDB is fixed as well. - I grepped the MySQL, TiDB and OceanBase catalog packages and the MySQL/OceanBase dialect packages. The remaining default-locale conversions are
MysqlDialect:208/OceanBaseMysqlDialect:207(toLowerCase().contains("where"), noi, locale-neutral) andOceanBaseOracleCreateTableSqlBuilder:49(Oracle mode, outside this PR). - Removing the second
toUpperCase()inOceanBaseMySqlCatalogis safe because the value is already upper case. - The PR description correctly scopes out MySQL CDC (types arrive from Debezium's own parser) and the query path (Connector/J already reports upper case).
- Solution type: Precise fix.
1.2 Compatibility Impact
Fully compatible. No option, default, API/SPI, protocol, serialization or checkpoint change. Only inputs that failed under Turkish/Azerbaijani locales change; results under other locales are identical. No incompatible-changes.md entry is needed.
1.3 Performance / Side-Effect Analysis
Metadata-time only, one normalization per column as before (one fewer in the OceanBase catalog). No per-record work, thread, lock, connection or buffer is added.
1.4 Error Handling and Logging
Unsupported-type errors are unchanged. No new logging or credential exposure. No new issues found in this round.
2. Code Quality Assessment
2.1 Coding Standards
Minimal and consistent with existing style. No new production method or field, so no new Javadoc is required. New tests carry explanatory Javadoc.
2.2 Test Coverage and Test Stability
Stability rating: Stable.
- Unit tests (
MySqlTypeConverterTest,OceanBaseMySqlTypeConverterTest,MySqlCatalogUnsignedDetectionTest,OceanBaseMySqlCatalogTest) use fixed inputs and restore the base locale and both category defaults infinally. - New ITs,
JdbcMysqlIT.java:633-692andJdbcOceanBaseMysqlIT.java:343-402: create a table withint,int unsigned,bigint,datetime, switch the JVM default totr-TRthenaz-AZ, read the table through the catalog the IT already opens, and assert exact types (INT, LONG, LONG, LOCAL_DATE_TIME). Locale is restored and the table dropped infinally. They are plain@Testmethods with no sleeps or timing. I checked that neither IT closes the sharedcatalogin an earlier test. Reverting theLocale.ROOTchanges would make the type assertions fail with the dotted-I names, so the tests have failure power. The contributor reports running both against MySQL 8.0.43 and OceanBase CE 4.4.2.1 with and without the fix; I did not run them.
2.3 Documentation Updates
No user-visible option or contract change; no EN/ZH doc update is needed.
3. Architectural Soundness
3.1 Elegance of the Solution
Precise fix at the metadata-token boundary in each affected class.
3.2 Maintainability
Small, mechanical, easy to audit.
3.3 Extensibility
Locale.ROOT is the right default for machine-defined tokens; other dialects with the same pattern can follow separately.
3.4 Historical-Version Compatibility
Existing jobs keep configuration and state formats; no checkpoint or serialization effect.
4. Issue Summary
| Number | Issue | Location | Severity |
|---|---|---|---|
| - | No open issues. Previous Issue 1 (IT coverage) is resolved by the two new ITs. | - | - |
5. Merge Recommendation
Conclusion: Ready to merge
- Blockers
- None on the source side.
- Recommended non-blocking fixes
- None.
CI state (checked at head 4f34fad2f1): the apache Build check is in_progress and the matching fork run is queued (run 41410003336). Please let it complete; the new ITs only get real CI evidence once the JDBC part-1 and part-2 lanes run. The branch is diverged by 4 commits behind dev with no conflict reported; I am not asking for a sync. Effective maintainer approval is a separate repository gate.
Overall assessment: the fix is correct and complete for the MySQL, TiDB and OceanBase (MySQL mode) catalog path, and the requested integration coverage is now present. I did not run any build or test locally; the conclusions come from reading the diff and callers.
DanielLeens
left a comment
There was a problem hiding this comment.
CI status update for head 4f34fad2f1: when I last reviewed this head, the current-head Build was still running, so I could not call CI yet. That run has now finished with a failure, so the CI fact in my previous comment is out of date. My source-side conclusion is unchanged.
What failed in the fork run (https://github.com/SEPURI-SAI-KRISHNA/seatunnel/actions/runs/41410003336): Run / edge-agent-it (11, ubuntu-latest). Specifically, the job failed while building/testing seatunnel-engine-server: JobHistoryServiceTest errored in AbstractSeaTunnelServerTest.before:73 with IllegalStateException (the Hazelcast test node did not start, after the 300 s limit).
That is an engine-server test bootstrap failure; it does not touch the MySQL/OceanBase catalog code this PR changes, so I do not attribute it to this diff. The branch already contains the related upstream Hazelcast join-port fix (#11868), so syncing with dev again would not change anything here.
The lowest-cost next step is to re-run only the failed jobs in your fork run ("Re-run failed jobs" on the run page). If the same test fails again on the re-run, please paste the new run link here and I will help narrow it down; please do not add unrelated changes to this PR just to chase it. Thanks again for your patience!
|
Thanks @DanielLeens. The run you looked at was already the re-run of the failed jobs: https://github.com/SEPURI-SAI-KRISHNA/seatunnel/actions/runs/41410003336
The same job passed on #13077 (run 41487880643), whose branch has the same |
|
@DanielLeens as promised, the result of the extra re-run: attempt 3 of https://github.com/SEPURI-SAI-KRISHNA/seatunnel/actions/runs/41410003336 is green on the unchanged head
|
…string split IT The IT only compared the rows it read, which cannot observe the locale bug: the first split still starts at the literal MIN and the last one still ends at the literal MAX, so every row is read exactly once whatever the encoding is. Under tr-TR the interior bounds decode to strings that start with the U+0001 placeholder instead of "i" - that is the difference a caller actually sees. Generate the splits once under Locale.ROOT and once under tr-TR and compare the boundaries. Additionally: - pin split.string_split_mode_collate, because isCaseInsensitive is derived from the connection collation (JdbcDialect#getCollationSequence) and, when the option is unset, a server default that does not tie A with a leaves the lower-casing branch untested; - assert that interior bounds are already folded, so the fixture fails loudly if that branch is ever skipped instead of silently going inert; - build the catalog table before switching the locale, so the test does not depend on MySqlCatalog from apache#13076.
DanielLeens
left a comment
There was a problem hiding this comment.
Thank you, @SEPURI-SAI-KRISHNA, for following through on the rerun and for posting the result. On the unchanged head 4f34fad2f1, attempt 3 of the fork run is green, edge-agent-it (11, ubuntu-latest) passed with JobHistoryServiceTest at 2 tests / 0 failures, and the apache Build check is now green. That confirms the earlier failure was the engine-server Hazelcast node-startup flake and not this change.
My source-side conclusion is unchanged: the Locale.ROOT fix in MySqlCatalog, OceanBaseMySqlCatalog, MySqlTypeConverter and OceanBaseMySqlTypeConverter is correct, and the two new ITs (JdbcMysqlIT and JdbcOceanBaseMysqlIT#testCatalogReadsColumnTypesUnderTurkishLocale) have real failure power. CI was the only remaining gate, so I am approving this head. Effective maintainer approval is still a separate repository gate. Thanks again!
Yes, glad to contribute to Apache SeaTunnel.
Purpose of this pull request
Fixes #13073.
MySqlCatalog.buildColumnupper casedDATA_TYPEwith the JVM default locale. MySQL reportsDATA_TYPEin lower case (int,datetime), and undertr-TRandaz-AZiupper cases toİ. SointbecameİNT, matched no type, and reading the table failed withCOMMON-17 unsupported convert type 'İNT'. That breaks a JDBC source withtable_pathfor almost any MySQL table on such a worker.TiDBCatalogextendsMySqlCatalog, andOceanBaseMySqlCataloghas the same code.The case conversions on this path now use
Locale.ROOT:MySqlCatalog:225andOceanBaseMySqlCatalog:146,DATA_TYPE. These are the calls that broke real jobs.MySqlTypeConverter:199andOceanBaseMySqlTypeConverter:126. The callers in this repository pass these converters an already upper cased name, but they are publicTypeConverters and their own tests call them with lowercase names. In a Turkish JVM, 15 of the existing tests inMySqlTypeConverterTestfail ondev.OceanBaseMySqlTypeConverter:304, which parsesvector(N). It contains noi, so this one never failed. It is changed so that no default locale conversion is left on the path.OceanBaseMySqlCatalog:151upper caseddataTypea second time. That call is removed, since the value is already upper case.Not changed here:
querypath. MySQL Connector/J reportsgetColumnTypeNamein upper case (MysqlType.getName()), so it was not affected.İNTfrom 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-TRandaz-AZ: 106 inputs that failed now convert. No input that converted before gives a different result, and every result now equals theROOTone.ROOT: identical results before and after.So no working job changes, and no entry in
incompatible-changes.mdis needed. No option, default or API changed.How was this patch tested?
Tests, each running under
tr-TRandaz-AZand restoring the base locale and theFORMATandDISPLAYdefaults:MySqlCatalogUnsignedDetectionTest.shouldReadLowerCaseDataTypeUnderAnyDefaultLocale:buildColumnwithDATA_TYPEin lower case, as the server reports it (int,intwithint(10) unsigned,datetime). The existing tests in this class mockDATA_TYPEas"INT", which is why they pass in a Turkish JVM.MySqlTypeConverterTest.testConvertDoesNotDependOnTheDefaultLocale.OceanBaseMySqlTypeConverterTest.testConvertDoesNotDependOnTheDefaultLocale.OceanBaseMySqlCatalogTest.shouldReadLowerCaseDataTypeUnderAnyDefaultLocale. This class is new, because no test coveredOceanBaseMySqlCatalog. Its constructor does not connect, so it runs offline.Reverting the four
DATA_TYPEcalls one at a time, each fails its own test.I also ran the whole
connector-jdbcsuite 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 thefield_idelookup that #13063 fixes.The existing ITs cover it on a real server too.
JdbcMysqlIT.testCatalogReadsColumnTypesUnderTurkishLocaleandJdbcOceanBaseMysqlIT.testCatalogReadsColumnTypesUnderTurkishLocalecreate a table withint,int unsigned,bigintanddatetimecolumns and read it through the catalog undertr-TRandaz-AZ. The catalog runs in the test JVM, so that is where the locale is set, and it is restored infinally. I ran both test methods against MySQL 8.0.43 (the IT image) and OceanBase CE 4.4.2.1 (the IT'slatestimage today). With this change they pass. With the four classes as they are ondev, both fail withCOMMON-20 ... {"c_int":"İNT","c_int_unsigned":"İNT","c_bigint":"BİGİNT","c_datetime":"DATETİME"}, and the default locale is restored after the failure.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 spotless:check -pl seatunnel-e2e/seatunnel-connector-v2-e2e/connector-jdbc-e2e/connector-jdbc-e2e-part-1,seatunnel-e2e/seatunnel-connector-v2-e2e/connector-jdbc-e2e/connector-jdbc-e2e-part-2: 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
License Notice: none added.
JdbcMysqlITandJdbcOceanBaseMysqlIT, see above.release-note: not applicable, the file was removed fromdevin [Improve] Remove release-note.md in dev branch #9304.