c-thiel commented on code in PR #2188:
URL: https://github.com/apache/iceberg-rust/pull/2188#discussion_r3356570823
##########
crates/iceberg/src/spec/schema/mod.rs:
##########
@@ -421,6 +425,67 @@ impl Schema {
pub fn field_id_to_fields(&self) -> &HashMap<i32, NestedFieldRef> {
&self.id_to_field
}
+
+ /// Returns the minimum [`FormatVersion`] required to represent all types
in this schema.
+ ///
+ /// Defaults to `FormatVersion::V1` if all types are universally supported.
+ pub fn min_format_version(&self) -> FormatVersion {
Review Comment:
Agreed on the visitor — I'll move the computation into a `SchemaVisitor` (`T
= FormatVersion`), which also lets me drop the recursive
`Type::min_format_version` you flagged below. I'd keep a public method
returning the value though: `datafusion` uses it to derive the table's format
version (`min_format_version().max(V2)`), not to check a known one, so a pure
`is_compatible(fv)` predicate wouldn't cover that use. So: visitor under the
hood, thin query on top.
--
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]
