liurenjie1024 commented on code in PR #79:
URL: https://github.com/apache/iceberg-rust/pull/79#discussion_r1424928716
##########
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:
BTW, could change the variable name to `manifest_list_entry`? Otherwise it's
really confusing to read this🤣
##########
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:
BTW, could change the variable name to `manifest_list_entry`? Otherwise it's
really confusing to read this🤣
--
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
