ZENOTME commented on code in PR #79:
URL: https://github.com/apache/iceberg-rust/pull/79#discussion_r1424917585
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -156,12 +171,36 @@ impl ManifestListWriter {
match self.format_version {
FormatVersion::V1 => {
for manifest_entry in manifest_entries {
- let manifest_entry: ManifestListEntryV1 =
manifest_entry.into();
+ let manifest_entry: ManifestListEntryV1 =
manifest_entry.try_into()?;
self.avro_writer.append_ser(manifest_entry)?;
}
}
FormatVersion::V2 => {
- for manifest_entry in manifest_entries {
+ for mut manifest_entry in manifest_entries {
+ if manifest_entry.sequence_number ==
UNASSIGNED_SEQUENCE_NUMBER {
+ if manifest_entry.added_snapshot_id !=
self.snapshot_id {
+ return Err(Error::new(
+ ErrorKind::DataInvalid,
+ format!(
+ "Found unassigned sequence number for a
manifest from snapshot {}.",
+ manifest_entry.added_snapshot_id
+ ),
+ ));
+ }
+ manifest_entry.sequence_number = self.sequence_number;
Review Comment:
The reason we postpone this assignment is that we don't need to recall
manifest file if commit is fail. E.g. if the we assign the sequence_number in
ManifestWriter
```
// write Manifest
let entry = ManifestWriter(seq_num);
// writer ManifestList
let res = ManifestListWriter(entry);
// if commit is failed, we should recall the ManifestWriter to get a new
entry using a new seq num.
let res = commit(res);
```
--
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]
