From 8a84a23ea76b7a09c94b3c682dc2888f5075229b Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Mon, 18 Mar 2024 14:46:53 -0400 Subject: [PATCH] Attribute builder and stylize similar to other structs --- examples/quickstart_dense.rs | 5 +- src/array/attribute.rs | 437 +++++++++++++++++++---------------- src/array/mod.rs | 8 +- 3 files changed, 244 insertions(+), 206 deletions(-) diff --git a/examples/quickstart_dense.rs b/examples/quickstart_dense.rs index dc16c3eb..d8086a29 100644 --- a/examples/quickstart_dense.rs +++ b/examples/quickstart_dense.rs @@ -44,11 +44,12 @@ fn create_array() -> TileDBResult<()> { .build() }; - let attribute_a = tiledb::array::Attribute::new( + let attribute_a = tiledb::array::AttributeBuilder::new( &tdb, QUICKSTART_ATTRIBUTE_NAME, tiledb::Datatype::Int32, - )?; + )? + .build(); let schema = tiledb::array::SchemaBuilder::new( &tdb, diff --git a/src/array/attribute.rs b/src/array/attribute.rs index ccb892a9..e4293d33 100644 --- a/src/array/attribute.rs +++ b/src/array/attribute.rs @@ -41,32 +41,6 @@ impl<'ctx> Attribute<'ctx> { *self.raw } - pub fn new( - context: &'ctx Context, - name: &str, - datatype: Datatype, - ) -> TileDBResult { - let c_context = context.as_mut_ptr(); - let mut c_attr: *mut ffi::tiledb_attribute_t = out_ptr!(); - let c_name = cstring!(name); - let res = unsafe { - ffi::tiledb_attribute_alloc( - c_context, - c_name.as_c_str().as_ptr(), - datatype as u32, - &mut c_attr, - ) - }; - if res == ffi::TILEDB_OK { - Ok(Attribute { - context, - raw: RawAttribute::Owned(c_attr), - }) - } else { - Err(context.expect_last_error()) - } - } - pub fn name(&self, ctx: &Context) -> TileDBResult { let mut c_name = std::ptr::null::(); let res = unsafe { @@ -104,144 +78,173 @@ impl<'ctx> Attribute<'ctx> { } } - pub fn set_nullable( - &self, - ctx: &Context, - nullable: bool, - ) -> TileDBResult<()> { - let nullable: u8 = if nullable { 1 } else { 0 }; - let res = unsafe { - ffi::tiledb_attribute_set_nullable( - ctx.as_mut_ptr(), + pub fn is_nullable(&self) -> bool { + let c_context = self.context.as_mut_ptr(); + let mut c_nullable: std::ffi::c_uchar = 0; + let c_ret = unsafe { + ffi::tiledb_attribute_get_nullable( + c_context, *self.raw, - nullable, + &mut c_nullable, ) }; - if res == ffi::TILEDB_OK { - Ok(()) - } else { - Err(ctx.expect_last_error()) - } + assert_eq!(ffi::TILEDB_OK, c_ret); // Rust API should prevent sanity check failure + c_nullable == 1 } - pub fn get_nullable(&self, ctx: &Context) -> TileDBResult { - let mut c_nullable: std::ffi::c_uchar = 0; + pub fn filter_list(&self) -> TileDBResult { + let c_context = self.context.as_mut_ptr(); + let mut flist = FilterList::default(); let res = unsafe { - ffi::tiledb_attribute_get_nullable( - ctx.as_mut_ptr(), + ffi::tiledb_attribute_get_filter_list( + c_context, *self.raw, - &mut c_nullable, + flist.as_mut_ptr_ptr(), ) }; if res == ffi::TILEDB_OK { - Ok(c_nullable == 1) + Ok(flist) } else { - Err(ctx.expect_last_error()) + Err(self.context.expect_last_error()) } } - pub fn set_filter_list( - &self, - ctx: &Context, - filter_list: &FilterList, - ) -> TileDBResult<()> { + pub fn is_var_sized(&self, ctx: &Context) -> TileDBResult { + self.cell_val_num(ctx).map(|num| num == u32::MAX) + } + + pub fn cell_val_num(&self, ctx: &Context) -> TileDBResult { + let mut c_num: std::ffi::c_uint = 0; let res = unsafe { - ffi::tiledb_attribute_set_filter_list( + ffi::tiledb_attribute_get_cell_val_num( ctx.as_mut_ptr(), *self.raw, - filter_list.as_mut_ptr(), + &mut c_num, ) }; if res == ffi::TILEDB_OK { - Ok(()) + Ok(c_num as u32) } else { Err(ctx.expect_last_error()) } } - pub fn get_filter_list(&self, ctx: &Context) -> TileDBResult { - let mut flist = FilterList::default(); + pub fn cell_size(&self, ctx: &Context) -> TileDBResult { + let mut c_size: std::ffi::c_ulonglong = 0; let res = unsafe { - ffi::tiledb_attribute_get_filter_list( + ffi::tiledb_attribute_get_cell_size( ctx.as_mut_ptr(), *self.raw, - flist.as_mut_ptr_ptr(), + &mut c_size, ) }; if res == ffi::TILEDB_OK { - Ok(flist) + Ok(c_size as u64) } else { Err(ctx.expect_last_error()) } } +} - pub fn set_var_sized(&self, ctx: &Context) -> TileDBResult<()> { - self.set_cell_val_num(ctx, u32::MAX) +impl<'ctx> From<(&'ctx Context, RawAttribute)> for Attribute<'ctx> { + fn from(value: (&'ctx Context, RawAttribute)) -> Self { + Attribute { + context: value.0, + raw: value.1, + } } +} - pub fn is_var_sized(&self, ctx: &Context) -> TileDBResult { - self.get_cell_val_num(ctx).map(|num| num == u32::MAX) +pub struct Builder<'ctx> { + attr: Attribute<'ctx>, +} + +impl<'ctx> Builder<'ctx> { + pub fn new( + context: &'ctx Context, + name: &str, + datatype: Datatype, + ) -> TileDBResult { + let c_context = context.as_mut_ptr(); + let mut c_attr: *mut ffi::tiledb_attribute_t = out_ptr!(); + let c_name = cstring!(name); + let res = unsafe { + ffi::tiledb_attribute_alloc( + c_context, + c_name.as_c_str().as_ptr(), + datatype as u32, + &mut c_attr, + ) + }; + if res == ffi::TILEDB_OK { + Ok(Builder { + attr: Attribute { + context, + raw: RawAttribute::Owned(c_attr), + }, + }) + } else { + Err(context.expect_last_error()) + } } - pub fn set_cell_val_num( - &self, - ctx: &Context, - num: u32, - ) -> TileDBResult<()> { + pub fn cell_val_num(self, num: u32) -> TileDBResult { + let c_context = self.attr.context.as_mut_ptr(); let c_num = num as std::ffi::c_uint; let res = unsafe { ffi::tiledb_attribute_set_cell_val_num( - ctx.as_mut_ptr(), - *self.raw, + c_context, + *self.attr.raw, c_num, ) }; if res == ffi::TILEDB_OK { - Ok(()) + Ok(self) } else { - Err(ctx.expect_last_error()) + Err(self.attr.context.expect_last_error()) } } - pub fn get_cell_val_num(&self, ctx: &Context) -> TileDBResult { - let mut c_num: std::ffi::c_uint = 0; + pub fn var_sized(self) -> TileDBResult { + self.cell_val_num(u32::MAX) + } + + pub fn nullability(self, nullable: bool) -> TileDBResult { + let c_context = self.attr.context.as_mut_ptr(); + let c_nullable: u8 = if nullable { 1 } else { 0 }; let res = unsafe { - ffi::tiledb_attribute_get_cell_val_num( - ctx.as_mut_ptr(), - *self.raw, - &mut c_num, + ffi::tiledb_attribute_set_nullable( + c_context, + *self.attr.raw, + c_nullable, ) }; if res == ffi::TILEDB_OK { - Ok(c_num as u32) + Ok(self) } else { - Err(ctx.expect_last_error()) + Err(self.attr.context.expect_last_error()) } } - pub fn get_cell_size(&self, ctx: &Context) -> TileDBResult { - let mut c_size: std::ffi::c_ulonglong = 0; + pub fn filter_list(self, filter_list: &FilterList) -> TileDBResult { + let c_context = self.attr.context.as_mut_ptr(); let res = unsafe { - ffi::tiledb_attribute_get_cell_size( - ctx.as_mut_ptr(), - *self.raw, - &mut c_size, + ffi::tiledb_attribute_set_filter_list( + c_context, + *self.attr.raw, + // TODO: does the C API copy this? Or alias the pointer? Safety is not obvious + filter_list.as_mut_ptr(), ) }; if res == ffi::TILEDB_OK { - Ok(c_size as u64) + Ok(self) } else { - Err(ctx.expect_last_error()) + Err(self.attr.context.expect_last_error()) } } -} -impl<'ctx> From<(&'ctx Context, RawAttribute)> for Attribute<'ctx> { - fn from(value: (&'ctx Context, RawAttribute)) -> Self { - Attribute { - context: value.0, - raw: value.1, - } + pub fn build(self) -> Attribute<'ctx> { + self.attr } } @@ -254,15 +257,16 @@ mod test { #[test] fn attribute_alloc() { let ctx = Context::new().expect("Error creating context instance."); - Attribute::new(&ctx, "foo", Datatype::UInt64) + Builder::new(&ctx, "foo", Datatype::UInt64) .expect("Error creating attribute instance."); } #[test] fn attribute_get_name_and_type() { let ctx = Context::new().expect("Error creating context instance."); - let attr = Attribute::new(&ctx, "xkcd", Datatype::UInt32) - .expect("Error creating attribute instance."); + let attr = Builder::new(&ctx, "xkcd", Datatype::UInt32) + .expect("Error creating attribute instance.") + .build(); let name = attr.name(&ctx).expect("Error getting attribute name."); assert_eq!(&name, "xkcd"); @@ -276,120 +280,149 @@ mod test { #[test] fn attribute_set_nullable() { let ctx = Context::new().expect("Error creating context instance."); - let attr = Attribute::new(&ctx, "foo", Datatype::UInt64) - .expect("Error creating attribute instance."); - let nullable = attr - .get_nullable(&ctx) - .expect("Error getting attribute nullability."); - assert!(!nullable); + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt64) + .expect("Error creating attribute instance.") + .build(); - attr.set_nullable(&ctx, true) - .expect("Error setting attribute nullability."); - - let nullable = attr - .get_nullable(&ctx) - .expect("Error getting attribute nullability."); - assert!(nullable); + let nullable = attr.is_nullable(); + assert!(!nullable); + } + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt64) + .expect("Error creating attribute instance.") + .nullability(true) + .expect("Error setting attribute nullability.") + .build(); + + let nullable = attr.is_nullable(); + assert!(nullable); + } } #[test] fn attribute_get_set_filter_list() { let ctx = Context::new().expect("Error creating context instance."); - let attr = Attribute::new(&ctx, "foo", Datatype::UInt8) - .expect("Error creating attribute instance."); - let flist1 = attr - .get_filter_list(&ctx) - .expect("Error getting attribute filter list."); - let nfilters = flist1 - .get_num_filters(&ctx) - .expect("Error getting number of filters."); - assert_eq!(nfilters, 0); - - let f1 = Filter::new(&ctx, FilterType::NONE) - .expect("Error creating filter 1."); - let f2 = Filter::new(&ctx, FilterType::BIT_WIDTH_REDUCTION) - .expect("Error creating filter 2."); - let f3 = Filter::new(&ctx, FilterType::ZSTD) - .expect("Error creating filter 3."); - let mut flist2 = - FilterList::new(&ctx).expect("Error creating filter list."); - flist2 - .add_filter(&ctx, &f1) - .expect("Error adding filter 1 to list."); - flist2 - .add_filter(&ctx, &f2) - .expect("Error adding filter 2 to list."); - flist2 - .add_filter(&ctx, &f3) - .expect("Error adding filter 3 to list."); - - attr.set_filter_list(&ctx, &flist2) - .expect("Error setting filter list."); - - let flist3 = attr - .get_filter_list(&ctx) - .expect("Error getting filter list."); - let nfilters = flist3 - .get_num_filters(&ctx) - .expect("Error getting number of filters."); - assert_eq!(nfilters, 3); + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt8) + .expect("Error creating attribute instance.") + .build(); + + let flist1 = attr + .filter_list() + .expect("Error getting attribute filter list."); + let nfilters = flist1 + .get_num_filters(&ctx) + .expect("Error getting number of filters."); + assert_eq!(nfilters, 0); + } + + { + let f1 = Filter::new(&ctx, FilterType::NONE) + .expect("Error creating filter 1."); + let f2 = Filter::new(&ctx, FilterType::BIT_WIDTH_REDUCTION) + .expect("Error creating filter 2."); + let f3 = Filter::new(&ctx, FilterType::ZSTD) + .expect("Error creating filter 3."); + let mut flist2 = + FilterList::new(&ctx).expect("Error creating filter list."); + flist2 + .add_filter(&ctx, &f1) + .expect("Error adding filter 1 to list."); + flist2 + .add_filter(&ctx, &f2) + .expect("Error adding filter 2 to list."); + flist2 + .add_filter(&ctx, &f3) + .expect("Error adding filter 3 to list."); + + let attr = Builder::new(&ctx, "foo", Datatype::UInt8) + .expect("Error creating attribute instance.") + .filter_list(&flist2) + .expect("Error setting filter list.") + .build(); + + let flist3 = + attr.filter_list().expect("Error getting filter list."); + let nfilters = flist3 + .get_num_filters(&ctx) + .expect("Error getting number of filters."); + assert_eq!(nfilters, 3); + } } #[test] fn attribute_cell_val_size() { let ctx = Context::new().expect("Error creating context instance."); - let attr = Attribute::new(&ctx, "foo", Datatype::UInt16) - .expect("Error creating attribute instance."); - - let num = attr - .get_cell_val_num(&ctx) - .expect("Error getting cell val num."); - assert_eq!(num, 1); - let size = attr - .get_cell_size(&ctx) - .expect("Error getting attribute cell size."); - assert_eq!(size, 2); - - attr.set_cell_val_num(&ctx, 3) - .expect("Error setting cell val num."); - let num = attr - .get_cell_val_num(&ctx) - .expect("Error getting cell val num."); - assert_eq!(num, 3); - let size = attr - .get_cell_size(&ctx) - .expect("Error getting attribute cell size."); - assert_eq!(size, 6); - - attr.set_cell_val_num(&ctx, u32::MAX) - .expect("Error setting cell val size."); - let is_var = attr - .is_var_sized(&ctx) - .expect("Error getting attribute var sized-ness."); - assert!(is_var); - - attr.set_cell_val_num(&ctx, 42) - .expect("Error setting cell val num."); - let num = attr - .get_cell_val_num(&ctx) - .expect("Error getting cell val num."); - assert_eq!(num, 42); - let size = attr - .get_cell_size(&ctx) - .expect("Error getting cell val size."); - assert_eq!(size, 84); - - attr.set_var_sized(&ctx) - .expect("Error setting attribute to var sized."); - let num = attr - .get_cell_val_num(&ctx) - .expect("Error getting cell val num."); - assert_eq!(num, u32::MAX); - let size = attr - .get_cell_size(&ctx) - .expect("Error getting cell val size."); - assert_eq!(size, u64::MAX); + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt16) + .expect("Error creating attribute instance.") + .build(); + + let num = attr + .cell_val_num(&ctx) + .expect("Error getting cell val num."); + assert_eq!(num, 1); + let size = attr + .cell_size(&ctx) + .expect("Error getting attribute cell size."); + assert_eq!(size, 2); + } + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt16) + .expect("Error creating attribute instance.") + .cell_val_num(3) + .expect("Error setting cell val num.") + .build(); + let num = attr + .cell_val_num(&ctx) + .expect("Error getting cell val num."); + assert_eq!(num, 3); + let size = attr + .cell_size(&ctx) + .expect("Error getting attribute cell size."); + assert_eq!(size, 6); + } + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt16) + .expect("Error creating attribute instance.") + .cell_val_num(u32::MAX) + .expect("Error setting cell val size.") + .build(); + let is_var = attr + .is_var_sized(&ctx) + .expect("Error getting attribute var sized-ness."); + assert!(is_var); + } + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt16) + .expect("Error creating attribute instance.") + .cell_val_num(42) + .expect("Error setting cell val num.") + .build(); + let num = attr + .cell_val_num(&ctx) + .expect("Error getting cell val num."); + assert_eq!(num, 42); + let size = + attr.cell_size(&ctx).expect("Error getting cell val size."); + assert_eq!(size, 84); + } + { + let attr = Builder::new(&ctx, "foo", Datatype::UInt16) + .expect("Error creating attribute instance.") + .var_sized() + .expect("Error setting var sized.") + .build(); + let num = attr + .cell_val_num(&ctx) + .expect("Error getting cell val num."); + assert_eq!(num, u32::MAX); + let size = + attr.cell_size(&ctx).expect("Error getting cell val size."); + assert_eq!(size, u64::MAX); + } } } diff --git a/src/array/mod.rs b/src/array/mod.rs index 8667c34e..6334b979 100644 --- a/src/array/mod.rs +++ b/src/array/mod.rs @@ -8,7 +8,7 @@ mod dimension; mod domain; mod schema; -pub use attribute::Attribute; +pub use attribute::{Attribute, Builder as AttributeBuilder}; pub use dimension::{Builder as DimensionBuilder, Dimension}; pub use domain::{Builder as DomainBuilder, Domain}; pub use schema::{ArrayType, Builder as SchemaBuilder, Schema}; @@ -185,7 +185,11 @@ mod tests { let s: Schema = SchemaBuilder::new(&c, ArrayType::Sparse, d) .unwrap() - .add_attribute(Attribute::new(&c, "a", Datatype::UInt64).unwrap()) + .add_attribute( + AttributeBuilder::new(&c, "a", Datatype::UInt64) + .unwrap() + .build(), + ) .unwrap() .into();