diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index cbe96772e..1805dab68 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -433,7 +433,7 @@ impl Catalog for RestCatalog { let request = self .client .0 - .post(self.config.table_endpoint(&commit.identifier())) + .post(self.config.table_endpoint(commit.identifier())) .json(&CommitTableRequest { identifier: commit.identifier().clone(), requirements: commit.take_requirements(), @@ -501,12 +501,7 @@ impl RestCatalog { props.extend(config); } - let file_io = match self - .config - .warehouse - .as_deref() - .or_else(|| metadata_location) - { + let file_io = match self.config.warehouse.as_deref().or(metadata_location) { Some(url) => FileIO::from_path(url)?.with_props(props).build()?, None => FileIOBuilder::new("s3").with_props(props).build()?, }; diff --git a/crates/iceberg/src/catalog/mod.rs b/crates/iceberg/src/catalog/mod.rs index 7e5cfff9d..ce3c5e7a0 100644 --- a/crates/iceberg/src/catalog/mod.rs +++ b/crates/iceberg/src/catalog/mod.rs @@ -275,7 +275,10 @@ pub enum TableRequirement { NotExist, /// The table UUID must match the requirement. #[serde(rename = "assert-table-uuid")] - UuidMatch { uuid: Uuid }, + UuidMatch { + /// Uuid of original table. + uuid: Uuid, + }, /// The table branch or tag identified by the requirement's `reference` must /// reference the requirement's `snapshot-id`. #[serde(rename = "assert-ref-snapshot-id")] @@ -290,12 +293,14 @@ pub enum TableRequirement { /// The table's last assigned column id must match the requirement. #[serde(rename = "assert-last-assigned-field-id")] LastAssignedFieldIdMatch { + /// The last assigned field id of the table to assert. #[serde(rename = "last-assigned-field-id")] last_assigned_field_id: i64, }, /// The table's current schema id must match the requirement. #[serde(rename = "assert-current-schema-id")] CurrentSchemaIdMatch { + /// Current schema id of the table to assert. #[serde(rename = "current-schema-id")] current_schema_id: i64, }, @@ -303,18 +308,21 @@ pub enum TableRequirement { /// requirement. #[serde(rename = "assert-last-assigned-partition-id")] LastAssignedPartitionIdMatch { + /// Last assigned partition id of the table to assert. #[serde(rename = "last-assigned-partition-id")] last_assigned_partition_id: i64, }, /// The table's default spec id must match the requirement. #[serde(rename = "assert-default-spec-id")] DefaultSpecIdMatch { + /// Default spec id of the table to assert. #[serde(rename = "default-spec-id")] default_spec_id: i64, }, /// The table's default sort order id must match the requirement. #[serde(rename = "assert-default-sort-order-id")] DefaultSortOrderIdMatch { + /// Default sort order id of the table to assert. #[serde(rename = "default-sort-order-id")] default_sort_order_id: i64, }, @@ -339,7 +347,9 @@ pub enum TableUpdate { /// Add a new schema to the table #[serde(rename_all = "kebab-case")] AddSchema { + /// The schema to add. schema: Schema, + /// The last column id of the table. last_column_id: Option, }, /// Set table's current schema @@ -349,7 +359,10 @@ pub enum TableUpdate { schema_id: i32, }, /// Add a new partition spec to the table - AddSpec { spec: PartitionSpec }, + AddSpec { + /// The partition spec to add. + spec: PartitionSpec, + }, /// Set table's default spec #[serde(rename_all = "kebab-case")] SetDefaultSpec { @@ -358,7 +371,10 @@ pub enum TableUpdate { }, /// Add sort order to table. #[serde(rename_all = "kebab-case")] - AddSortOrder { sort_order: SortOrder }, + AddSortOrder { + /// Sort order to add. + sort_order: SortOrder, + }, /// Set table's default sort order #[serde(rename_all = "kebab-case")] SetDefaultSortOrder { @@ -367,26 +383,46 @@ pub enum TableUpdate { }, /// Add snapshot to table. #[serde(rename_all = "kebab-case")] - AddSnapshot { snapshot: Snapshot }, + AddSnapshot { + /// Snapshot to add. + snapshot: Snapshot, + }, /// Set table's snapshot ref. #[serde(rename_all = "kebab-case")] SetSnapshotRef { + /// Name of snapshot reference to set. ref_name: String, + /// Snapshot reference to set. #[serde(flatten)] reference: SnapshotReference, }, /// Remove table's snapshots #[serde(rename_all = "kebab-case")] - RemoveSnapshots { snapshot_ids: Vec }, + RemoveSnapshots { + /// Snapshot ids to remove. + snapshot_ids: Vec, + }, /// Remove snapshot reference #[serde(rename_all = "kebab-case")] - RemoveSnapshotRef { ref_name: String }, + RemoveSnapshotRef { + /// Name of snapshot reference to remove. + ref_name: String, + }, /// Update table's location - SetLocation { location: String }, + SetLocation { + /// New location for table. + location: String, + }, /// Update table's properties - SetProperties { updates: HashMap }, + SetProperties { + /// Properties to update for table. + updates: HashMap, + }, /// Remove table's properties - RemoveProperties { removals: Vec }, + RemoveProperties { + /// Properties to remove + removals: Vec, + }, } #[cfg(test)] @@ -401,7 +437,7 @@ mod tests { use serde::de::DeserializeOwned; use serde::Serialize; use std::collections::HashMap; - use std::fmt::{Debug, Display}; + use std::fmt::Debug; use uuid::uuid; #[test] diff --git a/crates/iceberg/src/lib.rs b/crates/iceberg/src/lib.rs index 5bde2a3f2..378164838 100644 --- a/crates/iceberg/src/lib.rs +++ b/crates/iceberg/src/lib.rs @@ -17,7 +17,7 @@ //! Native Rust implementation of Apache Iceberg -// #![deny(missing_docs)] +#![deny(missing_docs)] #[macro_use] extern crate derive_builder; diff --git a/crates/iceberg/src/spec/schema.rs b/crates/iceberg/src/spec/schema.rs index d690c2d22..3aa1f8b5b 100644 --- a/crates/iceberg/src/spec/schema.rs +++ b/crates/iceberg/src/spec/schema.rs @@ -28,7 +28,7 @@ use itertools::Itertools; use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::fmt::{Display, Formatter}; -use std::sync::{Arc, OnceLock}; +use std::sync::Arc; use _serde::SchemaEnum; @@ -50,7 +50,6 @@ pub struct Schema { name_to_id: HashMap, id_to_name: HashMap, - lower_case_name_to_id: OnceLock>, } impl PartialEq for Schema { @@ -127,7 +126,6 @@ impl SchemaBuilder { name_to_id, id_to_name, - lower_case_name_to_id: OnceLock::default(), }) } diff --git a/crates/iceberg/src/transaction.rs b/crates/iceberg/src/transaction.rs index 93ec8ee29..4ea89a297 100644 --- a/crates/iceberg/src/transaction.rs +++ b/crates/iceberg/src/transaction.rs @@ -22,6 +22,7 @@ use crate::spec::{FormatVersion, NullOrder, SortDirection, SortField, SortOrder, use crate::table::Table; use crate::TableUpdate::UpgradeFormatVersion; use crate::{Catalog, Error, ErrorKind, TableCommit, TableRequirement, TableUpdate}; +use std::cmp::Ordering; use std::collections::HashMap; use std::mem::discriminant; @@ -68,16 +69,22 @@ impl<'a> Transaction<'a> { /// Sets table to a new version. pub fn upgrade_table_version(mut self, format_version: FormatVersion) -> Result { let current_version = self.table.metadata().format_version(); - if current_version > format_version { - return Err(Error::new( - ErrorKind::DataInvalid, - format!( - "Cannot downgrade table version from {} to {}", - current_version, format_version - ), - )); - } else if current_version < format_version { - self.append_updates(vec![UpgradeFormatVersion { format_version }])?; + match current_version.cmp(&format_version) { + Ordering::Greater => { + return Err(Error::new( + ErrorKind::DataInvalid, + format!( + "Cannot downgrade table version from {} to {}", + current_version, format_version + ), + )); + } + Ordering::Less => { + self.append_updates(vec![UpgradeFormatVersion { format_version }])?; + } + Ordering::Equal => { + // Do nothing. + } } Ok(self) } @@ -89,7 +96,7 @@ impl<'a> Transaction<'a> { } /// Creates replace sort order action. - pub fn replace_sort_order(mut self) -> ReplaceSortOrderAction<'a> { + pub fn replace_sort_order(self) -> ReplaceSortOrderAction<'a> { ReplaceSortOrderAction { tx: self, sort_fields: vec![], @@ -122,12 +129,12 @@ pub struct ReplaceSortOrderAction<'a> { impl<'a> ReplaceSortOrderAction<'a> { /// Adds a field for sorting in ascending order. - pub fn asc(mut self, name: &str, null_order: NullOrder) -> Result { + pub fn asc(self, name: &str, null_order: NullOrder) -> Result { self.add_sort_field(name, SortDirection::Ascending, null_order) } /// Adds a field for sorting in descending order. - pub fn desc(mut self, name: &str, null_order: NullOrder) -> Result { + pub fn desc(self, name: &str, null_order: NullOrder) -> Result { self.add_sort_field(name, SortDirection::Descending, null_order) } @@ -202,7 +209,7 @@ mod tests { use crate::transaction::Transaction; use crate::{TableIdent, TableRequirement, TableUpdate}; use std::collections::HashMap; - use std::fs::{metadata, File}; + use std::fs::File; use std::io::BufReader; fn make_v1_table() -> Table {