raminqaf opened a new pull request, #28919: URL: https://github.com/apache/flink/pull/28919
## Brief change log
- `BinaryVariantUtil.getString` decodes string values as UTF-8 instead of
using the JVM default charset
- `BinaryVariantUtil.getMetadataKey` decodes object field names the same
way
- Added round-trip coverage for non-ASCII string values and object field
names
## Verifying this change
This change added tests and can be verified as follows:
- `BinaryVariantTest.testNonAsciiStringsAndFieldNames` builds an object
with non-ASCII field names and values, then re-reads it through
`new BinaryVariant(value, metadata)`. That is the path taken once a variant
has been serialized, and the only one that decodes field names
from the metadata dictionary. It asserts `getFieldNames()`, `getField(...)`,
`getString()` and `toJson()`. The values cover both string encodings, one short
and one past `MAX_SHORT_STR_SIZE` so `LONG_STR` is exercised too.
-
`BinaryVariantInternalBuilderTest.testParseJsonWithNonAsciiStringsAndKeys`
covers the `PARSE_JSON` path with non-ASCII keys and values, and asserts the
document round-trips byte for byte.
- Red-green verified. With the fix reverted, both tests fail on Temurin 17
under `-Dfile.encoding=ISO-8859-1`, and pass with the fix applied. On a UTF-8
JVM they pass either way. That is inherent to the bug rather than a gap in the
tests, since JEP 400 makes UTF-8 the default from Java 18 on.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): **no**
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: **no**. `BinaryVariantUtil` is `@Internal`. The observable
behaviour of `@PublicEvolving Variant` does change, but only so that non-ASCII
text decodes correctly. No signatures change.
- The serializers: **no**. `VariantSerializer` writes and reads the raw
`value` and `metadata` byte arrays, and the binary layout is untouched, so
existing state stays readable. Nothing was corrupted at rest either. The writer
always encoded UTF-8, so only the read path was affected.
- The runtime per-record code paths (performance sensitive): **yes**, in
the sense that `getString` and `getMetadataKey` run per record for VARIANT
access. Naming the charset adds no work. On Java 18+ it selects the same
decoder the JDK already picked implicitly.
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: **no**
- The S3 file system connector: **no**
## Documentation
- Does this pull request introduce a new feature? **no**
- If yes, how is the feature documented? **not applicable**
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Claude Opus 5)
--
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]
