Skip to content

Commit

Permalink
fix clippy warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
@JanKaul
JanKaul committed Oct 24, 2024
1 parent 78f588f commit 24ab564
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 41 deletions.
12 changes: 5 additions & 7 deletions iceberg-file-catalog/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl Catalog for FileCatalog {
.map_err(IcebergError::from)
.map_ok(|x| {
let path = x.location.as_ref();
self.identifier(&path)
self.identifier(path)
})
.try_collect()
.await
Expand Down Expand Up @@ -501,15 +501,15 @@ impl Catalog for FileCatalog {

impl FileCatalog {
fn namespace_path(&self, namespace: &str) -> String {
self.path.as_str().trim_end_matches('/').to_owned() + "/" + &self.name + "/" + &namespace
self.path.as_str().trim_end_matches('/').to_owned() + "/" + &self.name + "/" + namespace
}

fn tabular_path(&self, namespace: &str, name: &str) -> String {
self.path.as_str().trim_end_matches('/').to_owned()
+ "/"
+ &self.name
+ "/"
+ &namespace
+ namespace
+ "/"
+ name
}
Expand All @@ -523,7 +523,7 @@ impl FileCatalog {
.try_filter(|x| {
future::ready(
x.ends_with("metadata.json")
&& x.starts_with(&(path.clone() + "/v").trim_start_matches('/')),
&& x.starts_with((path.clone() + "/v").trim_start_matches('/')),
)
})
.try_collect()
Expand Down Expand Up @@ -551,9 +551,7 @@ impl FileCatalog {
let parts = path
.trim_start_matches(self.path.trim_start_matches('/'))
.trim_start_matches('/')
.split('/')
.skip(1)
.next()
.split('/').nth(1)
.ok_or(IcebergError::InvalidFormat("Namespace in path".to_owned()))?
.to_owned();
Namespace::try_new(&[parts]).map_err(IcebergError::from)
Expand Down
6 changes: 3 additions & 3 deletions iceberg-rust-spec/src/spec/manifest_list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl<'a, 'metadata, R: Read> ManifestListReader<'a, 'metadata, R> {
FormatVersion::V2 => manifest_list_schema_v2(),
};
Ok(Self {
reader: AvroReader::with_schema(&schema, reader)?
reader: AvroReader::with_schema(schema, reader)?
.zip(repeat(table_metadata))
.map(avro_value_to_manifest_file),
})
Expand Down Expand Up @@ -781,7 +781,7 @@ mod tests {

let schema = manifest_list_schema_v2();

let mut writer = apache_avro::Writer::new(&schema, Vec::new());
let mut writer = apache_avro::Writer::new(schema, Vec::new());

writer.append_ser(manifest_file.clone()).unwrap();

Expand Down Expand Up @@ -861,7 +861,7 @@ mod tests {

let schema = manifest_list_schema_v1();

let mut writer = apache_avro::Writer::new(&schema, Vec::new());
let mut writer = apache_avro::Writer::new(schema, Vec::new());

writer.append_ser(manifest_file.clone()).unwrap();

Expand Down
13 changes: 12 additions & 1 deletion iceberg-rust-spec/src/spec/values.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ use std::{
any::Any,
collections::{btree_map::Keys, BTreeMap, HashMap},
fmt,
hash::Hash,
io::Cursor,
slice::Iter,
};

use chrono::{DateTime, NaiveDate, NaiveDateTime, NaiveTime, TimeZone, Utc};
use itertools::Itertools;
use ordered_float::OrderedFloat;
use rust_decimal::Decimal;
use serde::{
Expand Down Expand Up @@ -128,7 +130,7 @@ impl fmt::Display for Value {
/// The partition struct stores the tuple of partition values for each file.
/// Its type is derived from the partition fields of the partition spec used to write the manifest file.
/// In v2, the partition struct’s field ids must match the ids from the partition spec.
#[derive(Debug, Clone, Eq, Hash, PartialOrd, Ord)]
#[derive(Debug, Clone, Eq, PartialOrd, Ord)]
pub struct Struct {
/// Vector to store the field values
pub fields: Vec<Option<Value>>,
Expand Down Expand Up @@ -273,6 +275,15 @@ impl PartialEq for Struct {
}
}

impl Hash for Struct {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
for key in self.keys().sorted() {
key.hash(state);
self.get(&key).hash(state);
}
}
}

impl Value {
/// Perform a partition transformation for the given value
pub fn tranform(&self, transform: &Transform) -> Result<Value, Error> {
Expand Down
1 change: 1 addition & 0 deletions iceberg-rust/src/table/transaction/append.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
};

/// Split sets of datafiles depending on their partition_values
#[allow(clippy::type_complexity)]
pub(crate) fn split_datafiles_once(
files: impl Iterator<Item = Result<ManifestEntry, Error>>,
rect: Rectangle<Value>,
Expand Down
46 changes: 20 additions & 26 deletions iceberg-rust/src/table/transaction/operation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,10 @@ impl Operation {

let bounding_partition_values = files
.iter()
.fold(Ok::<_, Error>(None), |acc, x| {
let acc = acc?;
.try_fold(None, |acc, x| {
let node = struct_to_smallvec(x.partition(), &partition_column_names)?;
let Some(mut acc) = acc else {
return Ok(Some(Rectangle::new(node.clone(), node)));
return Ok::<_, Error>(Some(Rectangle::new(node.clone(), node)));
};
acc.expand_with_node(node);
Ok(Some(acc))
Expand All @@ -129,7 +128,7 @@ impl Operation {
};

let mut manifest_list_writer =
apache_avro::Writer::new(&manifest_list_schema, Vec::new());
apache_avro::Writer::new(manifest_list_schema, Vec::new());

let old_manifest_list_location = old_snapshot.map(|x| x.manifest_list()).cloned();

Expand All @@ -139,29 +138,27 @@ impl Operation {
let manifest = if let Some(old_manifest_list_location) = &old_manifest_list_location
{
let old_manifest_list_bytes = object_store
.get(&strip_prefix(&old_manifest_list_location).as_str().into())
.get(&strip_prefix(old_manifest_list_location).as_str().into())
.await?
.bytes()
.await?;

let manifest_list_reader =
let mut manifest_list_reader =
ManifestListReader::new(old_manifest_list_bytes.as_ref(), table_metadata)?;

// Check if table is partitioned
let manifest = if partition_column_names.is_empty() {
// Find the manifest with the lowest row count
manifest_list_reader
.fold(Ok::<_, Error>(None), |acc, x| {
let acc = acc?;

.try_fold(None, |acc, x| {
let manifest = x?;

let row_count = manifest.added_rows_count;

file_count += manifest.added_files_count.unwrap_or(0) as usize;

let Some((old_row_count, old_manifest)) = acc else {
return Ok(Some((row_count, manifest)));
return Ok::<_, Error>(Some((row_count, manifest)));
};

let Some(row_count) = row_count else {
Expand All @@ -183,9 +180,7 @@ impl Operation {
} else {
// Find the manifest with the smallest bounding partition values
manifest_list_reader
.fold(Ok::<_, Error>(None), |acc, x| {
let acc = acc?;

.try_fold(None, |acc, x| {
let manifest = x?;

let mut bounds = summary_to_rectangle(
Expand All @@ -200,7 +195,7 @@ impl Operation {
file_count += manifest.added_files_count.unwrap_or(0) as usize;

let Some((old_bounds, old_manifest)) = acc else {
return Ok(Some((bounds, manifest)));
return Ok::<_, Error>(Some((bounds, manifest)));
};

match old_bounds.cmp_with_priority(&bounds)? {
Expand Down Expand Up @@ -307,8 +302,9 @@ impl Operation {
+ "-m"
+ &0.to_string()
+ ".avro";
let manifest = ManifestListEntry {
format_version: table_metadata.format_version.clone(),

ManifestListEntry {
format_version: table_metadata.format_version,
manifest_path: manifest_location,
manifest_length: 0,
partition_spec_id: table_metadata.default_spec_id,
Expand All @@ -324,8 +320,7 @@ impl Operation {
deleted_rows_count: Some(0),
partitions: None,
key_metadata: None,
};
manifest
}
});

for manifest_entry in new_datafile_iter {
Expand Down Expand Up @@ -427,7 +422,7 @@ impl Operation {
+ &i.to_string()
+ ".avro";
let mut manifest = ManifestListEntry {
format_version: table_metadata.format_version.clone(),
format_version: table_metadata.format_version,
manifest_path: manifest_location,
manifest_length: 0,
partition_spec_id: table_metadata.default_spec_id,
Expand Down Expand Up @@ -564,11 +559,10 @@ impl Operation {

let bounding_partition_values = files
.iter()
.fold(Ok::<_, Error>(None), |acc, x| {
let acc = acc?;
.try_fold(None, |acc, x| {
let node = struct_to_smallvec(x.partition(), &partition_column_names)?;
let Some(mut acc) = acc else {
return Ok(Some(Rectangle::new(node.clone(), node)));
return Ok::<_, Error>(Some(Rectangle::new(node.clone(), node)));
};
acc.expand_with_node(node);
Ok(Some(acc))
Expand All @@ -584,7 +578,7 @@ impl Operation {
};

let mut manifest_list_writer =
apache_avro::Writer::new(&manifest_list_schema, Vec::new());
apache_avro::Writer::new(manifest_list_schema, Vec::new());

let new_file_count = files.len();

Expand Down Expand Up @@ -641,7 +635,7 @@ impl Operation {
+ &0.to_string()
+ ".avro";
let mut manifest = ManifestListEntry {
format_version: table_metadata.format_version.clone(),
format_version: table_metadata.format_version,
manifest_path: manifest_location,
manifest_length: 0,
partition_spec_id: table_metadata.default_spec_id,
Expand Down Expand Up @@ -739,7 +733,7 @@ impl Operation {
+ &i.to_string()
+ ".avro";
let mut manifest = ManifestListEntry {
format_version: table_metadata.format_version.clone(),
format_version: table_metadata.format_version,
manifest_path: manifest_location,
manifest_length: 0,
partition_spec_id: table_metadata.default_spec_id,
Expand Down Expand Up @@ -901,7 +895,7 @@ fn update_partitions(
partition_values: &Struct,
partition_columns: &[PartitionField],
) -> Result<(), Error> {
for (field, summary) in partition_columns.into_iter().zip(partitions.iter_mut()) {
for (field, summary) in partition_columns.iter().zip(partitions.iter_mut()) {
let value = partition_values.get(field.name()).and_then(|x| x.as_ref());
if let Some(value) = value {
if summary.lower_bound.is_none() {
Expand Down
8 changes: 4 additions & 4 deletions iceberg-rust/src/util/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ where
}

pub(crate) fn contains(&self, rect: &Rectangle<C>) -> bool {
if self.min.len() == 0 {
if self.min.is_empty() {
return false;
}
for i in 0..self.min.len() {
Expand All @@ -93,8 +93,8 @@ pub(crate) fn struct_to_smallvec(
names
.iter()
.map(|x| {
s.get(*x)
.and_then(|x| identity(x).as_ref().map(Clone::clone))
s.get(x)
.and_then(|x| identity(x).clone())
})
.collect::<Option<SmallVec<_>>>()
.ok_or(Error::InvalidFormat("Partition struct".to_owned()))
Expand Down Expand Up @@ -125,7 +125,7 @@ pub(crate) fn summary_to_rectangle(summaries: &[FieldSummary]) -> Result<Rectang
pub(crate) fn cmp_dist<C: PartialOrd>(left: &[C], right: &[C]) -> Result<Ordering, Error> {
while let (Some(own), Some(other)) = (left.iter().next(), right.iter().next()) {
let ordering = own
.partial_cmp(&other)
.partial_cmp(other)
.ok_or(Error::InvalidFormat("Types for Partial Order".to_owned()))?;
let Ordering::Equal = ordering else {
return Ok(ordering);
Expand Down

0 comments on commit 24ab564

Please sign in to comment.