amogh-jahagirdar commented on code in PR #15187:
URL: https://github.com/apache/iceberg/pull/15187#discussion_r2757245992
##########
core/src/test/java/org/apache/iceberg/data/avro/TestPlannedDataReader.java:
##########
@@ -150,7 +158,7 @@ public void timestampTzDataReader() throws IOException {
avroRecord.put(
"timestamp_millis_tz",
DateTimeUtil.millisFromTimestamptz(offsetTimestampMillis));
- Record result = readRecord(reader, avroSchema, avroRecord);
+ Record result = readRecords(reader, avroSchema,
Collections.singletonList(avroRecord)).get(0);
Review Comment:
Instead of changing all of the existing tests, can we just have readRecord
and readRecords? readRecord delegates to
`Iterables.getOnlyElement(readRecords(...))`
##########
core/src/main/java/org/apache/iceberg/avro/ValueReaders.java:
##########
@@ -305,6 +305,9 @@ private static ValueReader<?> createMissingFieldReader(
Types.NestedField field = expected.field(fieldId);
if (constant != null) {
+ if (fieldId == MetadataColumns.ROW_ID.fieldId()) {
+ return ValueReaders.rowIds((Long) constant, null);
+ }
Review Comment:
Spark + planned Avro works without the fix because the record schema passed
to `addFileFieldReadersToPlan` already include the row lineage fields but I
agree with this fix for enabling the Avro reader to read files independently;
the Avro reader should fill in the missing readers for lineage.
##########
core/src/test/java/org/apache/iceberg/data/avro/TestPlannedDataReader.java:
##########
@@ -186,19 +195,72 @@ public void timestampTzDataReader() throws IOException {
.isEqualTo(preEpochTimestampMillis.withOffsetSameInstant(ZoneOffset.UTC));
}
- private Record readRecord(
- PlannedDataReader<Record> reader, Schema avroSchema, GenericRecord
avroRecord)
+ @Test
+ public void testRowLineageInjectedWithPlannedReader() throws IOException {
Review Comment:
Could we also add a test where there is a mix of records where some records
have the materialized row lineage fields, and some do not?
--
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]
