Merge pull request JanKaul#237 from JanKaul/fix-builder-errors

JanKaul · web-flow · commit 5cfe00226677 · 2025-08-11T12:13:45.000+02:00
Fix builder errors
diff --git a/datafusion_iceberg/src/error.rs b/datafusion_iceberg/src/error.rs
@@ -56,7 +56,7 @@ pub enum Error {
     ParseInt(#[from] std::num::ParseIntError),
     /// parse int error
     #[error(transparent)]
-    ConfigBuilder(#[from] crate::table::DataFusionTableConfigBuilderError),
+    DeriveBuilder(#[from] derive_builder::UninitializedFieldError),
 }
 
 impl From<Error> for DataFusionError {
diff --git a/datafusion_iceberg/src/table.rs b/datafusion_iceberg/src/table.rs
@@ -107,6 +107,7 @@ impl From<MaterializedView> for DataFusionTable {
 }
 
 #[derive(Clone, Debug, Builder)]
+#[builder(build_fn(error = "Error"))]
 pub struct DataFusionTableConfig {
     /// With this option, an additional "__data_file_path" column is added to the output of the
     /// TableProvider that contains the path of the data-file the row originates from.
diff --git a/iceberg-rust-spec/src/error.rs b/iceberg-rust-spec/src/error.rs
@@ -55,30 +55,9 @@ pub enum Error {
     /// parse int error
     #[error(transparent)]
     ParseInt(#[from] std::num::ParseIntError),
-    /// table metadata builder
+    /// derive builder
     #[error(transparent)]
-    TableMetadataBuilder(#[from] crate::spec::table_metadata::TableMetadataBuilderError),
-    /// view metadata builder
-    #[error(transparent)]
-    ViewMetadataBuilder(#[from] crate::spec::view_metadata::GeneralViewMetadataBuilderError),
-    /// version builder
-    #[error(transparent)]
-    VersionBuilder(#[from] crate::spec::view_metadata::VersionBuilderError),
-    /// manifest builder
-    #[error(transparent)]
-    ManifestEntryBuilder(#[from] crate::spec::manifest::ManifestEntryBuilderError),
-    /// datafile builder
-    #[error(transparent)]
-    DatafileBuilder(#[from] crate::spec::manifest::DataFileBuilderError),
-    /// snapshot builder
-    #[error(transparent)]
-    SnapshotBuilder(#[from] crate::spec::snapshot::SnapshotBuilderError),
-    /// structype builder
-    #[error(transparent)]
-    StructTypeBuilder(#[from] crate::spec::types::StructTypeBuilderError),
-    /// partition spec builder
-    #[error(transparent)]
-    PartitionSpec(#[from] crate::spec::partition::PartitionSpecBuilderError),
+    DeriveBuilder(#[from] derive_builder::UninitializedFieldError),
 }
 
 impl From<apache_avro::Error> for Error {
diff --git a/iceberg-rust-spec/src/spec/manifest.rs b/iceberg-rust-spec/src/spec/manifest.rs
@@ -34,7 +34,7 @@ use super::{
 /// Entry in manifest with the iceberg spec version 2.
 #[derive(Debug, Serialize, PartialEq, Clone, Getters, Builder)]
 #[serde(into = "ManifestEntryEnum")]
-#[builder(setter(prefix = "with"))]
+#[builder(build_fn(error = "Error"), setter(prefix = "with"))]
 pub struct ManifestEntry {
     /// Table format version
     format_version: FormatVersion,
@@ -516,7 +516,7 @@ impl From<HashMap<i32, Value>> for AvroMap<ByteBuf> {
 }
 
 #[derive(Debug, PartialEq, Clone, Getters, Builder)]
-#[builder(setter(prefix = "with"))]
+#[builder(build_fn(error = "Error"), setter(prefix = "with"))]
 /// DataFile found in Manifest.
 pub struct DataFile {
     ///Type of content in data file.
diff --git a/iceberg-rust-spec/src/spec/materialized_view_metadata.rs b/iceberg-rust-spec/src/spec/materialized_view_metadata.rs
@@ -29,7 +29,7 @@ pub type MaterializedViewMetadata = GeneralViewMetadata<FullIdentifier>;
 pub type MaterializedViewMetadataBuilder = GeneralViewMetadataBuilder<FullIdentifier>;
 
 impl MaterializedViewMetadata {
-    pub fn as_ref(&self) -> TabularMetadataRef {
+    pub fn as_ref(&self) -> TabularMetadataRef<'_> {
         TabularMetadataRef::MaterializedView(self)
     }
 }
diff --git a/iceberg-rust-spec/src/spec/partition.rs b/iceberg-rust-spec/src/spec/partition.rs
@@ -169,7 +169,7 @@ impl PartitionField {
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Default, Builder, Getters)]
 #[serde(rename_all = "kebab-case")]
-#[builder(setter(prefix = "with"))]
+#[builder(build_fn(error = "Error"), setter(prefix = "with"))]
 ///  Partition spec that defines how to produce a tuple of partition values from a record.
 pub struct PartitionSpec {
     /// Identifier for PartitionSpec
diff --git a/iceberg-rust-spec/src/spec/snapshot.rs b/iceberg-rust-spec/src/spec/snapshot.rs
@@ -29,7 +29,7 @@ use _serde::SnapshotEnum;
 
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Builder, Getters)]
 #[serde(from = "SnapshotEnum", into = "SnapshotEnum")]
-#[builder(setter(prefix = "with"))]
+#[builder(build_fn(error = "Error"), setter(prefix = "with"))]
 /// A snapshot represents the state of a table at some time and is used to access the complete set of data files in the table.
 pub struct Snapshot {
     /// A unique long ID
diff --git a/iceberg-rust-spec/src/spec/table_metadata.rs b/iceberg-rust-spec/src/spec/table_metadata.rs
@@ -204,7 +204,7 @@ impl TableMetadata {
     pub fn current_partition_fields(
         &self,
         branch: Option<&str>,
-    ) -> Result<Vec<BoundPartitionField>, Error> {
+    ) -> Result<Vec<BoundPartitionField<'_>>, Error> {
         let schema = self.current_schema(branch)?;
         let partition_spec = self.default_partition_spec()?;
         partition_fields(partition_spec, schema)
@@ -218,7 +218,10 @@ impl TableMetadata {
     /// # Returns
     /// * `Result<Vec<BoundPartitionField>, Error>` - Vector of partition fields bound to their source schema fields,
     ///   or an error if the schema or partition spec cannot be found
-    pub fn partition_fields(&self, snapshot_id: i64) -> Result<Vec<BoundPartitionField>, Error> {
+    pub fn partition_fields(
+        &self,
+        snapshot_id: i64,
+    ) -> Result<Vec<BoundPartitionField<'_>>, Error> {
         let schema = self.schema(snapshot_id)?;
         self.default_partition_spec()?
             .fields()
@@ -325,7 +328,7 @@ impl TableMetadata {
             .map(|x| *x.sequence_number())
     }
 
-    pub fn as_ref(&self) -> TabularMetadataRef {
+    pub fn as_ref(&self) -> TabularMetadataRef<'_> {
         TabularMetadataRef::Table(self)
     }
 }
diff --git a/iceberg-rust-spec/src/spec/types.rs b/iceberg-rust-spec/src/spec/types.rs
@@ -196,6 +196,7 @@ impl fmt::Display for PrimitiveType {
 /// DataType for a specific struct
 #[derive(Debug, Serialize, PartialEq, Eq, Clone, Builder)]
 #[serde(rename = "struct", tag = "type")]
+#[builder(build_fn(error = "Error"))]
 pub struct StructType {
     /// Struct fields
     #[builder(setter(each(name = "with_struct_field")))]
diff --git a/iceberg-rust-spec/src/spec/view_metadata.rs b/iceberg-rust-spec/src/spec/view_metadata.rs
@@ -143,7 +143,7 @@ impl<T: Materialization> GeneralViewMetadata<T> {
 }
 
 impl ViewMetadata {
-    pub fn as_ref(&self) -> TabularMetadataRef {
+    pub fn as_ref(&self) -> TabularMetadataRef<'_> {
         TabularMetadataRef::View(self)
     }
 }
diff --git a/iceberg-rust/src/catalog/create.rs b/iceberg-rust/src/catalog/create.rs
@@ -53,7 +53,7 @@ use super::{identifier::Identifier, Catalog};
 /// can be serialized/deserialized using serde.
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Builder)]
 #[serde(rename_all = "kebab-case")]
-#[builder(build_fn(name = "create"), setter(prefix = "with"))]
+#[builder(build_fn(name = "create", error = "Error"), setter(prefix = "with"))]
 pub struct CreateTable {
     #[builder(setter(into))]
     /// Name of the table
@@ -175,7 +175,7 @@ impl TryInto<TableMetadata> for CreateTable {
 /// can be serialized/deserialized using serde.
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Builder)]
 #[serde(rename_all = "kebab-case")]
-#[builder(build_fn(name = "create"), setter(prefix = "with"))]
+#[builder(build_fn(name = "create", error = "Error"), setter(prefix = "with"))]
 pub struct CreateView<T: Materialization> {
     /// Name of the view
     #[builder(setter(into))]
@@ -285,7 +285,7 @@ impl TryInto<MaterializedViewMetadata> for CreateView<FullIdentifier> {
 /// can be serialized/deserialized using serde.
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Builder)]
 #[serde(rename_all = "kebab-case")]
-#[builder(build_fn(name = "create"), setter(prefix = "with"))]
+#[builder(build_fn(name = "create", error = "Error"), setter(prefix = "with"))]
 pub struct CreateMaterializedView {
     /// Name of the view
     #[builder(setter(into))]
diff --git a/iceberg-rust/src/error.rs b/iceberg-rust/src/error.rs
@@ -89,30 +89,9 @@ pub enum Error {
     /// parse int error
     #[error(transparent)]
     ParseInt(#[from] std::num::ParseIntError),
-    /// table metadata builder
+    /// derive builder
     #[error(transparent)]
-    TableMetadataBuilder(
-        #[from] iceberg_rust_spec::spec::table_metadata::TableMetadataBuilderError,
-    ),
-    /// view metadata builder
-    #[error(transparent)]
-    ViewMetadataBuilder(
-        #[from] iceberg_rust_spec::spec::view_metadata::GeneralViewMetadataBuilderError,
-    ),
-    /// version builder
-    #[error(transparent)]
-    VersionBuilder(#[from] iceberg_rust_spec::spec::view_metadata::VersionBuilderError),
-    /// create table builder
-    #[error(transparent)]
-    CreateTableBuilder(#[from] crate::catalog::create::CreateTableBuilderError),
-    /// create view builder
-    #[error(transparent)]
-    CreateViewBuilder(#[from] crate::catalog::create::CreateViewBuilderError),
-    /// create view builder
-    #[error(transparent)]
-    CreateMaterializedViewBuilder(
-        #[from] crate::catalog::create::CreateMaterializedViewBuilderError,
-    ),
+    DeriveBuilder(#[from] derive_builder::UninitializedFieldError),
 }
 
 impl From<apache_avro::Error> for Error {
diff --git a/iceberg-rust/src/file_format/parquet.rs b/iceberg-rust/src/file_format/parquet.rs
@@ -257,9 +257,7 @@ pub fn parquet_to_datafile(
         builder.with_equality_ids(Some(equality_ids.to_vec()));
     }
 
-    let content = builder
-        .build()
-        .map_err(iceberg_rust_spec::error::Error::from)?;
+    let content = builder.build()?;
     Ok(content)
 }
 
diff --git a/iceberg-rust/src/materialized_view/mod.rs b/iceberg-rust/src/materialized_view/mod.rs
@@ -152,7 +152,7 @@ impl MaterializedView {
     ///
     /// # Returns
     /// A new transaction that can be used to perform multiple operations atomically
-    pub fn new_transaction(&mut self, branch: Option<&str>) -> MaterializedViewTransaction {
+    pub fn new_transaction(&mut self, branch: Option<&str>) -> MaterializedViewTransaction<'_> {
         MaterializedViewTransaction::new(self, branch)
     }
     /// Returns the storage table that contains the materialized data for this view
diff --git a/iceberg-rust/src/object_store/mod.rs b/iceberg-rust/src/object_store/mod.rs
@@ -40,7 +40,7 @@ impl Display for Bucket<'_> {
 
 impl Bucket<'_> {
     /// Get the bucket and coud provider from the location string
-    pub fn from_path(path: &str) -> Result<Bucket, Error> {
+    pub fn from_path(path: &str) -> Result<Bucket<'_>, Error> {
         if path.starts_with("s3://") || path.starts_with("s3a://") {
             let prefix = if path.starts_with("s3://") {
                 "s3://"
diff --git a/iceberg-rust/src/table/manifest.rs b/iceberg-rust/src/table/manifest.rs
@@ -37,7 +37,7 @@ use iceberg_rust_spec::{
 };
 use object_store::ObjectStore;
 
-use crate::{error::Error, spec};
+use crate::error::Error;
 
 type ReaderZip<'a, R> = Zip<AvroReader<'a, R>, Repeat<Arc<(Schema, PartitionSpec, FormatVersion)>>>;
 type ReaderMap<'a, R> = Map<
@@ -130,8 +130,7 @@ impl<R: Read> ManifestReader<'_, R> {
         let partition_spec = PartitionSpec::builder()
             .with_spec_id(spec_id)
             .with_fields(partition_fields)
-            .build()
-            .map_err(spec::error::Error::from)?;
+            .build()?;
         Ok(Self {
             reader: reader
                 .zip(repeat(Arc::new((schema, partition_spec, format_version))))
diff --git a/iceberg-rust/src/table/mod.rs b/iceberg-rust/src/table/mod.rs
@@ -287,7 +287,7 @@ impl Table {
     ///
     /// The transaction must be committed for any changes to take effect.
     /// Multiple operations can be chained within a single transaction.
-    pub fn new_transaction(&mut self, branch: Option<&str>) -> TableTransaction {
+    pub fn new_transaction(&mut self, branch: Option<&str>) -> TableTransaction<'_> {
         TableTransaction::new(self, branch)
     }
 }
diff --git a/iceberg-rust/src/table/transaction/operation.rs b/iceberg-rust/src/table/transaction/operation.rs
@@ -150,7 +150,6 @@ impl Operation {
                                 .with_status(Status::Added)
                                 .with_data_file(data_file)
                                 .build()
-                                .map_err(crate::spec::error::Error::from)
                                 .map_err(Error::from)
                         });
 
@@ -197,9 +196,7 @@ impl Operation {
                 if let Some(snapshot) = old_snapshot {
                     snapshot_builder.with_parent_snapshot_id(*snapshot.snapshot_id());
                 }
-                let snapshot = snapshot_builder
-                    .build()
-                    .map_err(iceberg_rust_spec::error::Error::from)?;
+                let snapshot = snapshot_builder.build()?;
 
                 Ok((
                     old_snapshot.map(|x| TableRequirement::AssertRefSnapshotId {
@@ -254,7 +251,6 @@ impl Operation {
                         .with_sequence_number(sequence_number)
                         .with_data_file(data_file.clone())
                         .build()
-                        .map_err(crate::spec::error::Error::from)
                         .map_err(Error::from)
                 });
 
@@ -346,9 +342,7 @@ impl Operation {
                         operation: iceberg_rust_spec::spec::snapshot::Operation::Overwrite,
                         other: additional_summary.unwrap_or_default(),
                     });
-                let snapshot = snapshot_builder
-                    .build()
-                    .map_err(iceberg_rust_spec::error::Error::from)?;
+                let snapshot = snapshot_builder.build()?;
 
                 Ok((
                     old_snapshot.map(|x| TableRequirement::AssertRefSnapshotId {
@@ -423,7 +417,6 @@ impl Operation {
                         .with_status(Status::Added)
                         .with_data_file(data_file)
                         .build()
-                        .map_err(crate::spec::error::Error::from)
                         .map_err(Error::from)
                 });
 
@@ -488,9 +481,7 @@ impl Operation {
                             .schema_id(),
                     );
                 snapshot_builder.with_parent_snapshot_id(*old_snapshot.snapshot_id());
-                let snapshot = snapshot_builder
-                    .build()
-                    .map_err(iceberg_rust_spec::error::Error::from)?;
+                let snapshot = snapshot_builder.build()?;
 
                 Ok((
                     Some(TableRequirement::AssertRefSnapshotId {
diff --git a/iceberg-rust/src/view/mod.rs b/iceberg-rust/src/view/mod.rs
@@ -181,7 +181,7 @@ impl View {
     ///
     /// Transactions ensure that all changes are applied atomically with ACID guarantees.
     /// Multiple operations can be chained and will be committed together.
-    pub fn new_transaction(&mut self, branch: Option<&str>) -> ViewTransaction {
+    pub fn new_transaction(&mut self, branch: Option<&str>) -> ViewTransaction<'_> {
         ViewTransaction::new(self, branch)
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ pub enum Error {`
`56`	`56`	`ParseInt(#[from] std::num::ParseIntError),`
`57`	`57`	`/// parse int error`
`58`	`58`	`#[error(transparent)]`
`59`		`- ConfigBuilder(#[from] crate::table::DataFusionTableConfigBuilderError),`
	`59`	`+ DeriveBuilder(#[from] derive_builder::UninitializedFieldError),`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`impl From<Error> for DataFusionError {`
Original file line number	Diff line number	Diff line change
`@@ -107,6 +107,7 @@ impl From<MaterializedView> for DataFusionTable {`
`107`	`107`	`}`
`108`	`108`
`109`	`109`	`#[derive(Clone, Debug, Builder)]`
	`110`	`+#[builder(build_fn(error = "Error"))]`
`110`	`111`	`pub struct DataFusionTableConfig {`
`111`	`112`	`/// With this option, an additional "__data_file_path" column is added to the output of the`
`112`	`113`	`/// TableProvider that contains the path of the data-file the row originates from.`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ pub type MaterializedViewMetadata = GeneralViewMetadata<FullIdentifier>;`
`29`	`29`	`pub type MaterializedViewMetadataBuilder = GeneralViewMetadataBuilder<FullIdentifier>;`
`30`	`30`
`31`	`31`	`impl MaterializedViewMetadata {`
`32`		`- pub fn as_ref(&self) -> TabularMetadataRef {`
	`32`	`+ pub fn as_ref(&self) -> TabularMetadataRef<'_> {`
`33`	`33`	`TabularMetadataRef::MaterializedView(self)`
`34`	`34`	`}`
`35`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ impl<T: Materialization> GeneralViewMetadata<T> {`
`143`	`143`	`}`
`144`	`144`
`145`	`145`	`impl ViewMetadata {`
`146`		`- pub fn as_ref(&self) -> TabularMetadataRef {`
	`146`	`+ pub fn as_ref(&self) -> TabularMetadataRef<'_> {`
`147`	`147`	`TabularMetadataRef::View(self)`
`148`	`148`	`}`
`149`	`149`	`}`