Skip to content

Commit 32dddbe

Browse files
committed
Clean up comments.
1 parent 54530bb commit 32dddbe

File tree

2 files changed

+29
-16
lines changed

2 files changed

+29
-16
lines changed

crates/iceberg/src/arrow/caching_delete_file_loader.rs

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ mod tests {
688688
}
689689

690690
/// Test loading a FileScanTask with BOTH positional and equality deletes.
691-
/// This reproduces the "Missing predicate for equality delete file" error from TestSparkExecutorCache.
691+
/// Verifies the fix for the inverted condition that caused "Missing predicate for equality delete file" errors.
692692
#[tokio::test]
693693
async fn test_load_deletes_with_mixed_types() {
694694
use crate::scan::FileScanTask;
@@ -705,16 +705,27 @@ mod tests {
705705
let data_file_schema = Arc::new(
706706
Schema::builder()
707707
.with_fields(vec![
708-
crate::spec::NestedField::optional(2, "y", crate::spec::Type::Primitive(crate::spec::PrimitiveType::Long)).into(),
709-
crate::spec::NestedField::optional(3, "z", crate::spec::Type::Primitive(crate::spec::PrimitiveType::Long)).into(),
708+
crate::spec::NestedField::optional(
709+
2,
710+
"y",
711+
crate::spec::Type::Primitive(crate::spec::PrimitiveType::Long),
712+
)
713+
.into(),
714+
crate::spec::NestedField::optional(
715+
3,
716+
"z",
717+
crate::spec::Type::Primitive(crate::spec::PrimitiveType::Long),
718+
)
719+
.into(),
710720
])
711721
.build()
712-
.unwrap()
722+
.unwrap(),
713723
);
714724

715725
// Write positional delete file
716726
let positional_delete_schema = crate::arrow::delete_filter::tests::create_pos_del_schema();
717-
let file_path_values = vec![format!("{}/data-1.parquet", table_location.to_str().unwrap()); 4];
727+
let file_path_values =
728+
vec![format!("{}/data-1.parquet", table_location.to_str().unwrap()); 4];
718729
let file_path_col = Arc::new(StringArray::from_iter_values(&file_path_values));
719730
let pos_col = Arc::new(Int64Array::from_iter_values(vec![0i64, 1, 2, 3]));
720731

@@ -767,22 +778,25 @@ mod tests {
767778
schema: data_file_schema.clone(),
768779
project_field_ids: vec![2, 3],
769780
predicate: None,
770-
deletes: vec![pos_del, eq_del], // BOTH types of deletes!
781+
deletes: vec![pos_del, eq_del],
771782
};
772783

773-
// Load the deletes - this should handle both types
784+
// Load the deletes - should handle both types without error
774785
let delete_file_loader = CachingDeleteFileLoader::new(file_io.clone(), 10);
775786
let delete_filter = delete_file_loader
776787
.load_deletes(&file_scan_task.deletes, file_scan_task.schema_ref())
777788
.await
778789
.unwrap()
779790
.unwrap();
780791

781-
// Try to build the equality delete predicate - this should fail with
782-
// "Missing predicate for equality delete file" if the bug exists
783-
let result = delete_filter.build_equality_delete_predicate(&file_scan_task).await;
784-
785-
// For now, this should fail, but once we fix the bug, this assertion should pass
786-
assert!(result.is_ok(), "Expected to successfully build equality delete predicate, but got error: {:?}", result.err());
792+
// Verify both delete types can be processed together
793+
let result = delete_filter
794+
.build_equality_delete_predicate(&file_scan_task)
795+
.await;
796+
assert!(
797+
result.is_ok(),
798+
"Failed to build equality delete predicate: {:?}",
799+
result.err()
800+
);
787801
}
788802
}

crates/iceberg/src/arrow/delete_filter.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,12 @@ impl DeleteFilter {
6868
pub(crate) fn try_start_eq_del_load(&self, file_path: &str) -> Option<Arc<Notify>> {
6969
let mut state = self.state.write().unwrap();
7070

71-
// If this equality delete file is already being loaded or has been loaded, return None
72-
// to indicate that another task is handling it or it's already available
71+
// Skip if already loaded/loading - another task owns it
7372
if state.equality_deletes.contains_key(file_path) {
7473
return None;
7574
}
7675

77-
// First time seeing this equality delete file - mark it as Loading
76+
// Mark as loading to prevent duplicate work
7877
let notifier = Arc::new(Notify::new());
7978
state
8079
.equality_deletes

0 commit comments

Comments
 (0)