anoopj commented on code in PR #2966:
URL: https://github.com/apache/iceberg-rust/pull/2966#discussion_r3744915772
##########
crates/iceberg/src/arrow/reader/pipeline.rs:
##########
@@ -300,6 +301,55 @@ impl FileScanTaskReader {
.with_constant(RESERVED_FIELD_ID_SPEC_ID, spec_id_datum);
}
+ if task
+ .project_field_ids()
+ .contains(&RESERVED_FIELD_ID_LAST_UPDATED_SEQUENCE_NUMBER)
+ {
+ // A data file may physically carry a per-row
`_last_updated_sequence_number`
+ // column, e.g. one written by another engine such as Iceberg Java
when carrying
+ // rows forward across a rewrite. The spec requires reading such
non-null
+ // per-row values unmodified, falling back to the derived value
only where
+ // null. That per-row coalesce is not implemented yet, so rather
than silently
+ // overwrite genuine per-row values with the derived value, reject
the file
+ // loudly. Checks the full pre-projection file schema, since the
column is
+ // stripped from the projection mask.
+ let file_has_column = record_batch_stream_builder
+ .schema()
+ .fields()
+ .iter()
+ .any(|f| {
+ f.metadata()
+ .get(PARQUET_FIELD_ID_META_KEY)
+ .and_then(|id| id.parse::<i32>().ok())
+ == Some(RESERVED_FIELD_ID_LAST_UPDATED_SEQUENCE_NUMBER)
+ });
+ if file_has_column {
+ return Err(Error::new(
+ ErrorKind::FeatureUnsupported,
Review Comment:
You are right that we need to fix the interop gap. I am planning to do a
fast follow on this.
But this PR is a strict improvement over `main` though. In the current
`main`, projecting `_last_updated_sequence_number` already errors out. This PR
turns an opaque `Unexpected` into a clear `FeatureUnsupported` error. Note that
`SELECT *` is unaffected because metadata columns aren't included, so only an
explicit projection reaches this path.
On pass-through vs. hard error as the interim: I thought about it, but
pass-through leaves null rows null when the spec says they should coalesce to
the derived value, so it's readable but silently wrong for exactly the
carried-forward rows. This trades a loud error for a quiet incorrect value. I'd
rather keep the loud `FeatureUnsupported` until the per-row coalesce lands as
the follow-up.
--
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]
