Skip to content

Commit

Permalink
refactor: minor cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
sdd committed Oct 10, 2024
1 parent 28021a4 commit 2732a49
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 33 deletions.
33 changes: 21 additions & 12 deletions crates/iceberg/src/arrow/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,18 +283,27 @@ impl ArrowReader {
fn get_equality_deletes(_delete_files: &[Deletes]) -> Result<Option<BoundPredicate>> {
let result = None;

// TODO:
// reject DeleteFiles that are not equality deletes
// * for each delete file:
// * set `file_predicate` = AlwaysTrue
// * for each row in the file:
// * for each cell in the row:
// * create a predicate of the form `field` = `val`
// where `field` is the column name and `val` is the value
// of the cell
// * Bind this predicate to the table schema to get a `BoundPredicate`
// * set file_predicate = file_predicate.and(bound_predicate_for_cell)
// * set result = result.or(file_predicate)
for delete_file in _delete_files {
let Deletes::Equality(_delete_file_batches) = delete_file else {
continue;
};

return Err(Error::new(
ErrorKind::FeatureUnsupported,
"Equality delete files are not yet supported.",
));

// TODO:
// * set `file_predicate` = AlwaysTrue
// * for each row in the file:
// * for each cell in the row:
// * create a predicate of the form `field` = `val`
// where `field` is the column name and `val` is the value
// of the cell
// * Bind this predicate to the table schema to get a `BoundPredicate`
// * set file_predicate = file_predicate.and(bound_predicate_for_cell)
// * set result = result.or(file_predicate)
}

Ok(result)
}
Expand Down
18 changes: 1 addition & 17 deletions crates/iceberg/src/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,35 +572,19 @@ impl TableScan {
}

if let Some(ref bound_predicates) = manifest_entry_context.bound_predicates {
let BoundPredicates {
// ref snapshot_bound_predicate,
ref partition_bound_predicate,
..
} = bound_predicates.as_ref();

let expression_evaluator_cache =
manifest_entry_context.expression_evaluator_cache.as_ref();

let expression_evaluator = expression_evaluator_cache.get(
manifest_entry_context.partition_spec_id,
partition_bound_predicate,
&bound_predicates.partition_bound_predicate,
)?;

// skip any data file whose partition data indicates that it can't contain
// any data that matches this scan's filter
if !expression_evaluator.eval(manifest_entry_context.manifest_entry.data_file())? {
return Ok(());
}

// TODO: I don't think that this is required. Need to confirm
// skip any data file whose metrics don't match this scan's filter
// if !InclusiveMetricsEvaluator::eval(
// snapshot_bound_predicate,
// manifest_entry_context.manifest_entry.data_file(),
// false,
// )? {
// return Ok(());
// }
}

file_scan_task_delete_file_tx
Expand Down
6 changes: 2 additions & 4 deletions crates/iceberg/src/spec/delete_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ use crate::{Error, ErrorKind, Result};

pub(crate) const FIELD_ID_DELETE_FILE_PATH: i32 = i32::MAX - 101;
pub(crate) const FIELD_ID_DELETE_POS: i32 = i32::MAX - 102;
// pub(crate) const FIELD_ID_DELETE_ROW: i32 = i32::MAX - 103;

pub(crate) const FIELD_NAME_DELETE_FILE_PATH: &str = "file_path";
pub(crate) const FIELD_NAME_DELETE_POS: &str = "pos";
// pub(crate) const FIELD_NAME_DELETE_ROW: &str = "row";

// Represents a parsed Delete file that can be safely stored
// in the Object Cache.
Expand Down Expand Up @@ -111,8 +109,8 @@ fn validate_schema(schema: SchemaRef) -> Result<PosDelSchema> {
} else if fields.len() == 2 {
Ok(PosDelSchema::WithoutRow)
} else {
// TODO: should check that col 3 is of type Struct
// and that it contains a subset of the table schema
// TODO: should we check that col 3 is of type Struct
// and that it contains a subset of the table schema?
Ok(PosDelSchema::WithRow)
}
}
Expand Down

0 comments on commit 2732a49

Please sign in to comment.