jdockerty commented on code in PR #2158:
URL: https://github.com/apache/iceberg-rust/pull/2158#discussion_r2890720584
##########
crates/iceberg/src/writer/file_writer/parquet_writer.rs:
##########
@@ -317,6 +321,19 @@ impl ParquetWriter {
// TODO: support adding to partitioned table
let mut data_files: Vec<DataFile> = Vec::new();
+ let table_schema = table_metadata.current_schema();
+ let name_mapping = table_metadata
+ .properties()
+ .get(DEFAULT_SCHEMA_NAME_MAPPING)
+ .map(|s| serde_json::from_str::<NameMapping>(s))
+ .transpose()
Review Comment:
Is this `transpose` usage required?
If I'm understanding this rightly, it is being used to flip the `map` output
on `Err` (after `from_str`) for the `map_err` to bubble up afterwards.
Although it seems like some indirection when a `match` would be more
explicit/easy to follow too.
This may just be a preference thing though, feel free to disregard 👍
##########
crates/iceberg/src/spec/schema/mod.rs:
##########
@@ -431,6 +431,133 @@ impl Display for Schema {
}
}
+/// Check whether `file_schema` is compatible with `table_schema` for
appending data files.
+///
+/// Walks the table schema and checks that every field is present and
type-compatible
+/// in the file schema (looked up by field ID). Follows the same semantics as
+/// iceberg-python's `_check_schema_compatible`.
Review Comment:
I'm not sure how likely it is that this method name would change, but it is
perhaps worth linking to the specific commit that you've referenced this from?
That way the details are clear from looking at the reference implementation.
##########
crates/iceberg/src/spec/datatypes.rs:
##########
@@ -250,6 +250,34 @@ pub enum PrimitiveType {
}
impl PrimitiveType {
+ /// Returns `true` if `self` can be promoted to `target` per the Iceberg
spec's
+ /// valid type promotion rules.
+ ///
+ /// See <https://iceberg.apache.org/spec/#schema-evolution>
Review Comment:
Great reference 💯
--
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]
