linhongyu510 commented on code in PR #3145:
URL: https://github.com/apache/iceberg-rust/pull/3145#discussion_r4054377301
##########
crates/iceberg/src/scan/task.rs:
##########
@@ -535,6 +540,154 @@ mod _serde {
}
}
+impl FileScanTaskDeleteFile {
+ /// Returns the delete file path.
+ pub fn file_path(&self) -> &str {
+ &self.file_path
+ }
+
+ /// Returns the total size of the delete file in bytes.
+ pub fn file_size_in_bytes(&self) -> u64 {
+ self.file_size_in_bytes
+ }
+
+ /// Returns the delete file content type.
+ pub fn file_type(&self) -> DataContentType {
+ self.file_type
+ }
+
+ /// Returns the delete file format.
+ pub fn file_format(&self) -> DataFileFormat {
+ self.file_format
+ }
+
+ /// Returns the partition spec id.
+ pub fn partition_spec_id(&self) -> i32 {
+ self.partition_spec_id
+ }
+
+ /// Returns the equality field ids for an equality delete file.
+ pub fn equality_ids(&self) -> Option<&[i32]> {
+ self.equality_ids.as_deref()
+ }
+
+ /// Returns the referenced data file path.
+ pub fn referenced_data_file(&self) -> Option<&str> {
+ self.referenced_data_file.as_deref()
+ }
+
+ /// Returns the deletion vector blob offset.
+ pub fn content_offset(&self) -> Option<i64> {
+ self.content_offset
+ }
+
+ /// Returns the deletion vector blob size in bytes.
+ pub fn content_size_in_bytes(&self) -> Option<i64> {
+ self.content_size_in_bytes
+ }
+
+ /// Returns the number of records in the delete file.
+ pub fn record_count(&self) -> Option<u64> {
+ self.record_count
+ }
+
+ /// Returns the key metadata for the encrypted delete file.
+ pub fn key_metadata(&self) -> Option<&[u8]> {
+ self.key_metadata.as_deref()
+ }
+
+ fn is_deletion_vector(&self) -> bool {
+ self.file_type == DataContentType::PositionDeletes
+ && self.file_format == DataFileFormat::Puffin
+ }
+
+ fn validate(&self) -> Result<()> {
+ if let Some(offset) = self.content_offset
+ && offset < 0
+ {
+ let kind = if self.is_deletion_vector() {
+ "deletion vector"
+ } else {
+ "delete file"
+ };
Review Comment:
Fixed, and the `u64` change above is what made it collapsible rather than
just tidier.
The duplication existed only to build the `"deletion vector"` vs `"delete
file"` prefix for two near-identical negative-value errors. Once a negative is
unrepresentable, both blocks disappear entirely — there is nothing left to
duplicate. What remains is presence checking, which is now one lookup and one
error site:
```rust
let missing = if self.referenced_data_file.is_none() {
Some("referenced_data_file")
} else if self.content_offset.is_none() {
Some("content_offset")
} else if self.content_size_in_bytes.is_none() {
Some("content_size_in_bytes")
} else if self.record_count.is_none() {
Some("record_count")
} else {
None
};
if let Some(field) = missing {
return Err(Error::new(
ErrorKind::DataInvalid,
format!("deletion vector {} is missing {field}", self.file_path),
));
}
```
`validate()` went from ~70 lines to ~25, and the four `Error::new` sites
became one. Error messages are unchanged, so the existing assertions still hold.
--
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]
