diff --git a/src/comparator.rs b/src/comparator.rs index 97d5243..9cfc895 100644 --- a/src/comparator.rs +++ b/src/comparator.rs @@ -18,11 +18,11 @@ use std::cmp::Ordering; use std::ffi::CString; use std::slice; -pub type CompareFn = fn(&[u8], &[u8]) -> Ordering; +pub type CompareFn = dyn Fn(&[u8], &[u8]) -> Ordering; pub struct ComparatorCallback { pub name: CString, - pub f: CompareFn, + pub f: Box, } pub unsafe extern "C" fn destructor_callback(raw_cb: *mut c_void) { diff --git a/src/db_options.rs b/src/db_options.rs index c9fb5c5..cb8e978 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -1329,7 +1329,7 @@ impl Options { /// The client must ensure that the comparator supplied here has the same /// name and orders keys *exactly* the same as the comparator provided to /// previous open calls on the same DB. - pub fn set_comparator(&mut self, name: impl CStrLike, compare_fn: CompareFn) { + pub fn set_comparator(&mut self, name: impl CStrLike, compare_fn: Box) { let cb = Box::new(ComparatorCallback { name: name.into_c_string().unwrap(), f: compare_fn, @@ -1352,14 +1352,6 @@ impl Options { } } - #[deprecated( - since = "0.5.0", - note = "add_comparator has been renamed to set_comparator" - )] - pub fn add_comparator(&mut self, name: &str, compare_fn: CompareFn) { - self.set_comparator(name, compare_fn); - } - pub fn optimize_for_point_lookup(&mut self, cache_size: u64) { unsafe { ffi::rocksdb_options_optimize_for_point_lookup(self.inner, cache_size); diff --git a/tests/test_comparator.rs b/tests/test_comparator.rs new file mode 100644 index 0000000..81910a2 --- /dev/null +++ b/tests/test_comparator.rs @@ -0,0 +1,74 @@ +use rocksdb::{Options, DB}; +use std::cmp::Ordering; +use std::iter::FromIterator; + +/// This function is for ensuring test of backwards compatibility +pub fn rocks_old_compare(one: &[u8], two: &[u8]) -> Ordering { + one.cmp(two) +} + +/// create database add some values, and iterate over these +pub fn write_to_db_with_comparator( + compare_fn: Box Ordering>, +) -> Vec { + let mut result_vec = Vec::new(); + + let path = "_path_for_rocksdb_storage"; + { + let mut db_opts = Options::default(); + + db_opts.create_missing_column_families(true); + db_opts.create_if_missing(true); + db_opts.set_comparator("cname", compare_fn); + let db = DB::open(&db_opts, path).unwrap(); + db.put(b"a-key", b"a-value").unwrap(); + db.put(b"b-key", b"b-value").unwrap(); + let mut iter = db.raw_iterator(); + iter.seek_to_first(); + while iter.valid() { + let key = iter.key().unwrap(); + // maybe not best way to copy? + let key_str = key.iter().map(|b| *b as char).collect::>(); + result_vec.push(String::from_iter(key_str)); + iter.next(); + } + } + let _ = DB::destroy(&Options::default(), path); + result_vec +} + +#[test] +/// First verify that using a function as a comparator works as expected +/// This should verify backwards compatablity +/// Then run a test with a clojure where an x-variable is passed +/// Keep in mind that this variable must be moved to the clojure +/// Then run a test with a reverse sorting clojure and make sure the order is reverted +fn test_comparator() { + let local_compare = move |one: &[u8], two: &[u8]| one.cmp(two); + + let x = 0; + let local_compare_reverse = move |one: &[u8], two: &[u8]| { + println!( + "Use the x value from the closure scope to do something smart: {:?}", + x + ); + match one.cmp(two) { + Ordering::Less => Ordering::Greater, + Ordering::Equal => Ordering::Equal, + Ordering::Greater => Ordering::Less, + } + }; + + let old_res = write_to_db_with_comparator(Box::new(rocks_old_compare)); + println!("Keys in normal sort order, no closure: {:?}", old_res); + assert_eq!(vec!["a-key", "b-key"], old_res); + let res_closure = write_to_db_with_comparator(Box::new(local_compare)); + println!("Keys in normal sort order, closure: {:?}", res_closure); + assert_eq!(res_closure, old_res); + let res_closure_reverse = write_to_db_with_comparator(Box::new(local_compare_reverse)); + println!( + "Keys in reverse sort order, closure: {:?}", + res_closure_reverse + ); + assert_eq!(vec!["b-key", "a-key"], res_closure_reverse); +}