You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ayushtkn (via GitHub)" <gi...@apache.org> on 2026/01/30 11:13:12 UTC
[PR] Core: Avro: Row Lineage Column (ROW_ID) is not populated correctly in case the Data File doesn't have them [iceberg]
ayushtkn opened a new pull request, #15187:
URL: https://github.com/apache/iceberg/pull/15187
Fixes: https://github.com/apache/iceberg/issues/15186
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Avro: Row Lineage Column (ROW_ID) is not populated correctly in case the Data File doesn't have them [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #15187:
URL: https://github.com/apache/iceberg/pull/15187#issuecomment-3827432877
The fix LGTM. @amogh-jahagirdar Could you please take a look as well?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Avro: Row Lineage Column (ROW_ID) is not populated correctly in case the Data File doesn't have them [iceberg]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #15187:
URL: https://github.com/apache/iceberg/pull/15187#issuecomment-3849660070
Thanx @huaxingao @singhpk234 @amogh-jahagirdar for the reviews, I have addressed the review comments. Let me know if this looks good
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Avro: Row Lineage Column (ROW_ID) is not populated correctly in case the Data File doesn't have them [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Avro: Row Lineage Column (ROW_ID) is not populated correctly in case the Data File doesn't have them [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao merged PR #15187:
URL: https://github.com/apache/iceberg/pull/15187
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Avro: Row Lineage Column (ROW_ID) is not populated correctly in case the Data File doesn't have them [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #15187:
URL: https://github.com/apache/iceberg/pull/15187#issuecomment-3861654442
Thanks @ayushtkn for the PR! Thanks @amogh-jahagirdar @singhpk234 for the review!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Avro: Row Lineage Column (ROW_ID) is not populated correctly in case the Data File doesn't have them [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #15187:
URL: https://github.com/apache/iceberg/pull/15187#discussion_r2748841459
##########
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 {
+ org.apache.iceberg.Schema icebergSchema =
+ new org.apache.iceberg.Schema(
+ Types.NestedField.required(1, "data", Types.StringType.get()),
+ MetadataColumns.ROW_ID,
+ MetadataColumns.LAST_UPDATED_SEQUENCE_NUMBER);
+
+ Schema avroFileSchema =
+ SchemaBuilder.record("test")
+ .fields()
+ .name("data")
+ .type()
+ .stringType()
+ .noDefault()
+ .endRecord();
+ avroFileSchema.getField("data").addProp("field-id", 1);
+
+ Map<Integer, Object> idToConstant =
+ ImmutableMap.of(
+ MetadataColumns.ROW_ID.fieldId(), 1000L,
+ MetadataColumns.LAST_UPDATED_SEQUENCE_NUMBER.fieldId(), 5L);
+
+ PlannedDataReader<Record> reader = PlannedDataReader.create(icebergSchema, idToConstant);
+ reader.setSchema(avroFileSchema);
+ reader.setRowPositionSupplier(() -> 0L);
+
+ GenericRecord rec1 = new GenericData.Record(avroFileSchema);
+ rec1.put("data", "a");
+ GenericRecord rec2 = new GenericData.Record(avroFileSchema);
+ rec2.put("data", "b");
+
+ List<Record> results = readRecord(reader, avroFileSchema, Lists.newArrayList(rec1, rec2));
+
+ assertThat(results).hasSize(2);
+
+ assertThat(results.get(0).getField(MetadataColumns.ROW_ID.name())).isEqualTo(1000L);
+ assertThat(results.get(0).getField(MetadataColumns.LAST_UPDATED_SEQUENCE_NUMBER.name()))
+ .isEqualTo(5L);
+ assertThat(results.get(1).getField(MetadataColumns.ROW_ID.name())).isEqualTo(1001L);
+ assertThat(results.get(1).getField(MetadataColumns.LAST_UPDATED_SEQUENCE_NUMBER.name()))
+ .isEqualTo(5L);
+ }
+
+ private List<Record> readRecord(
Review Comment:
nit: renname to `readRecords`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org