Skip to content

Data, Parquet: Fix UUID ClassCastException when reading Parquet files with UUIDs #14027

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

Merged
nastra merged 5 commits into from
Oct 15, 2025
Merged

Data, Parquet: Fix UUID ClassCastException when reading Parquet files with UUIDs #14027

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3c77bba
Parquet: Fix UUID ClassCastException
ndrluis Sep 8, 2025
ebd82de
fixup! Parquet: Fix UUID ClassCastException
ndrluis Oct 3, 2025
b51a298
Remove unused variables
ndrluis Oct 7, 2025
5aa04cf
Apply suggestion from @nastra
nastra Oct 10, 2025
1a51553
fixup! Parquet: Fix UUID ClassCastException
ndrluis Oct 14, 2025
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
30 changes: 28 additions & 2 deletions data/src/test/java/org/apache/iceberg/data/TestMetricsRowGroupFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ public static List<Object> parameters() {
optional(14, "all_nans", DoubleType.get()),
optional(15, "some_nans", FloatType.get()),
optional(16, "no_nans", DoubleType.get()),
optional(17, "some_double_nans", DoubleType.get()));
optional(17, "some_double_nans", DoubleType.get()),
optional(18, "uuid_col", Types.UUIDType.get()));

private static final Types.StructType UNDERSCORE_STRUCT_FIELD_TYPE =
Types.StructType.of(Types.NestedField.required(8, "_int_field", IntegerType.get()));
Expand All @@ -137,7 +138,8 @@ public static List<Object> parameters() {
optional(14, "_all_nans", Types.DoubleType.get()),
optional(15, "_some_nans", FloatType.get()),
optional(16, "_no_nans", Types.DoubleType.get()),
optional(17, "_some_double_nans", Types.DoubleType.get()));
optional(17, "_some_double_nans", Types.DoubleType.get()),
optional(18, "_uuid_col", Types.UUIDType.get()));

private static final Schema VARIANT_SCHEMA =
new Schema(
Expand All @@ -157,6 +159,11 @@ public static List<Object> parameters() {
private static final int INT_MIN_VALUE = 30;
private static final int INT_MAX_VALUE = 79;

private static final UUID UUID_WITH_ZEROS =
UUID.fromString("00000000-0000-0000-0000-000000000000");
private static final UUID UUID_WITH_ONES =

@nastra nastra Oct 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when is this actually used?

@ndrluis ndrluis Oct 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad, the b51a298 commit solves this.

UUID.fromString("11111111-1111-1111-1111-111111111111");

private File orcFile = null;
private MessageType parquetSchema = null;
private BlockMetaData rowGroupMetadata = null;
Expand Down Expand Up @@ -211,6 +218,9 @@ public void createOrcInputFile() throws IOException {
structNotNull.setField("_int_field", INT_MIN_VALUE + i);
record.setField("_struct_not_null", structNotNull); // struct with int

record.setField(
"_uuid_col", (i % 3 == 0) ? UUID_WITH_ZEROS : (i % 3 == 1) ? UUID_WITH_ONES : null);

@nastra nastra Sep 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: what's the reason for doing the modulo here? why not just write UUID_WITH_ZEROS?

@ndrluis ndrluis Sep 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The idea was to use different values to make sure the filtering works, but now I think just with_zeros and null are enough. WDYT?

@nastra nastra Oct 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it comes down to also testing other expressions. See my other comment on this


appender.add(record);
}
}
Expand Down Expand Up @@ -248,6 +258,10 @@ private void createParquetInputFile() throws IOException {
GenericRecord structNotNull = GenericRecord.create(UNDERSCORE_STRUCT_FIELD_TYPE);
structNotNull.setField("_int_field", INT_MIN_VALUE + i);
builder.setField("_struct_not_null", structNotNull); // struct with int

Comment thread
nastra marked this conversation as resolved.
Outdated
builder.setField(
"_uuid_col", (i % 3 == 0) ? UUID_WITH_ZEROS : (i % 3 == 1) ? UUID_WITH_ONES : null);

records.add(builder);
}

Expand Down Expand Up @@ -1063,6 +1077,18 @@ public void testVariantFieldAllNullsNotNull() throws IOException {
}
}

@TestTemplate
public void testUUIDEq() {
assumeThat(format).as("Only valid for Parquet").isEqualTo(FileFormat.PARQUET);

boolean shouldRead = shouldRead(equal("uuid_col", UUID_WITH_ZEROS));

@nastra nastra Sep 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about testing other expressions?

assertThat(shouldRead).as("Should read: UUID value exists in row group").isTrue();

UUID nonExistentUuid = UUID.fromString("99999999-9999-9999-9999-999999999999");
boolean shouldSkip = shouldRead(equal("uuid_col", nonExistentUuid));
assertThat(shouldSkip).as("Should skip: UUID value does not exist in row group").isFalse();
}

private boolean shouldRead(Expression expression) {
return shouldRead(expression, true);
}
Expand Down
2 changes: 2 additions & 0 deletions parquet/src/main/java/org/apache/iceberg/parquet/ParquetConversions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ static Function<Object, Object> converterFromParquet(
} else if (icebergType.typeId() == Type.TypeID.DOUBLE
&& parquetType.getPrimitiveTypeName() == PrimitiveType.PrimitiveTypeName.FLOAT) {
return value -> ((Float) fromParquet.apply(value)).doubleValue();
} else if (icebergType.typeId() == Type.TypeID.UUID) {
return binary -> UUIDUtil.convert(((Binary) binary).toByteBuffer());
Comment on lines +86 to +87

@Fokko Fokko Sep 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems like an odd place to apply this conversion since the rows above are more about schema evolution. However, looking at it a bit closer, I think it makes sense. Other logical types, such as TimestampLiteral store the primitive type internally (long), while the UUIDLiteral keeps a UUID rather than bytes.

This will just compare the bytes using an unsigned lexicographical binary comparator.

}
}

Expand Down
31 changes: 29 additions & 2 deletions parquet/src/test/java/org/apache/iceberg/parquet/TestDictionaryRowGroupFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ public class TestDictionaryRowGroupFilter {
14,
"decimal_fixed",
DecimalType.of(20, 10)), // >18 precision to enforce FIXED_LEN_BYTE_ARRAY
optional(15, "_nans_and_nulls", DoubleType.get()));
optional(15, "_nans_and_nulls", DoubleType.get()),
optional(16, "uuid_col", Types.UUIDType.get()));

private static final Types.StructType UNDERSCORE_STRUCT_FIELD_TYPE =
Types.StructType.of(Types.NestedField.required(9, "_int_field", IntegerType.get()));
Expand All @@ -133,7 +134,8 @@ public class TestDictionaryRowGroupFilter {
14,
"_decimal_fixed",
DecimalType.of(20, 10)), // >18 precision to enforce FIXED_LEN_BYTE_ARRAY
optional(15, "_nans_and_nulls", DoubleType.get()));
optional(15, "_nans_and_nulls", DoubleType.get()),
optional(16, "_uuid_col", Types.UUIDType.get()));

private static final String TOO_LONG_FOR_STATS;

Expand All @@ -153,6 +155,11 @@ public class TestDictionaryRowGroupFilter {
.subtract(DECIMAL_MIN_VALUE)
.divide(new BigDecimal(INT_MAX_VALUE - INT_MIN_VALUE), RoundingMode.HALF_UP);

private static final UUID UUID_WITH_ZEROS =
UUID.fromString("00000000-0000-0000-0000-000000000000");
private static final UUID UUID_WITH_ONES =
UUID.fromString("11111111-1111-1111-1111-111111111111");

private MessageType parquetSchema = null;
private BlockMetaData rowGroupMetadata = null;
private DictionaryPageReadStore dictionaryStore = null;
Expand Down Expand Up @@ -203,6 +210,9 @@ public void createInputFile() throws IOException {
structNotNull.put("_int_field", INT_MIN_VALUE + i);
builder.set("_struct_not_null", structNotNull); // struct with int

builder.set(
"_uuid_col", (i % 3 == 0) ? UUID_WITH_ZEROS : (i % 3 == 1) ? UUID_WITH_ONES : null);

appender.add(builder.build());
}
}
Expand Down Expand Up @@ -1267,6 +1277,23 @@ public void testTransformFilter() {
.isTrue();
}

@TestTemplate
public void testUUIDDictionaryFilter() {
assumeThat(getColumnForName(rowGroupMetadata, "_uuid_col").getEncodings())
.contains(Encoding.RLE_DICTIONARY);

boolean shouldReadExisting =
new ParquetDictionaryRowGroupFilter(SCHEMA, equal("uuid_col", UUID_WITH_ZEROS))
.shouldRead(parquetSchema, rowGroupMetadata, dictionaryStore);
assertThat(shouldReadExisting).as("Should read: Dictionary contains a matching entry").isTrue();

UUID nonExistentUUID = UUID.fromString("22222222-2222-2222-2222-222222222222");
boolean shouldRead =
new ParquetDictionaryRowGroupFilter(SCHEMA, equal("uuid_col", nonExistentUUID))
.shouldRead(parquetSchema, rowGroupMetadata, dictionaryStore);
assertThat(shouldRead).as("Should skip: UUID not found in dictionary").isFalse();
}

private ColumnChunkMetaData getColumnForName(BlockMetaData rowGroup, String columnName) {
ColumnPath columnPath = ColumnPath.fromDotString(columnName);
for (ColumnChunkMetaData column : rowGroup.getColumns()) {
Expand Down