brgr-s commented on code in PR #2936:
URL: https://github.com/apache/iceberg-rust/pull/2936#discussion_r3811406834
##########
crates/iceberg/src/delete_file_index.rs:
##########
@@ -113,25 +115,74 @@ impl DeleteFileIndex {
}
}
+/// The single data file a position delete file applies to, `None` if it is
not tied
+/// to a single data file.
+fn position_delete_target(data_file: &DataFile) -> Option<String> {
+ // data files is named directly
+ if let Some(path) = data_file.referenced_data_file() {
+ return Some(path);
+ }
+
+ // lower and upper bound of reserved field are equal, so all rows
+ // have the same value
+ let lower = data_file
+ .lower_bounds()
+ .get(&RESERVED_FIELD_ID_DELETE_FILE_PATH)?;
+ let upper = data_file
+ .upper_bounds()
+ .get(&RESERVED_FIELD_ID_DELETE_FILE_PATH)?;
+ if lower != upper {
+ return None;
+ }
+
+ match lower.literal() {
+ PrimitiveLiteral::String(path) => Some(path.clone()),
+ _ => None,
+ }
+}
+
+/// Whether a position delete file's sequence number lets it apply to a data
file whose
+/// own sequence number is `data_file_seq_num`.
+fn position_delete_applies(delete_seq_num: Option<i64>, data_file_seq_num:
Option<i64>) -> bool {
+ data_file_seq_num
+ .map(|seq| delete_seq_num >= Some(seq))
+ .unwrap_or(true)
+}
+
impl PopulatedDeleteFileIndex {
/// Creates a new populated delete file index from a list of delete file
contexts, which
/// allows for fast lookup when determining which delete files apply to a
given data file.
///
/// 1. The partition information is extracted from each delete file's
manifest entry.
/// 2. If the partition is empty and the delete file is not a positional
delete,
/// it is added to the `global_equality_deletes` vector
- /// 3. Otherwise, the delete file is added to one of two hash maps based
on its content type.
+ /// 3. A positional delete that names a single data file is keyed by that
path.
+ /// 4. Any other delete file is keyed by partition, in the map for its
content type.
fn new(files: Vec<DeleteFileContext>) -> PopulatedDeleteFileIndex {
- let mut eq_deletes_by_partition: HashMap<Struct,
Vec<Arc<DeleteFileContext>>> =
- HashMap::default();
- let mut pos_deletes_by_partition: HashMap<Struct,
Vec<Arc<DeleteFileContext>>> =
+ let mut eq_deletes_by_partition: HashMap<
+ i32,
+ HashMap<Struct, Vec<Arc<DeleteFileContext>>>,
+ > = HashMap::default();
+ let mut pos_deletes_by_partition: HashMap<
+ i32,
+ HashMap<Struct, Vec<Arc<DeleteFileContext>>>,
+ > = HashMap::default();
+ let mut pos_deletes_by_path: HashMap<String,
Vec<Arc<DeleteFileContext>>> =
HashMap::default();
let mut global_equality_deletes: Vec<Arc<DeleteFileContext>> = vec![];
files.into_iter().for_each(|ctx| {
let arc_ctx = Arc::new(ctx);
+ if arc_ctx.manifest_entry.sequence_number().is_none() {
+ tracing::warn!(
Review Comment:
@xanderbailey Sorry for the late response, I was busy with other
assignments. Unfortunately I currently do not have the time to follow up on
this, so I'd be grateful if you create the issue.
--
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]
