Skip to content

Parquet: Fix variant BINARY upper bound to truncate up #16880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vishnuprakaz wants to merge 2 commits into
base: main
Choose a base branch
from
+51 −1
Open

Parquet: Fix variant BINARY upper bound to truncate up #16880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd2458a
Parquet: Fix variant BINARY upper bound to truncate up
vishnuprakaz Jun 19, 2026
85051f1
Parquet: Add test for variant BINARY upper bound overflow
vishnuprakaz Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion parquet/src/main/java/org/apache/iceberg/parquet/ParquetVariantUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ private static VariantValue truncateUpperBound(VariantValue value) {
return truncatedString != null ? Variants.of(PhysicalType.STRING, truncatedString) : null;
case BINARY:
ByteBuffer truncatedBuffer =
BinaryUtil.truncateBinaryMin((ByteBuffer) value.asPrimitive().get(), 16);
BinaryUtil.truncateBinaryMax((ByteBuffer) value.asPrimitive().get(), 16);

@nssalian nssalian Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that BINARY uses truncateBinaryMax, the all-0xFF overflow case (returns null) is reachable now. Worth a case with an all-0xFF value longer than 16 bytes asserting the upper bound is absent?

return truncatedBuffer != null ? Variants.of(PhysicalType.BINARY, truncatedBuffer) : null;
default:
return value;
Expand Down
50 changes: 50 additions & 0 deletions parquet/src/test/java/org/apache/iceberg/parquet/TestVariantMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,56 @@ public void testShreddedPrimitiveTypeMismatch(VariantValue value) throws IOExcep
assertThat(metrics).extracting("originalTypes").isEqualTo(Map.of(1, Types.LongType.get()));
}

@Test
public void testShreddedBinaryBoundsTruncation() throws IOException {
// binary longer than the 16-byte truncation length so the bounds are truncated
byte[] bytes = new byte[20];
for (int i = 0; i < bytes.length; i += 1) {
bytes[i] = (byte) (i + 1);
}
VariantValue value = Variants.of(ByteBuffer.wrap(bytes));

Metrics metrics =
writeParquet(
(id, name) -> ParquetVariantUtil.toParquetSchema(value),
Variant.of(EMPTY, value),
Variant.of(EMPTY, Variants.ofNull()),
null);

assertThat(metrics.lowerBounds().get(2))
.extracting(b -> Variant.from(b).value().asObject().get(ROOT_FIELD))
.isEqualTo(Variants.of(BinaryUtil.truncateBinaryMin(ByteBuffer.wrap(bytes), 16)));

assertThat(metrics.upperBounds().get(2))
.extracting(b -> Variant.from(b).value().asObject().get(ROOT_FIELD))
.isEqualTo(Variants.of(BinaryUtil.truncateBinaryMax(ByteBuffer.wrap(bytes), 16)));
}

@Test
public void testShreddedBinaryUpperBoundOverflow() throws IOException {
// an all-0xFF binary cannot be truncated up so the upper bound is omitted
byte[] bytes = new byte[20];
for (int i = 0; i < bytes.length; i += 1) {
bytes[i] = (byte) 0xFF;
}
VariantValue value = Variants.of(ByteBuffer.wrap(bytes));

Metrics metrics =
writeParquet(
(id, name) -> ParquetVariantUtil.toParquetSchema(value),
Variant.of(EMPTY, value),
Variant.of(EMPTY, Variants.ofNull()),
null);

assertThat(metrics.lowerBounds().get(2))
.extracting(b -> Variant.from(b).value().asObject().get(ROOT_FIELD))
.isEqualTo(Variants.of(BinaryUtil.truncateBinaryMin(ByteBuffer.wrap(bytes), 16)));

assertThat(metrics.upperBounds().get(2))
.extracting(b -> Variant.from(b).value().asObject().get(ROOT_FIELD))
.isNull();
}

@Test
public void testVariantFloatNaN() throws IOException {
// NaN values are not counted because there is no ID for FieldMetrics
Expand Down