Skip to content

Commit

Permalink
Fix linters
Browse files Browse the repository at this point in the history
  • Loading branch information
liurenjie1024 committed Nov 20, 2023
1 parent 39a7387 commit 4db89a8
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 35 deletions.
9 changes: 2 additions & 7 deletions crates/catalog/rest/src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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()?,
};
Expand Down
56 changes: 46 additions & 10 deletions crates/iceberg/src/catalog/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -290,31 +293,36 @@ 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,
},
/// The table's last assigned partition id must match the
/// 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,
},
Expand All @@ -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<i32>,
},
/// Set table's current schema
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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<i64> },
RemoveSnapshots {
/// Snapshot ids to remove.
snapshot_ids: Vec<i64>,
},
/// 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<String, String> },
SetProperties {
/// Properties to update for table.
updates: HashMap<String, String>,
},
/// Remove table's properties
RemoveProperties { removals: Vec<String> },
RemoveProperties {
/// Properties to remove
removals: Vec<String>,
},
}

#[cfg(test)]
Expand All @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion crates/iceberg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! Native Rust implementation of Apache Iceberg
// #![deny(missing_docs)]
#![deny(missing_docs)]

#[macro_use]
extern crate derive_builder;
Expand Down
4 changes: 1 addition & 3 deletions crates/iceberg/src/spec/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -50,7 +50,6 @@ pub struct Schema {

name_to_id: HashMap<String, i32>,
id_to_name: HashMap<i32, String>,
lower_case_name_to_id: OnceLock<HashMap<String, i32>>,
}

impl PartialEq for Schema {
Expand Down Expand Up @@ -127,7 +126,6 @@ impl SchemaBuilder {

name_to_id,
id_to_name,
lower_case_name_to_id: OnceLock::default(),
})
}

Expand Down
35 changes: 21 additions & 14 deletions crates/iceberg/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Self> {
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)
}
Expand All @@ -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![],
Expand Down Expand Up @@ -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<Self> {
pub fn asc(self, name: &str, null_order: NullOrder) -> Result<Self> {
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<Self> {
pub fn desc(self, name: &str, null_order: NullOrder) -> Result<Self> {
self.add_sort_field(name, SortDirection::Descending, null_order)
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 4db89a8

Please sign in to comment.