pvary commented on code in PR #17320:
URL: https://github.com/apache/iceberg/pull/17320#discussion_r3639289823
##########
parquet/src/main/java/org/apache/iceberg/data/parquet/BaseParquetReaders.java:
##########
@@ -280,29 +280,91 @@ public ParquetValueReader<?> struct(
}
}
- int constantDefinitionLevel = type.getMaxDefinitionLevel(currentPath());
+ String[] structPath = currentPath();
+ int constantDefinitionLevel = type.getMaxDefinitionLevel(structPath);
List<Types.NestedField> expectedFields = expected.fields();
+
+ // When a nullable struct's projected fields are all constants (initial
defaults, metadata
+ // columns, or partition values) with no real column read from the file,
the struct has no
+ // per-row definition level to indicate whether an ancestor struct is
null. In that case,
+ // borrow the definition level from a real leaf column that exists under
the struct in the
+ // file so a null ancestor is not incorrectly materialized as a struct
of default values.
+ boolean hasRealFieldReader =
+ expectedFields.stream().anyMatch(field ->
readersById.containsKey(field.fieldId()));
+ ColumnDescriptor probe = null;
+ Integer probeHostId = null;
+ if (!hasRealFieldReader && constantDefinitionLevel > 0) {
+ probe = firstLeafUnder(structPath);
+ if (probe != null) {
+ probeHostId = probeHostFieldId(expectedFields);
+ }
+ }
+
List<ParquetValueReader<?>> reorderedFields =
Lists.newArrayListWithExpectedSize(expectedFields.size());
-
for (Types.NestedField field : expectedFields) {
int id = field.fieldId();
ParquetValueReader<?> reader =
ParquetValueReaders.replaceWithMetadataReader(
id, readersById.get(id), idToConstant,
constantDefinitionLevel);
- reorderedFields.add(defaultReader(field, reader,
constantDefinitionLevel));
+ boolean hostsProbe = probeHostId != null && id == probeHostId;
+ reorderedFields.add(
Review Comment:
This will this mess up structs which only contain constant fields?
```
Schema readSchema =
new Schema(
Types.NestedField.required(1, "id", Types.LongType.get()),
Types.NestedField.optional("nested")
.withId(2)
.ofType(
Types.StructType.of(
Types.NestedField.optional(4, "part",
Types.StringType.get())))
.build());
```
The AI wrote this unit test for me:
```
@Test
public void testNestedConstantWhenAncestorStructIsNull() throws
IOException {
// one row has an inner value, the other has a null nested struct
Schema writeSchema =
new Schema(
Types.NestedField.required(1, "id", Types.LongType.get()),
Types.NestedField.optional("nested")
.withId(2)
.ofType(
Types.StructType.of(
Types.NestedField.required(3, "inner",
Types.StringType.get())))
.build());
Record present = GenericRecord.create(writeSchema);
present.setField("id", 1L);
Record presentNested =
GenericRecord.create(writeSchema.findField("nested").type().asStructType());
presentNested.setField("inner", "a");
present.setField("nested", presentNested);
Record nullNested = GenericRecord.create(writeSchema);
nullNested.setField("id", 2L);
nullNested.setField("nested", null);
OutputFile output = new InMemoryOutputFile();
try (FileAppender<Record> appender =
Parquet.write(output)
.schema(writeSchema)
.createWriterFunc(GenericParquetWriter::create)
.build()) {
appender.add(present);
appender.add(nullNested);
}
// project only a metadata/partition constant field (served from
idToConstant); inner is dropped
Schema readSchema =
new Schema(
Types.NestedField.required(1, "id", Types.LongType.get()),
Types.NestedField.optional("nested")
.withId(2)
.ofType(
Types.StructType.of(
Types.NestedField.optional(4, "part",
Types.StringType.get())))
.build());
Map<Integer, ?> idToConstant = ImmutableMap.of(4, "US");
List<Record> rows;
try (CloseableIterable<Record> reader =
Parquet.read(output.toInputFile())
.project(readSchema)
.createReaderFunc(
fileSchema ->
GenericParquetReaders.buildReader(readSchema,
fileSchema, idToConstant))
.build()) {
rows = Lists.newArrayList(reader);
}
assertThat(rows).hasSize(2);
// present struct reads the constant
Record row1Nested = (Record) rows.get(0).getField("nested");
assertThat(row1Nested).isNotNull();
assertThat(row1Nested.getField("part")).isEqualTo("US");
// null struct reads as null
assertThat(rows.get(1).getField("nested")).isNull();
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
