Alterntaive to DomainType

This implements a different approach to converting between types and
Datatype dependent type checks.

The main idea here that's different than previously is that the
DomainType enforced a 1:1 correspondence between TileDB's Datatype
enumeration and the Rust type system. Unfortunately, multiple TileDB
Datatypes map to the same Rust type. The idea here is to separate the
conversion of types and the validation that a given Rust type is valid
for a given TileDB Datatype.

The basic idea is to move type conversion between Rust and FFI types
(i.e., i32 -> uint32_t) to be a separate concern from "Is an i32 valid
as a Datatype::Blob". The type conversions basically map directly to how
DomainType worked. The validation is moved to a generic method on
Datatype in tiledb-sys that makes heavy use of `std::any::TypeId` to
restrict the generic type paramter to specific Datatype enum variants.

examples/quickstart_dense.rs

@@ -1,5 +1,6 @@
 extern crate tiledb;
 
+use tiledb::Datatype;
 use tiledb::Result as TileDBResult;
 
 const QUICKSTART_DENSE_ARRAY_URI: &str = "quickstart_dense_array";
@@ -25,6 +26,7 @@ fn create_array() -> TileDBResult<()> {
             tiledb::array::DimensionBuilder::new::<i32>(
                 &tdb,
                 "rows",
+                Datatype::Int32,
                 &[1, 4],
                 &4,
             )?
@@ -33,6 +35,7 @@ fn create_array() -> TileDBResult<()> {
             tiledb::array::DimensionBuilder::new::<i32>(
                 &tdb,
                 "columns",
+                Datatype::Int32,
                 &[1, 4],
                 &4,
             )?

src/array/attribute.rs

@@ -5,7 +5,7 @@ use std::ops::Deref;
 pub use tiledb_sys::Datatype;
 
 use crate::context::Context;
-use crate::datatype::DomainType;
+use crate::convert::CAPIConverter;
 use crate::error::Error;
 use crate::filter_list::FilterList;
 use crate::Result as TileDBResult;
@@ -237,27 +237,27 @@ impl Attribute {
     }
 
     // This currently does not support setting multi-value cells.
-    pub fn set_fill_value<DT: DomainType>(
+    pub fn set_fill_value<Conv: CAPIConverter + 'static>(
         &self,
         ctx: &Context,
-        value: DT,
+        value: Conv,
     ) -> TileDBResult<()> {
-        let c_val: DT::CApiType = value.as_capi();
+        let c_val: Conv::CAPIType = value.to_capi();
 
-        if DT::DATATYPE != self.datatype(ctx)? {
+        if !self.datatype(ctx)?.is_valid(&value) {
             return Err(Error::from(format!(
                 "Dimension type mismatch: expected {}, found {}",
                 self.datatype(ctx)?,
-                DT::DATATYPE
+                std::any::type_name::<Conv>()
             )));
         }
 
         let res = unsafe {
             ffi::tiledb_attribute_set_fill_value(
                 ctx.as_mut_ptr(),
                 *self.raw,
-                &c_val as *const DT::CApiType as *const std::ffi::c_void,
-                std::mem::size_of::<DT::CApiType>() as u64,
+                &c_val as *const Conv::CAPIType as *const std::ffi::c_void,
+                std::mem::size_of::<Conv::CAPIType>() as u64,
             )
         };
 
@@ -268,10 +268,10 @@ impl Attribute {
         }
     }
 
-    pub fn get_fill_value<DT: DomainType>(
+    pub fn get_fill_value<Conv: CAPIConverter>(
         &self,
         ctx: &Context,
-    ) -> TileDBResult<DT> {
+    ) -> TileDBResult<Conv> {
         let mut c_ptr: *const std::ffi::c_void = out_ptr!();
         let mut c_size: u64 = 0;
 
@@ -288,13 +288,13 @@ impl Attribute {
             return Err(ctx.expect_last_error());
         }
 
-        if c_size != std::mem::size_of::<DT::CApiType>() as u64 {
+        if c_size != std::mem::size_of::<Conv::CAPIType>() as u64 {
             return Err(Error::from("Invalid value size returned by TileDB"));
         }
 
-        let c_val: DT::CApiType = unsafe { *c_ptr.cast::<DT::CApiType>() };
+        let c_val: Conv::CAPIType = unsafe { *c_ptr.cast::<Conv::CAPIType>() };
 
-        Ok(DT::from_capi(&c_val))
+        Ok(Conv::to_rust(&c_val))
     }
 }
 

src/array/dimension.rs

@@ -1,8 +1,9 @@
 use std::ops::Deref;
 
+use tiledb_sys::Datatype;
+
 use crate::context::Context;
-use crate::datatype::{Datatype, DomainType};
-use crate::error::Error;
+use crate::convert::CAPIConverter;
 use crate::filter_list::FilterList;
 use crate::Result as TileDBResult;
 
@@ -56,19 +57,11 @@ impl<'ctx> Dimension<'ctx> {
         Datatype::from_capi_enum(c_datatype)
     }
 
-    pub fn domain<DT: DomainType>(&self) -> TileDBResult<[DT; 2]> {
+    pub fn domain<Conv: CAPIConverter>(&self) -> TileDBResult<[Conv; 2]> {
         let c_context = self.context.as_mut_ptr();
         let c_dimension = self.capi();
         let mut c_domain_ptr: *const std::ffi::c_void = out_ptr!();
 
-        if DT::DATATYPE != self.datatype() {
-            return Err(Error::from(format!(
-                "Dimension type mismatch: expected {}, found {}",
-                self.datatype(),
-                DT::DATATYPE
-            )));
-        }
-
         let c_ret = unsafe {
             ffi::tiledb_dimension_get_domain(
                 c_context,
@@ -80,10 +73,10 @@ impl<'ctx> Dimension<'ctx> {
         // the only errors are possible via mis-use of the C API, which Rust prevents
         assert_eq!(ffi::TILEDB_OK, c_ret);
 
-        let c_domain: &[DT::CApiType; 2] =
-            unsafe { &*c_domain_ptr.cast::<[DT::CApiType; 2]>() };
+        let c_domain: &[Conv::CAPIType; 2] =
+            unsafe { &*c_domain_ptr.cast::<[Conv::CAPIType; 2]>() };
 
-        Ok([DT::from_capi(&c_domain[0]), DT::from_capi(&c_domain[1])])
+        Ok([Conv::to_rust(&c_domain[0]), Conv::to_rust(&c_domain[1])])
     }
 
     pub fn filters(&self) -> FilterList {
@@ -113,20 +106,21 @@ pub struct Builder<'ctx> {
 impl<'ctx> Builder<'ctx> {
     // TODO: extent might be optional?
     // and it
-    pub fn new<DT: DomainType>(
+    pub fn new<Conv: CAPIConverter>(
         context: &'ctx Context,
         name: &str,
-        domain: &[DT; 2],
-        extent: &DT,
+        datatype: Datatype,
+        domain: &[Conv; 2],
+        extent: &Conv,
     ) -> TileDBResult<Self> {
         let c_context = context.as_mut_ptr();
-        let c_datatype = DT::DATATYPE.capi_enum();
+        let c_datatype = datatype.capi_enum();
 
         let c_name = cstring!(name);
 
-        let c_domain: [DT::CApiType; 2] =
-            [domain[0].as_capi(), domain[1].as_capi()];
-        let c_extent: DT::CApiType = extent.as_capi();
+        let c_domain: [Conv::CAPIType; 2] =
+            [domain[0].to_capi(), domain[1].to_capi()];
+        let c_extent: Conv::CAPIType = extent.to_capi();
 
         let mut c_dimension: *mut ffi::tiledb_dimension_t =
             std::ptr::null_mut();
@@ -136,10 +130,9 @@ impl<'ctx> Builder<'ctx> {
                 c_context,
                 c_name.as_ptr(),
                 c_datatype,
-                &c_domain[0] as *const <DT as DomainType>::CApiType
-                    as *const std::ffi::c_void,
-                &c_extent as *const <DT as DomainType>::CApiType
+                &c_domain[0] as *const <Conv>::CAPIType
                     as *const std::ffi::c_void,
+                &c_extent as *const <Conv>::CAPIType as *const std::ffi::c_void,
                 &mut c_dimension,
             )
         } == ffi::TILEDB_OK
@@ -197,6 +190,7 @@ mod tests {
             Builder::new::<i32>(
                 &context,
                 "test_dimension_alloc",
+                Datatype::Int32,
                 &domain,
                 &extent,
             )
@@ -210,6 +204,7 @@ mod tests {
             let b = Builder::new::<i32>(
                 &context,
                 "test_dimension_alloc",
+                Datatype::Int32,
                 &domain,
                 &extent,
             );
@@ -223,6 +218,7 @@ mod tests {
             let b = Builder::new::<i32>(
                 &context,
                 "test_dimension_alloc",
+                Datatype::Int32,
                 &domain,
                 &extent,
             );
@@ -241,6 +237,7 @@ mod tests {
             let dim = Builder::new::<i32>(
                 &context,
                 "test_dimension_domain",
+                Datatype::Int32,
                 &domain_in,
                 &extent,
             )
@@ -266,6 +263,7 @@ mod tests {
             let dimension: Dimension = Builder::new::<i32>(
                 &context,
                 "test_dimension_alloc",
+                Datatype::Int32,
                 &domain,
                 &extent,
             )
@@ -286,6 +284,7 @@ mod tests {
             let dimension: Dimension = Builder::new::<i32>(
                 &context,
                 "test_dimension_alloc",
+                Datatype::Int32,
                 &domain,
                 &extent,
             )

src/array/domain.rs

@@ -150,6 +150,7 @@ mod tests {
                 DimensionBuilder::new::<i32>(
                     &context,
                     "d1",
+                    Datatype::Int32,
                     &dim_domain,
                     &extent,
                 )
@@ -179,6 +180,7 @@ mod tests {
                 DimensionBuilder::new::<i32>(
                     &context,
                     "d1",
+                    Datatype::Int32,
                     &dim1_domain,
                     &extent,
                 )
@@ -191,6 +193,7 @@ mod tests {
                 DimensionBuilder::new::<f64>(
                     &context,
                     "d2",
+                    Datatype::Float64,
                     &dim2_domain,
                     &extent,
                 )

src/array/mod.rs

@@ -165,14 +165,24 @@ mod tests {
 
         // "quickstart_dense" example
         let d: Domain = {
-            let rows: Dimension =
-                DimensionBuilder::new::<i32>(&c, "rows", &[1, 4], &4)
-                    .expect("Error constructing rows dimension")
-                    .build();
-            let cols: Dimension =
-                DimensionBuilder::new::<i32>(&c, "cols", &[1, 4], &4)
-                    .expect("Error constructing cols dimension")
-                    .build();
+            let rows: Dimension = DimensionBuilder::new::<i32>(
+                &c,
+                "rows",
+                Datatype::Int32,
+                &[1, 4],
+                &4,
+            )
+            .expect("Error constructing rows dimension")
+            .build();
+            let cols: Dimension = DimensionBuilder::new::<i32>(
+                &c,
+                "cols",
+                Datatype::Int32,
+                &[1, 4],
+                &4,
+            )
+            .expect("Error constructing cols dimension")
+            .build();
 
             DomainBuilder::new(&c)
                 .unwrap()

src/array/schema.rs

@@ -177,12 +177,19 @@ mod tests {
     use crate::array::schema::*;
     use crate::array::{DimensionBuilder, DomainBuilder};
     use crate::context::Context;
+    use tiledb_sys::Datatype;
 
     /// Helper function to make a Domain which isn't needed for the purposes of the test
     fn unused_domain(c: &Context) -> Domain {
-        let dim = DimensionBuilder::new::<i32>(c, "test", &[-100, 100], &100)
-            .unwrap()
-            .build();
+        let dim = DimensionBuilder::new::<i32>(
+            c,
+            "test",
+            Datatype::Int32,
+            &[-100, 100],
+            &100,
+        )
+        .unwrap()
+        .build();
         DomainBuilder::new(c)
             .unwrap()
             .add_dimension(dim)

src/convert.rs

@@ -0,0 +1,42 @@
+pub trait CAPIConverter {
+    type CAPIType: Default + Copy;
+
+    fn to_capi(&self) -> Self::CAPIType;
+    fn to_rust(value: &Self::CAPIType) -> Self;
+}
+
+impl CAPIConverter for i32 {
+    type CAPIType = std::ffi::c_int;
+
+    fn to_capi(&self) -> Self::CAPIType {
+        *self as Self::CAPIType
+    }
+
+    fn to_rust(value: &Self::CAPIType) -> Self {
+        *value as Self
+    }
+}
+
+impl CAPIConverter for u32 {
+    type CAPIType = std::ffi::c_uint;
+
+    fn to_capi(&self) -> Self::CAPIType {
+        *self as Self::CAPIType
+    }
+
+    fn to_rust(value: &Self::CAPIType) -> Self {
+        *value as Self
+    }
+}
+
+impl CAPIConverter for f64 {
+    type CAPIType = std::ffi::c_double;
+
+    fn to_capi(&self) -> Self::CAPIType {
+        *self as Self::CAPIType
+    }
+
+    fn to_rust(value: &Self::CAPIType) -> Self {
+        *value as Self
+    }
+}