raminqaf opened a new pull request, #29580:
URL: https://github.com/apache/flink/pull/29580
## What is the purpose of the change
`Variant.getField` decoded every metadata key it compared into a new
`String`. `v['a']` in SQL calls it once per record, so one lookup in a 13-key
object could create 13 throwaway strings.
For objects below 32 fields, `getField` now encodes the wanted key once and
compares UTF-8 bytes with the metadata dictionary. The binary search for larger
objects is unchanged. It searches in the builder's `String.compareTo` order,
which is not the UTF-8 byte order the Parquet Variant spec asks for. Fixing
that order is a separate change.
JMH on JDK 17, 4 lookups per row of 13 keys:
| | master | this PR |
|-------------------|--------|---------|
| time per row | 285 ns | 132 ns |
| allocated per row | 1368 B | 296 B |
Two edge cases change below the threshold. Both are pinned by the new tests:
| Lookup | master |
this PR |
|---------------------------------------------------------------|--------|---------|
| `getField("a\uD800")` on a field written with that key | `null` |
found |
| `getField("�")` on a key whose bytes are not valid UTF-8 | found | `null`
|
Flink never writes keys that are not valid UTF-8. Two distinct malformed
keys already decoded to the same name.
## Brief change log
- `BinaryVariant.getFieldByKey` compares UTF-8 bytes in its linear search.
- `BinaryVariantUtil` adds `encodeKey`, which `addKey` in the builder now
uses too, and `metadataKeyEquals`. `getMetadataKey` shares the key range lookup
with it.
## Verifying this change
This change added tests and can be verified as follows:
- `BinaryVariantTest#testGetFieldWithUnpairedSurrogate`,
`#testGetFieldWithEmptyKey` and `#testGetFieldWithMalformedUtf8Key`.
- The existing VARIANT tests in flink-core and flink-table-common cover
hits, missing keys, prefix keys and multi-byte keys.
## 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
- The serializers: no
- The runtime per-record code paths (performance sensitive): yes
- 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 2.1.309 (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]
