ZENOTME commented on code in PR #122:
URL: https://github.com/apache/iceberg-rust/pull/122#discussion_r1430867133
##########
crates/iceberg/src/spec/manifest.rs:
##########
@@ -79,6 +79,22 @@ impl Manifest {
Ok(Manifest { metadata, entries })
}
+
+ /// Upgrade the format version of this manifest.
+ pub fn upgrade_version(&mut self, format_version: FormatVersion) ->
Result<(), Error> {
Review Comment:
> +1 for this concern. Version upgrade is part of transaction api, it
involves full rewrite of all things like snapshot, and should not do it here.
Yes, upgrading the version is more. Here it's more like upgrading this
manifest file.
https://github.com/apache/iceberg-rust/issues/119#issuecomment-1855355460
This interface is confusing. There is another way:
1. `pub fn parse_avro(bs: &[u8], version) -> Result<Self, Error>`. Then we
can parse v1 manifest as v2. After we parse it, we will modify the version as
v2.
2. `pub async fn write(mut self, manifest: Manifest, version)`. And then we
can write as v1 manifest as v2. I think this way may be more clear.🤔
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
