| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
… keys The Variant spec requires object field ids to be sorted by the unsigned byte order of the field names' UTF-8 encoding, so readers can binary search them. VariantBuilder sorted fields - and Variant.getFieldByKey binary-searched them - with String.compareTo, which orders UTF-16 code units instead. The two orders diverge for keys containing supplementary-plane characters (U+10000 and above). - Add VariantUtil.encodeKey/compareKeys and use them when sorting object fields and binary-searching by key (adapted from apache#3736) - Retry lookups in UTF-16 order for keys containing code units at or above U+D800, so objects written before this fix remain readable Co-authored-by: rayokota <rayokota@gmail.com>
There was a problem hiding this comment.
One comment about potential perf issues, non-blocking as obviously correctness is more important. Also, great test coverage!
Sorry, something went wrong.
| int midId = VariantUtil.readUnsignedLittleEndian(value, idStart + info.idSize * mid, info.idSize); | ||
| String midKey = getMetadataKeyCached(midId); | ||
| int cmp = attempt == 0 | ||
| ? VariantUtil.compareKeys(VariantUtil.encodeKey(midKey), keyBytes) |
There was a problem hiding this comment.
IIUC VariantUtil.encodeKey(midKey) means we now do an allocation on every iteration, correct? Can we measure the impact of this? Would it be better to cache the encoded keys, similar to getMetadataKeyCached?
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Rationale for this change
The Variant encoding specification requires the field ids in an object's header to be sorted by the unsigned byte order of the field names' UTF-8 encoding, so readers can binary-search them. VariantBuilder sorted the fields — and Variant.getFieldByKey binary-searched them — using String.compareTo, which orders UTF-16 code units instead.
The two orderings agree for all keys in the Basic Multilingual Plane but diverge for supplementary-plane characters (U+10000 and above): String.compareTo orders a leading high surrogate (0xD800–0xDBFF) before code points in U+E000..U+FFFF, whereas UTF-8 byte order (and the spec) orders them after. Consequences:
This adapts #3736 by @rayokota and adds the read-compatibility fallback from the equivalent Spark fix (apache/spark#58239), per the discussion on that PR.
What changes are included in this PR?
Are these changes tested?
Three new tests in TestVariantObjectBuilder:
All 183 parquet-variant tests and the parquet-avro variant read/write suites pass locally.
Are there any user-facing changes?
Newly written Variant objects containing supplementary-plane field keys now use the specification's unsigned UTF-8 field order. Objects written in the legacy UTF-16 order remain readable via getFieldByKey.
Closes #3735
🤖 Generated with Claude Code