Skip to content

Parquet: Fix variant STRING metrics bounds to use UTF-8 byte order - #17397

Open
vishnuprakaz wants to merge 1 commit into
apache:mainfrom
vishnuprakaz:variant-string-comparator-fix
Open

Parquet: Fix variant STRING metrics bounds to use UTF-8 byte order#17397
vishnuprakaz wants to merge 1 commit into
apache:mainfrom
vishnuprakaz:variant-string-comparator-fix

Conversation

@vishnuprakaz

Copy link
Copy Markdown
Contributor

Rationale for this change

Queries over a shredded variant STRING column can silently return wrong (missing) results a file that actually contains matching rows gets skipped.

variant string bounds are aggregated across row groups with String.compareTo (UTF-16 code-unit order), but Parquet's per-column stats and the scan-time evaluator (Comparators.charSequences()) use UTF-8 byte order.

The two disagree when a supplementary-plane character (code point ≥ U+10000, e.g. an emoji a surrogate pair starting 0xD800) is compared with a BMP character in U+E000U+FFFF: String.compareTo ranks the supplementary char below it (0xD800 < 0xE000), UTF-8 ranks it above. For a string column spanning multiple row groups with that mix, the aggregated lower/upper bounds invert, and the evaluator (UTF-8 order) then prunes files that match.

The BINARY branch already uses Comparators.unsignedBytes(), and the non-variant ParquetMetrics path uses Comparators.charSequences() for
strings; only the variant STRING branch fell through to naturalOrder(). Same class as #16880.

What changes are included in this PR?

Use Comparators.charSequences() for STRING in ParquetVariantUtil.comparator(), matching the BINARY branch and the string ordering used
elsewhere.

Are these changes tested?

Yes. TestVariantMetrics.testShreddedStringBoundsAcrossRowGroups writes a shredded string variant across multiple row groups mixing U+E000 and U+10000 and asserts the bounds are not inverted fails before the fix, passes after. Existing tests used single-row-group ASCII values, so this path was uncovered.

Are there any user-facing changes?

No API changes.

Present in 1.9.0–1.11.0; backports to 1.10.x and 1.11.x. will do follow up.

@uros-b

uros-b commented Jul 28, 2026

Copy link
Copy Markdown
Member

Nice fix, thank you @vishnuprakaz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants