SEPURI-SAI-KRISHNA opened a new pull request, #13074:
URL: https://github.com/apache/seatunnel/pull/13074
Yes, glad to contribute to Apache SeaTunnel.
### Purpose of this pull request
Fixes #13071.
The `schema` option was parsed with case conversions that use the JVM
default locale. Under `tr-TR` and `az-AZ`, `i` upper cases to `İ` (U+0130), so
lowercase values containing an `i` matched nothing:
- **Field types.** `string`, `int`, `bigint`, `timestamp`, `decimal(10, 2)`
and the arrays and maps built from them failed with `COMMON-07 unsupported data
type`.
- **Vector index keys.** `indexType` and `metricType` values such as
`ivf_flat`, `inverted`, `ip` and `cosine` were rejected, although the schema
documentation says both are case insensitive.
`ReadonlyConfigParser` handles the `schema` option of every connector that
has one, so such jobs failed at startup on those workers.
All `toUpperCase()` calls on this path now use `Locale.ROOT`:
- `SeaTunnelDataTypeConvertorUtil`:
- `:50` the `SqlType.valueOf` lookup;
- `:130` the `MAP`, `ARRAY` and `DECIMAL` prefix checks;
- `:196` the decimal map key check;
- `:117` the `long`, `short` and `byte` aliases. These contain no `i`, so
this one never failed. It is changed so that no default locale conversion is
left in the class.
- `VectorIndex`: `IndexType.of` and `MetricType.of`.
### Does this PR introduce any user facing change?
Yes, only on workers whose default locale is Turkish or Azerbaijani: values
that failed there now parse. I compared `dev`'s classes with these on 125 type
declarations and 81 vector index values, each in lowercase, upper case and
capitalised form:
- `tr-TR` and `az-AZ`: 38 declarations and 25 index values that failed now
parse. No value that parsed 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 signature changed.
### How was this patch tested?
Two tests, each in the existing test class:
-
`SeaTunnelDataTypeConvertorUtilTest.testParseDoesNotDependOnTheDefaultLocale`
parses `string`, `int`, `timestamp`, `decimal(10, 2)`, `array<int>` and
`map<decimal(10, 2), string>`.
-
`ReadonlyConfigParserTest.parseVectorIndexKeyDoesNotDependOnTheDefaultLocale`
parses a vector index key with `ivf_flat` and `cosine`. The existing case
insensitivity test uses `hnsw` and `l2`, which contain no `i`.
Both run under `tr-TR` and `az-AZ`, and restore the base locale and the
`FORMAT` and `DISPLAY` defaults afterwards. Both fail with `dev`'s classes.
Reverting the calls one at a time, `:50`, `:130` and `:196` each fail the first
test.
I also ran the whole `seatunnel-api` suite in a JVM started with
`-Duser.language=tr -Duser.country=TR`. On `dev`, 13 existing schema parsing
tests fail, for example `CatalogTableUtilTest.testSimpleSchemaParse`. With this
change, 11 of them pass. The other 2, `ReadonlyConfigParserTest.parseColumn`
and `parseField`, now parse correctly but fail on the test's own
`toLowerCase()` of the expected type name. The default CI locale is unaffected,
so I left those assertions alone. The run also shows 3 failures in
`GravitinoTableSchemaConvertorTest`. That is the metalake type conversion, a
separate code path not touched here.
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-api`: 412 tests, 0 failures.
- `./mvnw spotless:check -pl seatunnel-api`: 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 #10221 and #11493 also edit `SeaTunnelDataTypeConvertorUtil` and
its test. A three-way merge with either one 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. The documented type names and index values now parse in
every locale, as the documentation already says.
* [X] If you are contributing the connector code, please check that the
following files are updated: not applicable, no connector change.
* [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]
