raminqaf opened a new pull request, #29607: URL: https://github.com/apache/flink/pull/29607
> Builds on #29580 (FLINK-41110). Its commit is the first one here. Please review only the last two commits: [the diff on top of #29580](https://github.com/raminqaf/flink/compare/1b0a0eb903a...c38a2430629). ## What is the purpose of the change The Variant spec sorts the fields of an object by the unsigned UTF-8 bytes of their names. Flink sorted and binary-searched them with `String.compareTo`, which is UTF-16 order. The two orders disagree when one name has a character from U+E000 to U+FFFF and the other one from U+10000 up: | Key | UTF-8 | UTF-16 | |--------------|---------------|-------------| | `｡` U+FF61 | `EF BD A1` | `FF61` | | `😀` U+1F600 | `F0 9F 98 80` | `D83D DE00` | Take an object with the keys `k00` to `k29`, `｡` and `😀`, so 32 fields. From 32 fields on, `getField` binary-searches: | Case | Before | After | |-------------------------------------------------------------|-----------------------|--------------------------------------| | Field order the builder writes | `..., 😀, ｡` | `..., ｡, 😀`, the spec order | | `getField("😀")` on a spec-ordered object, e.g. from another engine | `null` | found | | `getField("😀")` / `getField("｡")` on an object older Flink wrote | found | found | | `getField("a\uD800")`, stored as `a?`, in 32+ fields | `null` | found | | `{"k?": 1, "k\uD800": 2}`, duplicates not allowed | two fields named `k?` | fails with `VARIANT_DUPLICATE_KEY` | ## Brief change log - The builder sorts fields by their stored UTF-8 key bytes. Keys stored as the same bytes are duplicates. - `getField` binary-searches in byte order. A miss for a key with a code point from U+E000 up falls back to a linear scan, so objects in UTF-16 order stay readable. For any other key both orders agree, so a miss is final. - `[hotfix][docs]` syncs the zh VARIANT section, which is kept in English, with the en page. The FLINK-41180 commit then states the key order on both pages. ## Verifying this change This change added tests to `BinaryVariantTest`: - Field order of a builder-written object, which fails before the change. - Lookups in a 32-field object in both byte order and UTF-16 order. The fixture covers a 2-byte, a 3-byte, a U+E000 and a 4-byte key, so a signed compare or a fallback threshold of `0xEF` fails it. - Unpaired surrogate lookup at 1 and 32 fields. - Duplicate keys with the same UTF-8 bytes, with the field order asserted after compaction. ## Trade-offs - The fallback scans all fields, because an object doesn't record which order it uses. Only misses for keys from U+E000 up pay for it, in objects of 32+ fields. A rough nanoTime loop, not JMH, on 1000 fields: an absent emoji key went from 112 ns to 4988 ns. An absent ASCII key went from 120 ns to 79 ns, because the binary search no longer decodes a String per probe. A follow-up can replace the scan with a second binary search in UTF-16 order, as Spark does in SPARK-58949. - Compatibility: values that older versions wrote stay readable. An older version can miss keys from U+E000 up in objects of 32+ fields that this version writes. `BinaryVariant.equals` compares bytes, so an object that mixes the two key ranges and was stored before the upgrade is not equal to the same object built after it. The release note on FLINK-41180 says this. - This targets master (2.5). No backport proposed. ## 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)`: yes, behavior only. `BinaryVariantBuilder` treats keys with the same UTF-8 bytes as duplicates. No signature changes. - The serializers: no - The runtime per-record code paths (performance sensitive): yes, `getField` and object building. - 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. `data-types.md` (en and zh) now names the key order. --- ##### Was generative AI tooling used to co-author this PR? - [X] Yes (please specify the tool below) Generated-by: Claude Code 2.1.310 (Claude Opus 5.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]
