You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "rayokota (via GitHub)" <gi...@apache.org> on 2026/08/22 02:57:04 UTC
[PR] GH-3735 Fix Variant field name comparisons to use UTF-8 byte order [parquet-java]
rayokota opened a new pull request, #3736:
URL: https://github.com/apache/parquet-java/pull/3736
<!--
Thanks for opening a pull request!
If you're new to Parquet-Java, information on how to contribute can be found here: https://parquet.apache.org/docs/contribution-guidelines/contributing
Please open a GitHub issue for this pull request: https://github.com/apache/parquet-java/issues/new/choose
and format pull request title as below:
GH-${GITHUB_ISSUE_ID}: ${SUMMARY}
or simply use the title below if it is a minor issue:
MINOR: ${SUMMARY}
-->
### Rationale for this change
The Variant spec requires the field ids in an object's header to be sorted by the
**UTF-8 byte order** of the field names, so a reader can binary-search them.
`VariantBuilder` sorted the fields — and `Variant.getFieldByKey` binary-searched them —
using `String.compareTo`, which orders by **UTF-16 code units**, not UTF-8 bytes.
The two orderings are identical for all field names in the Basic Multilingual Plane, but
they diverge for names containing 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:
- An object parquet-java builds with such keys has field ids sorted in an order that
violates the spec.
- A spec-compliant reader (e.g. the Apache Arrow C++/Rust/Go Variant readers) binary-searching
that object can fail to find fields.
- Conversely, parquet-java's own binary search fails to find a supplementary-plane key in an
object produced by a spec-compliant writer.
The bug only surfaces when an object both contains a supplementary-plane key and is large
enough to take the binary-search path, so it has gone unnoticed.
### What changes are included in this PR?
- Add `VariantUtil.compareKeys(String, String)`, comparing two field names by the unsigned
lexicographic order of their UTF-8 encoded bytes (`Arrays.compareUnsigned` over
`getBytes(UTF_8)`).
- Use it at the two order-dependent sites, which must agree:
- `VariantBuilder.FieldEntry.compareTo` — the field sort in `endObject()`.
- `Variant.getFieldByKey` — the binary-search comparison.
- The small-object linear-scan branch already uses `String.equals` (order-independent) and is
unchanged; the metadata dictionary is written unsorted, so it is unaffected.
### Are these changes tested?
Two new tests in `TestVariantObjectBuilder`:
- `testObjectKeysSortedByUtf8ByteOrder` — builds an object with keys U+FFFF (`EF BF BF`) and
U+10000 (`F0 90 80 80`) appended in reverse and asserts the encoded field order is U+FFFF
then U+10000 (UTF-8 order), which the previous `compareTo` reversed.
- `testLargeObjectBinarySearchWithSupplementaryKey` — a 42-field object (above
`BINARY_SEARCH_THRESHOLD`) mixing ASCII keys with U+FFFF and U+10000, asserting
`getFieldByKey` resolves both through the binary-search path.
### Are there any user-facing changes?
<!-- Please uncomment the line below and replace ${GITHUB_ISSUE_ID} with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] GH-3735 Fix Variant field name comparisons to use UTF-8 byte order [parquet-java]
Posted by "rayokota (via GitHub)" <gi...@apache.org>.
rayokota closed pull request #3736: GH-3735 Fix Variant field name comparisons to use UTF-8 byte order
URL: https://github.com/apache/parquet-java/pull/3736
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] GH-3735 Fix Variant field name comparisons to use UTF-8 byte order [parquet-java]
Posted by "rayokota (via GitHub)" <gi...@apache.org>.
rayokota commented on PR #3736:
URL: https://github.com/apache/parquet-java/pull/3736#issuecomment-5427812142
Closed in favor of https://github.com/apache/parquet-java/pull/3746
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] GH-3735 Fix Variant field name comparisons to use UTF-8 byte order [parquet-java]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #3736:
URL: https://github.com/apache/parquet-java/pull/3736#discussion_r3838838586
##########
parquet-variant/src/main/java/org/apache/parquet/variant/Variant.java:
##########
@@ -273,6 +273,8 @@ public Variant getFieldByKey(String key) {
}
}
Review Comment:
I'm thinking if we could just do binary search anyway. as arrow-rs does it.
https://github.com/apache/arrow-rs/blob/7ec3f5ab0c41f10ece65d3745b989ddde510539b/parquet-variant/src/variant/metadata.rs#L378-L397
##########
parquet-variant/src/main/java/org/apache/parquet/variant/Variant.java:
##########
@@ -282,7 +284,7 @@ public Variant getFieldByKey(String key) {
int mid = (low + high) >>> 1;
int midId = VariantUtil.readUnsignedLittleEndian(value, idStart + info.idSize * mid, info.idSize);
String midKey = getMetadataKeyCached(midId);
- int cmp = midKey.compareTo(key);
+ int cmp = VariantUtil.compareKeys(VariantUtil.encodeKey(midKey), keyBytes);
Review Comment:
Should we preserve read compatibility with Variant values written by earlier parquet-java versions? Its writers sorted object entries using Java `String.compareTo`.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] GH-3735 Fix Variant field name comparisons to use UTF-8 byte order [parquet-java]
Posted by "rayokota (via GitHub)" <gi...@apache.org>.
rayokota commented on PR #3736:
URL: https://github.com/apache/parquet-java/pull/3736#issuecomment-5397727335
@peterxcli , I see you made the equivalent fixes for Spark. Feel free to adapt this PR to match or to close this PR and open a new one. Thanks!
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org