mbutrovich commented on code in PR #6851:
URL: https://github.com/apache/datafusion-comet/pull/6851#discussion_r4296383523
##########
native/core/src/execution/operators/iceberg_scan.rs:
##########
@@ -342,19 +342,41 @@ impl IcebergScanExec {
let deletes = task
.deletes()
.iter()
- .map(|delete| {
- let mut delete = delete.clone();
- if let Some(&size) = size_map.get(&delete.file_path) {
- delete.file_size_in_bytes = size;
- }
- delete
+ .map(|delete| match size_map.get(delete.file_path()) {
+ Some(&size) => Self::sized_delete_file(delete, size),
+ None => Ok(delete.clone()),
})
- .collect::<Vec<_>>();
+ .collect::<Result<Vec<_>, Error>>()?;
*task = Self::rebuild_task_with_deletes(task, deletes)?;
}
Ok(())
}
+ /// Copies a delete file with its size set to `size`.
+ ///
+ /// iceberg-rust 1f3bc34 made `FileScanTaskDeleteFile`'s fields private
too, so, as for the
+ /// task in [`Self::rebuild_task_with_deletes`], the copy goes through the
builder, forwarding
+ /// every other field through its public accessors. The builder repeats
the delete file's
+ /// validation on `build()`.
Review Comment:
Should this doc describe the API as it is rather than the revision that
changed it? Once the pin moves past `1f3bc34`, the hash only sends a reader off
to look up a commit. The [`rebuild_task_with_deletes`
doc](https://github.com/apache/datafusion-comet/blob/a66d0e5ed77b680534e1e38d16a236b6009399cb/native/core/src/execution/operators/iceberg_scan.rs#L382-L384)
right below already reads that way:
```suggestion
/// `FileScanTaskDeleteFile`'s fields are private with no mutator, so,
as for the task in
/// [`Self::rebuild_task_with_deletes`], the copy goes through the
builder, forwarding every
/// other field through its public accessors. The builder repeats the
delete file's validation
/// on `build()`.
```
--
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]
