c-thiel commented on code in PR #2188:
URL: https://github.com/apache/iceberg-rust/pull/2188#discussion_r3330353395
##########
crates/iceberg/src/spec/datatypes.rs:
##########
@@ -178,6 +187,17 @@ impl Type {
Type::Primitive(PrimitiveType::Float) |
Type::Primitive(PrimitiveType::Double)
)
}
+
+ /// Returns the minimum format version required for the type.
+ #[inline(always)]
+ pub fn min_format_version(&self) -> FormatVersion {
Review Comment:
Good catch — I fixed the recursion. I went with a direct recursive match in
`Type::min_format_version` (struct/list/map arms recurse) rather than a
`SchemaVisitor`: it achieves both of your goals — correct for any Type and the
per-type version table living in one place — without the extra visitor struct
and Result plumbing (this fold is infallible). `Schema::min_format_version` now
just maps over the top-level fields and relies on that recursion, so the old
`id_to_field` split is gone too. Added a unit test for the nested case.
##########
crates/iceberg/src/spec/datatypes.rs:
##########
@@ -178,6 +187,17 @@ impl Type {
Type::Primitive(PrimitiveType::Float) |
Type::Primitive(PrimitiveType::Double)
)
}
+
+ /// Returns the minimum format version required for the type.
+ #[inline(always)]
+ pub fn min_format_version(&self) -> FormatVersion {
Review Comment:
Good catch — I fixed the recursion. I went with a direct recursive match in
`Type::min_format_version` (struct/list/map arms recurse) rather than a
`SchemaVisitor`: it achieves both of your goals — correct for any Type and the
per-type version table living in one place — without the extra visitor struct
and Result plumbing (this fold is infallible). `Schema::min_format_version` now
just maps over the top-level fields and relies on that recursion, so the old
`id_to_field` split is gone too. Added a unit test for the nested case.
Let me know what you think!
--
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]
