From 6b449a3ae12108d6164362fac2229fd3620a27e9 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 21:15:12 +0800 Subject: [PATCH 01/16] perf(xor): store optional item counts in one word --- datasketches/src/xor/filter.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 36fc374d..6e787718 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -35,6 +35,8 @@ const HASH_SEED: u64 = 0; const LOAD_FACTOR: f64 = 1.23; const CAPACITY_OFFSET: u64 = 32; const MAX_CONSTRUCTION_ATTEMPTS: usize = 100; +// Counts cannot exceed the allocated fingerprint capacity, so this value is unreachable. +const UNKNOWN_NUM_ITEMS: usize = usize::MAX; const MURMUR_C1: u64 = 0xff51_afd7_ed55_8ccd; const MURMUR_C2: u64 = 0xc4ce_b9fe_1a85_ec53; @@ -114,7 +116,7 @@ impl XorFingerprints { #[derive(Debug, Clone, PartialEq, Eq)] pub struct XorFilter { segment_length: usize, - num_items: Option, + num_items: usize, seed: u64, fingerprints: XorFingerprints, } @@ -249,7 +251,7 @@ impl XorFilter { Ok(Self { segment_length, - num_items: Some(num_items), + num_items, seed: construction_seed, fingerprints, }) @@ -307,7 +309,7 @@ impl XorFilter { } Ok(Self { segment_length, - num_items, + num_items: num_items.unwrap_or(UNKNOWN_NUM_ITEMS), seed, fingerprints, }) @@ -355,19 +357,19 @@ impl XorFilter { }; // Avoid branching on the fingerprint comparison, including rare false // positives. Unknown counts must preserve the raw comparison. - found & (self.num_items != Some(0)) + found & (self.num_items != 0) } /// Returns whether the input set was empty, or `None` if imported without an item count. /// /// Fingerprint contents cannot reliably establish whether the original set was empty. pub fn is_empty(&self) -> Option { - self.num_items.map(|count| count == 0) + self.num_items().map(|count| count == 0) } /// Returns the number of distinct input hashes, or `None` if imported without a count. pub fn num_items(&self) -> Option { - self.num_items + (self.num_items != UNKNOWN_NUM_ITEMS).then_some(self.num_items) } /// Returns the fingerprint representation. @@ -405,7 +407,7 @@ impl XorFilter { /// sufficiently large inputs the value approaches `9.84` for [`XorFilterType::Xor8`], /// `19.68` for [`XorFilterType::Xor16`], and `39.36` for [`XorFilterType::Xor32`]. pub fn bits_per_item(&self) -> Option { - self.num_items.map(|count| { + self.num_items().map(|count| { if count == 0 { 0.0 } else { @@ -437,9 +439,6 @@ impl XorFilter { /// standard serialization. Use raw parts for other storage formats. pub fn serialize(&self) -> Result, Error> { let size = self.serialized_size()?; - let num_items = self - .num_items - .expect("serialized_size checked the item count"); let mut bytes = SketchBytes::with_capacity(size); bytes.write_u8(Family::XORFILTER.min_pre_longs); @@ -451,7 +450,7 @@ impl XorFilter { bytes.write_u16_le(0); // unused bytes.write_u64_le(self.seed); bytes.write_i32_le(self.segment_length as i32); - bytes.write_i32_le(num_items as i32); + bytes.write_i32_le(self.num_items as i32); match &self.fingerprints { XorFingerprints::Xor8(fingerprints) => bytes.write(fingerprints), @@ -601,7 +600,7 @@ impl XorFilter { Ok(Self { segment_length, - num_items: Some(num_items), + num_items, seed, fingerprints, }) @@ -609,7 +608,7 @@ impl XorFilter { fn serialized_size(&self) -> Result { let count = self - .num_items + .num_items() .ok_or_else(|| Error::invalid_argument("standard xor format requires an item count"))?; if count > i32::MAX as usize || self.segment_length > i32::MAX as usize From 72069da03d64bc4804d22c1273f3f2a4aebc7192 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 21:45:09 +0800 Subject: [PATCH 02/16] refactor(xor): trim redundant metadata accessors --- benchmarks/xor/query_shards.rs | 13 +++--- datasketches/src/xor/filter.rs | 46 ++++------------------ tests-integration/tests/serde_tests/xor.rs | 1 - tests-integration/tests/xor_test/filter.rs | 17 -------- tests-integration/tests/xor_test/parts.rs | 3 +- 5 files changed, 14 insertions(+), 66 deletions(-) diff --git a/benchmarks/xor/query_shards.rs b/benchmarks/xor/query_shards.rs index 4a52ede3..e85579f7 100644 --- a/benchmarks/xor/query_shards.rs +++ b/benchmarks/xor/query_shards.rs @@ -112,18 +112,15 @@ fn query( } pub fn register(runner: &mut Runner) { - for kind in [ - XorFilterType::Xor8, - XorFilterType::Xor16, - XorFilterType::Xor32, + for (bits, kind) in [ + (8, XorFilterType::Xor8), + (16, XorFilterType::Xor16), + (32, XorFilterType::Xor32), ] { for cardinality in [128, 16_384] { for (name, present) in [("present", true), ("absent", false)] { runner.compare( - &format!( - "xor::query_shards::{}_{name}/{cardinality}", - kind.bits_per_fingerprint() - ), + &format!("xor::query_shards::{bits}_{name}/{cardinality}"), 20_000, &[ ("DataSketches", &|b| { diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 6e787718..dbd1bbb5 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -35,8 +35,9 @@ const HASH_SEED: u64 = 0; const LOAD_FACTOR: f64 = 1.23; const CAPACITY_OFFSET: u64 = 32; const MAX_CONSTRUCTION_ATTEMPTS: usize = 100; -// Counts cannot exceed the allocated fingerprint capacity, so this value is unreachable. -const UNKNOWN_NUM_ITEMS: usize = usize::MAX; +// Sentinel for an absent item count. Valid counts cannot exceed the fingerprint count; +// u8/u16/u32 slices occupy at most isize::MAX bytes, so their lengths are below usize::MAX. +const NUM_ITEMS_SENTINEL: usize = usize::MAX; const MURMUR_C1: u64 = 0xff51_afd7_ed55_8ccd; const MURMUR_C2: u64 = 0xc4ce_b9fe_1a85_ec53; @@ -58,8 +59,7 @@ pub enum XorFilterType { } impl XorFilterType { - /// Returns the number of bits in each fingerprint. - pub const fn bits_per_fingerprint(self) -> u8 { + const fn bits_per_fingerprint(self) -> u8 { match self { Self::Xor8 => 8, Self::Xor16 => 16, @@ -309,7 +309,7 @@ impl XorFilter { } Ok(Self { segment_length, - num_items: num_items.unwrap_or(UNKNOWN_NUM_ITEMS), + num_items: num_items.unwrap_or(NUM_ITEMS_SENTINEL), seed, fingerprints, }) @@ -369,7 +369,7 @@ impl XorFilter { /// Returns the number of distinct input hashes, or `None` if imported without a count. pub fn num_items(&self) -> Option { - (self.num_items != UNKNOWN_NUM_ITEMS).then_some(self.num_items) + (self.num_items != NUM_ITEMS_SENTINEL).then_some(self.num_items) } /// Returns the fingerprint representation. @@ -381,41 +381,11 @@ impl XorFilter { } } - /// Returns the number of bits in each fingerprint. - pub fn bits_per_fingerprint(&self) -> u8 { - self.filter_type().bits_per_fingerprint() - } - - /// Returns the number of hash locations used by non-empty filter queries. - pub fn num_hashes(&self) -> u8 { - NUM_HASHES - } - - /// Returns the number of fingerprint slots in the filter. - pub fn capacity(&self) -> usize { - self.fingerprints.len() - } - /// Returns the construction seed stored with the filter. pub fn seed(&self) -> u64 { self.seed } - /// Returns the number of fingerprint bits allocated per distinct input hash. - /// - /// Returns `None` for an unknown item count and `Some(0.0)` for an empty filter. For - /// sufficiently large inputs the value approaches `9.84` for [`XorFilterType::Xor8`], - /// `19.68` for [`XorFilterType::Xor16`], and `39.36` for [`XorFilterType::Xor32`]. - pub fn bits_per_item(&self) -> Option { - self.num_items().map(|count| { - if count == 0 { - 0.0 - } else { - self.capacity() as f64 * f64::from(self.bits_per_fingerprint()) / count as f64 - } - }) - } - /// Returns the estimated in-memory size of the filter in bytes. pub fn estimated_size(&self) -> usize { size_of::() + self.fingerprints.byte_len() @@ -445,7 +415,7 @@ impl XorFilter { bytes.write_u8(SERIAL_VERSION); bytes.write_u8(Family::XORFILTER.id); bytes.write_u8(0); // flags - bytes.write_u8(self.bits_per_fingerprint()); + bytes.write_u8(self.filter_type().bits_per_fingerprint()); bytes.write_u8(NUM_HASHES); bytes.write_u16_le(0); // unused bytes.write_u64_le(self.seed); @@ -612,7 +582,7 @@ impl XorFilter { .ok_or_else(|| Error::invalid_argument("standard xor format requires an item count"))?; if count > i32::MAX as usize || self.segment_length > i32::MAX as usize - || self.capacity() > i32::MAX as usize + || self.fingerprints.len() > i32::MAX as usize { return Err(Error::invalid_argument( "xor filter exceeds standard format limits", diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index 18de001c..f21e4888 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -307,7 +307,6 @@ fn raw_parts_preserve_an_application_storage_layout() { for hash in [0, 1, 2, u64::MAX] { assert!(filter.contains_hash(hash)); } - assert_eq!(filter.capacity(), slots); assert_eq!(filter.num_items(), None); let (seed, segment) = (filter.seed(), filter.segment_length()); diff --git a/tests-integration/tests/xor_test/filter.rs b/tests-integration/tests/xor_test/filter.rs index cc2a714d..15e7e93f 100644 --- a/tests-integration/tests/xor_test/filter.rs +++ b/tests-integration/tests/xor_test/filter.rs @@ -64,29 +64,12 @@ fn duplicates_are_removed_before_construction() { assert!(filter.contains(&11_u64)); } -#[test] -fn metadata_describes_the_compact_payload() { - let hashes: Vec<_> = (0..10_000_u64).collect(); - let filter = XorFilter::from_hashes(&hashes, XorFilterType::Xor8, 0).unwrap(); - - assert_eq!(filter.is_empty(), Some(false)); - assert_eq!(filter.num_items(), Some(10_000)); - assert_eq!(filter.filter_type(), XorFilterType::Xor8); - assert_eq!(filter.bits_per_fingerprint(), 8); - assert_eq!(filter.num_hashes(), 3); - assert_eq!(filter.capacity() % 3, 0); - assert!((9.5..10.5).contains(&filter.bits_per_item().unwrap())); - assert!(filter.estimated_size() >= filter.capacity()); -} - #[test] fn empty_filter_is_well_formed() { let filter = XorFilter::from_hashes(&[], XorFilterType::Xor16, 0).unwrap(); assert_eq!(filter.is_empty(), Some(true)); assert_eq!(filter.num_items(), Some(0)); - assert_eq!(filter.capacity(), 30); - assert_eq!(filter.bits_per_item().unwrap(), 0.0); } #[test] diff --git a/tests-integration/tests/xor_test/parts.rs b/tests-integration/tests/xor_test/parts.rs index 9b5272ae..33885389 100644 --- a/tests-integration/tests/xor_test/parts.rs +++ b/tests-integration/tests/xor_test/parts.rs @@ -59,7 +59,6 @@ fn raw_parts_interoperate_with_xorf_in_both_directions() { }; let imported = XorFilter::from_parts(seed, segment, payload, None).unwrap(); assert_eq!(imported.num_items(), None); - assert_eq!(imported.bits_per_item(), None); assert_eq!(imported.is_empty(), None); assert_eq!( imported.serialize().unwrap_err().kind(), @@ -105,6 +104,7 @@ fn raw_parts_reject_invalid_layouts_and_counts() { (1, 4, None), (usize::MAX, 3, None), (1, 3, Some(4)), + (1, 3, Some(usize::MAX)), ] { assert_eq!( XorFilter::from_parts( @@ -193,7 +193,6 @@ fn unknown_count_does_not_infer_emptiness_from_zero_fingerprints() { .unwrap(); assert_eq!(unknown.is_empty(), None); assert_eq!(unknown.num_items(), None); - assert_eq!(unknown.bits_per_item(), None); // An omitted count must preserve the raw fingerprint comparison, even for all-zero slots. assert!(unknown.contains_hash(zero_hash)); assert_eq!( From 62278aad135dba2d288929af8e5574cb6d5552ae Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 21:48:18 +0800 Subject: [PATCH 03/16] refactor(xor): retain optional counts and inline serialization checks --- datasketches/src/xor/filter.rs | 51 ++++++++++------------- tests-integration/tests/xor_test/parts.rs | 1 - 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index dbd1bbb5..026c419b 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -35,9 +35,6 @@ const HASH_SEED: u64 = 0; const LOAD_FACTOR: f64 = 1.23; const CAPACITY_OFFSET: u64 = 32; const MAX_CONSTRUCTION_ATTEMPTS: usize = 100; -// Sentinel for an absent item count. Valid counts cannot exceed the fingerprint count; -// u8/u16/u32 slices occupy at most isize::MAX bytes, so their lengths are below usize::MAX. -const NUM_ITEMS_SENTINEL: usize = usize::MAX; const MURMUR_C1: u64 = 0xff51_afd7_ed55_8ccd; const MURMUR_C2: u64 = 0xc4ce_b9fe_1a85_ec53; @@ -116,7 +113,7 @@ impl XorFingerprints { #[derive(Debug, Clone, PartialEq, Eq)] pub struct XorFilter { segment_length: usize, - num_items: usize, + num_items: Option, seed: u64, fingerprints: XorFingerprints, } @@ -251,7 +248,7 @@ impl XorFilter { Ok(Self { segment_length, - num_items, + num_items: Some(num_items), seed: construction_seed, fingerprints, }) @@ -309,7 +306,7 @@ impl XorFilter { } Ok(Self { segment_length, - num_items: num_items.unwrap_or(NUM_ITEMS_SENTINEL), + num_items, seed, fingerprints, }) @@ -357,19 +354,19 @@ impl XorFilter { }; // Avoid branching on the fingerprint comparison, including rare false // positives. Unknown counts must preserve the raw comparison. - found & (self.num_items != 0) + found & (self.num_items != Some(0)) } /// Returns whether the input set was empty, or `None` if imported without an item count. /// /// Fingerprint contents cannot reliably establish whether the original set was empty. pub fn is_empty(&self) -> Option { - self.num_items().map(|count| count == 0) + self.num_items.map(|count| count == 0) } /// Returns the number of distinct input hashes, or `None` if imported without a count. pub fn num_items(&self) -> Option { - (self.num_items != NUM_ITEMS_SENTINEL).then_some(self.num_items) + self.num_items } /// Returns the fingerprint representation. @@ -408,7 +405,20 @@ impl XorFilter { /// and raw parts allow larger capacities, so successful construction does not guarantee /// standard serialization. Use raw parts for other storage formats. pub fn serialize(&self) -> Result, Error> { - let size = self.serialized_size()?; + let count = self + .num_items + .ok_or_else(|| Error::invalid_argument("standard xor format requires an item count"))?; + if count > i32::MAX as usize + || self.segment_length > i32::MAX as usize + || self.fingerprints.len() > i32::MAX as usize + { + return Err(Error::invalid_argument( + "xor filter exceeds standard format limits", + )); + } + let size = PREAMBLE_BYTES + .checked_add(self.fingerprints.byte_len()) + .ok_or_else(|| Error::invalid_argument("xor filter serialized size overflow"))?; let mut bytes = SketchBytes::with_capacity(size); bytes.write_u8(Family::XORFILTER.min_pre_longs); @@ -420,7 +430,7 @@ impl XorFilter { bytes.write_u16_le(0); // unused bytes.write_u64_le(self.seed); bytes.write_i32_le(self.segment_length as i32); - bytes.write_i32_le(self.num_items as i32); + bytes.write_i32_le(count as i32); match &self.fingerprints { XorFingerprints::Xor8(fingerprints) => bytes.write(fingerprints), @@ -570,28 +580,11 @@ impl XorFilter { Ok(Self { segment_length, - num_items, + num_items: Some(num_items), seed, fingerprints, }) } - - fn serialized_size(&self) -> Result { - let count = self - .num_items() - .ok_or_else(|| Error::invalid_argument("standard xor format requires an item count"))?; - if count > i32::MAX as usize - || self.segment_length > i32::MAX as usize - || self.fingerprints.len() > i32::MAX as usize - { - return Err(Error::invalid_argument( - "xor filter exceeds standard format limits", - )); - } - PREAMBLE_BYTES - .checked_add(self.fingerprints.byte_len()) - .ok_or_else(|| Error::invalid_argument("xor filter serialized size overflow")) - } } fn hash_item(item: &T) -> u64 { diff --git a/tests-integration/tests/xor_test/parts.rs b/tests-integration/tests/xor_test/parts.rs index 33885389..ebbdb022 100644 --- a/tests-integration/tests/xor_test/parts.rs +++ b/tests-integration/tests/xor_test/parts.rs @@ -104,7 +104,6 @@ fn raw_parts_reject_invalid_layouts_and_counts() { (1, 4, None), (usize::MAX, 3, None), (1, 3, Some(4)), - (1, 3, Some(usize::MAX)), ] { assert_eq!( XorFilter::from_parts( From cb079f874dd346bb0ecd1042dde0c9cb54f1f29b Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 22:27:20 +0800 Subject: [PATCH 04/16] refactor(xor): decouple membership queries from item counts --- datasketches/src/xor/filter.rs | 15 ++++++--------- datasketches/src/xor/mod.rs | 4 +--- tests-integration/tests/serde_tests/xor.rs | 3 --- tests-integration/tests/xor_test/parts.rs | 10 ++++++---- 4 files changed, 13 insertions(+), 19 deletions(-) diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 026c419b..5da1c4dc 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -103,9 +103,9 @@ impl XorFingerprints { /// An immutable xor filter for probabilistic set membership. /// /// A query that returns `false` proves that the item was not in the input set. A query that returns -/// `true` may be a false positive, with probability determined by [`XorFilterType`]. A filter -/// known to be empty always returns `false`. Values cannot -/// be added after construction; rebuild the filter when the set changes. +/// `true` may be a false positive, with probability determined by [`XorFilterType`], even for an +/// empty input set. Values cannot be added after construction; rebuild the filter when the set +/// changes. /// /// Ordinary values passed to [`contains`](Self::contains) are reduced to 64-bit hashes with /// xxHash64 and seed `0`. Use [`contains_hash`](Self::contains_hash) only with the same hashes that @@ -260,7 +260,7 @@ impl XorFilter { /// with the original input hashes. A supplied item count must describe the original set. /// A valid layout alone cannot establish these invariants. /// Pass `None` when the storage format omits the item count. Standard serialization requires - /// a known count; a known zero count makes every query return `false`. + /// a known count. The item count does not affect membership queries. /// /// # Errors /// @@ -341,7 +341,7 @@ impl XorFilter { let mixed = mix(hash, self.seed); let [h0, h1, h2] = indexes(mixed, self.segment_length); - let found = match &self.fingerprints { + match &self.fingerprints { XorFingerprints::Xor8(fingerprints) => { fingerprint(mixed) as u8 == fingerprints[h0] ^ fingerprints[h1] ^ fingerprints[h2] } @@ -351,10 +351,7 @@ impl XorFilter { XorFingerprints::Xor32(fingerprints) => { fingerprint(mixed) as u32 == fingerprints[h0] ^ fingerprints[h1] ^ fingerprints[h2] } - }; - // Avoid branching on the fingerprint comparison, including rare false - // positives. Unknown counts must preserve the raw comparison. - found & (self.num_items != Some(0)) + } } /// Returns whether the input set was empty, or `None` if imported without an item count. diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index 104edc18..71f2937c 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -48,9 +48,7 @@ //! //! [`XorFilter::from_parts`] imports a construction seed, segment length, and typed fingerprint //! slots without adding metadata to an existing disk format. Accessors expose these fields for -//! serialization. Imports may omit the item count. Filters with a -//! known zero item count always reject queries; imported parts with an unknown count retain the -//! ordinary fingerprint comparison, including possible false positives. +//! serialization. Imports may omit the item count, which does not affect membership queries. //! //! # Examples //! diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index f21e4888..90e6a61e 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -90,9 +90,6 @@ fn serialization_round_trips_standard_fingerprint_types() { for hash in 0..count { assert!(restored.contains_hash(hash)); } - if count == 0 { - assert!(!restored.contains_hash(restored.seed().wrapping_neg())); - } } } } diff --git a/tests-integration/tests/xor_test/parts.rs b/tests-integration/tests/xor_test/parts.rs index ebbdb022..2bc958d5 100644 --- a/tests-integration/tests/xor_test/parts.rs +++ b/tests-integration/tests/xor_test/parts.rs @@ -173,7 +173,7 @@ fn construction_recovers_after_an_unpeelable_first_attempt() { } #[test] -fn unknown_count_does_not_infer_emptiness_from_zero_fingerprints() { +fn item_count_does_not_change_queries() { for kind in [ XorFilterType::Xor8, XorFilterType::Xor16, @@ -182,7 +182,8 @@ fn unknown_count_does_not_infer_emptiness_from_zero_fingerprints() { let empty = XorFilter::from_hashes(&[], kind, 0).unwrap(); let zero_hash = empty.seed().wrapping_neg(); assert_eq!(empty.is_empty(), Some(true)); - assert!(!empty.contains_hash(zero_hash)); + // This query produces a false positive even though the input set is empty. + assert!(empty.contains_hash(zero_hash)); let unknown = XorFilter::from_parts( empty.seed(), empty.segment_length(), @@ -192,8 +193,9 @@ fn unknown_count_does_not_infer_emptiness_from_zero_fingerprints() { .unwrap(); assert_eq!(unknown.is_empty(), None); assert_eq!(unknown.num_items(), None); - // An omitted count must preserve the raw fingerprint comparison, even for all-zero slots. - assert!(unknown.contains_hash(zero_hash)); + for hash in (0..1_000).chain([zero_hash]) { + assert_eq!(unknown.contains_hash(hash), empty.contains_hash(hash)); + } assert_eq!( unknown.serialize().unwrap_err().kind(), ErrorKind::InvalidArgument From 6faf16148b10fab8363d5b6bb7a78f11acb68e70 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 22:59:33 +0800 Subject: [PATCH 05/16] refactor(xor): derive imported segment lengths from fingerprints --- benchmarks/xor/query_shards.rs | 22 +++------- datasketches/src/xor/filter.rs | 30 +++++-------- datasketches/src/xor/mod.rs | 2 +- tests-integration/tests/serde_tests/xor.rs | 4 +- tests-integration/tests/xor_test/parts.rs | 50 +++++----------------- 5 files changed, 29 insertions(+), 79 deletions(-) diff --git a/benchmarks/xor/query_shards.rs b/benchmarks/xor/query_shards.rs index e85579f7..013ad126 100644 --- a/benchmarks/xor/query_shards.rs +++ b/benchmarks/xor/query_shards.rs @@ -56,24 +56,12 @@ impl Reference { } fn into_datasketches(self) -> XorFilter { - let (seed, segment, fingerprints) = match self { - Self::Xor8(f) => ( - f.seed, - f.block_length, - XorFingerprints::Xor8(f.fingerprints), - ), - Self::Xor16(f) => ( - f.seed, - f.block_length, - XorFingerprints::Xor16(f.fingerprints), - ), - Self::Xor32(f) => ( - f.seed, - f.block_length, - XorFingerprints::Xor32(f.fingerprints), - ), + let (seed, fingerprints) = match self { + Self::Xor8(f) => (f.seed, XorFingerprints::Xor8(f.fingerprints)), + Self::Xor16(f) => (f.seed, XorFingerprints::Xor16(f.fingerprints)), + Self::Xor32(f) => (f.seed, XorFingerprints::Xor32(f.fingerprints)), }; - XorFilter::from_parts(seed, segment, fingerprints, None).unwrap() + XorFilter::from_parts(seed, fingerprints, None).unwrap() } } diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 5da1c4dc..f29b91c9 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -112,6 +112,7 @@ impl XorFingerprints { /// were supplied to [`from_hashes`](Self::from_hashes). #[derive(Debug, Clone, PartialEq, Eq)] pub struct XorFilter { + // Cache the derived length to avoid division on every query. segment_length: usize, num_items: Option, seed: u64, @@ -257,15 +258,15 @@ impl XorFilter { /// Imports a filter from its raw fields without requiring a DataSketches serialization header. /// /// The caller must preserve the original construction seed and fingerprint order, and query - /// with the original input hashes. A supplied item count must describe the original set. + /// with the original input hashes. A supplied item count must match the distinct input hashes. /// A valid layout alone cannot establish these invariants. /// Pass `None` when the storage format omits the item count. Standard serialization requires /// a known count. The item count does not affect membership queries. /// /// # Errors /// - /// Returns an error unless the segment length is positive, the payload has exactly three - /// segments, and a supplied item count fits the capacity. Segment lengths above `u32::MAX` + /// Returns an error unless the payload has three equally sized, nonempty segments and a + /// supplied item count fits the capacity. Segment lengths above `u32::MAX` /// are rejected because the index reduction uses 32-bit hash words. /// /// # Examples @@ -275,37 +276,29 @@ impl XorFilter { /// use datasketches::xor::XorFilterType; /// /// let filter = XorFilter::from_hashes(&[10, 20, 30], XorFilterType::Xor8, 0)?; - /// let imported = XorFilter::from_parts( - /// filter.seed(), - /// filter.segment_length(), - /// filter.fingerprints().clone(), - /// None, - /// )?; + /// let imported = XorFilter::from_parts(filter.seed(), filter.fingerprints().clone(), None)?; /// assert!(imported.contains_hash(20)); /// assert_eq!(imported.num_items(), None); /// # Ok::<(), datasketches::error::Error>(()) /// ``` pub fn from_parts( seed: u64, - segment_length: usize, fingerprints: XorFingerprints, num_items: Option, ) -> Result { - if segment_length == 0 - || segment_length > u32::MAX as usize - || segment_length.checked_mul(3) != Some(fingerprints.len()) - { + let slots = fingerprints.len(); + if slots == 0 || slots % 3 != 0 || slots / 3 > u32::MAX as usize { return Err(Error::invalid_argument( "invalid xor filter segment or payload length", )); } - if num_items.is_some_and(|count| count > fingerprints.len()) { + if num_items.is_some_and(|count| count > slots) { return Err(Error::invalid_argument( "item count exceeds xor filter capacity", )); } Ok(Self { - segment_length, + segment_length: slots / 3, num_items, seed, fingerprints, @@ -405,10 +398,7 @@ impl XorFilter { let count = self .num_items .ok_or_else(|| Error::invalid_argument("standard xor format requires an item count"))?; - if count > i32::MAX as usize - || self.segment_length > i32::MAX as usize - || self.fingerprints.len() > i32::MAX as usize - { + if count > i32::MAX as usize || self.fingerprints.len() > i32::MAX as usize { return Err(Error::invalid_argument( "xor filter exceeds standard format limits", )); diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index 71f2937c..88445405 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -46,7 +46,7 @@ //! //! # Application-defined storage //! -//! [`XorFilter::from_parts`] imports a construction seed, segment length, and typed fingerprint +//! [`XorFilter::from_parts`] imports a construction seed and typed fingerprint //! slots without adding metadata to an existing disk format. Accessors expose these fields for //! serialization. Imports may omit the item count, which does not affect membership queries. //! diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index 90e6a61e..67abb05a 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -249,7 +249,6 @@ fn xor32_image_preserves_full_width_little_endian_slots() { )); let filter = XorFilter::from_parts( 0x0123_4567_89ab_cdef, - 1, XorFingerprints::Xor32(vec![0x0123_4567, 0x89ab_cdef, u32::MAX].into_boxed_slice()), Some(1), ) @@ -300,7 +299,8 @@ fn raw_parts_preserve_an_application_storage_layout() { } _ => unreachable!(), }; - let filter = XorFilter::from_parts(seed, segment, payload, None).unwrap(); + let filter = XorFilter::from_parts(seed, payload, None).unwrap(); + assert_eq!(filter.segment_length(), segment); for hash in [0, 1, 2, u64::MAX] { assert!(filter.contains_hash(hash)); } diff --git a/tests-integration/tests/xor_test/parts.rs b/tests-integration/tests/xor_test/parts.rs index 2bc958d5..aee4fed5 100644 --- a/tests-integration/tests/xor_test/parts.rs +++ b/tests-integration/tests/xor_test/parts.rs @@ -31,33 +31,21 @@ fn raw_parts_interoperate_with_xorf_in_both_directions() { XorFilterType::Xor16, XorFilterType::Xor32, ] { - let (seed, segment, payload) = match kind { + let (seed, payload) = match kind { XorFilterType::Xor8 => { let f = xorf::Xor8::from(&keys); - ( - f.seed, - f.block_length, - XorFingerprints::Xor8(f.fingerprints), - ) + (f.seed, XorFingerprints::Xor8(f.fingerprints)) } XorFilterType::Xor16 => { let f = xorf::Xor16::from(&keys); - ( - f.seed, - f.block_length, - XorFingerprints::Xor16(f.fingerprints), - ) + (f.seed, XorFingerprints::Xor16(f.fingerprints)) } XorFilterType::Xor32 => { let f = xorf::Xor32::from(&keys); - ( - f.seed, - f.block_length, - XorFingerprints::Xor32(f.fingerprints), - ) + (f.seed, XorFingerprints::Xor32(f.fingerprints)) } }; - let imported = XorFilter::from_parts(seed, segment, payload, None).unwrap(); + let imported = XorFilter::from_parts(seed, payload, None).unwrap(); assert_eq!(imported.num_items(), None); assert_eq!(imported.is_empty(), None); assert_eq!( @@ -98,22 +86,11 @@ fn raw_parts_interoperate_with_xorf_in_both_directions() { #[test] fn raw_parts_reject_invalid_layouts_and_counts() { - for (segment, length, count) in [ - (0, 0, None), - (1, 2, None), - (1, 4, None), - (usize::MAX, 3, None), - (1, 3, Some(4)), - ] { + for (length, count) in [(0, None), (2, None), (4, None), (3, Some(4))] { assert_eq!( - XorFilter::from_parts( - 0, - segment, - XorFingerprints::Xor8(vec![0; length].into()), - count - ) - .unwrap_err() - .kind(), + XorFilter::from_parts(0, XorFingerprints::Xor8(vec![0; length].into()), count) + .unwrap_err() + .kind(), ErrorKind::InvalidArgument ); } @@ -184,13 +161,8 @@ fn item_count_does_not_change_queries() { assert_eq!(empty.is_empty(), Some(true)); // This query produces a false positive even though the input set is empty. assert!(empty.contains_hash(zero_hash)); - let unknown = XorFilter::from_parts( - empty.seed(), - empty.segment_length(), - empty.fingerprints().clone(), - None, - ) - .unwrap(); + let unknown = + XorFilter::from_parts(empty.seed(), empty.fingerprints().clone(), None).unwrap(); assert_eq!(unknown.is_empty(), None); assert_eq!(unknown.num_items(), None); for hash in (0..1_000).chain([zero_hash]) { From 12dbf187158e2fb5132e840bf2d4224074594e4a Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 23:15:36 +0800 Subject: [PATCH 06/16] refactor(xor): keep segment lengths internal --- datasketches/src/xor/filter.rs | 5 ----- tests-integration/tests/serde_tests/xor.rs | 9 +++++++-- tests-integration/tests/xor_test/parts.rs | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index f29b91c9..1e91f7a4 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -305,11 +305,6 @@ impl XorFilter { }) } - /// Returns the number of fingerprint slots in each of the three segments. - pub fn segment_length(&self) -> usize { - self.segment_length - } - /// Returns the fingerprint slots for application-defined serialization. pub fn fingerprints(&self) -> &XorFingerprints { &self.fingerprints diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index 67abb05a..f4347afb 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -289,6 +289,7 @@ fn raw_parts_preserve_an_application_storage_layout() { let segment = cursor.read_u32_le().unwrap() as usize; let seed = cursor.read_u64_le().unwrap(); let slots = cursor.remaining().len() / usize::from(width / 8); + assert_eq!(segment * 3, slots); let payload = match width { 8 => XorFingerprints::Xor8(cursor.remaining().into()), 16 => { @@ -300,14 +301,18 @@ fn raw_parts_preserve_an_application_storage_layout() { _ => unreachable!(), }; let filter = XorFilter::from_parts(seed, payload, None).unwrap(); - assert_eq!(filter.segment_length(), segment); for hash in [0, 1, 2, u64::MAX] { assert!(filter.contains_hash(hash)); } assert_eq!(filter.num_items(), None); - let (seed, segment) = (filter.seed(), filter.segment_length()); + let seed = filter.seed(); let payload = filter.fingerprints(); + let segment = match payload { + XorFingerprints::Xor8(values) => values.len() / 3, + XorFingerprints::Xor16(values) => values.len() / 3, + XorFingerprints::Xor32(values) => values.len() / 3, + }; let mut encoded = b"TEST".to_vec(); encoded.push(width); encoded.extend((segment as u32).to_le_bytes()); diff --git a/tests-integration/tests/xor_test/parts.rs b/tests-integration/tests/xor_test/parts.rs index aee4fed5..fabd0c77 100644 --- a/tests-integration/tests/xor_test/parts.rs +++ b/tests-integration/tests/xor_test/parts.rs @@ -58,23 +58,23 @@ fn raw_parts_interoperate_with_xorf_in_both_directions() { let built = XorFilter::from_hashes(&keys, kind, 0).unwrap(); assert_eq!(built.num_items(), Some(keys.len())); - let (seed, block_length) = (built.seed(), built.segment_length()); + let seed = built.seed(); let payload = built.fingerprints().clone(); // Compare all answers, including false positives, after exporting the fingerprints. let reference: Box> = match payload { XorFingerprints::Xor8(fingerprints) => Box::new(xorf::Xor8 { seed, - block_length, + block_length: fingerprints.len() / 3, fingerprints, }), XorFingerprints::Xor16(fingerprints) => Box::new(xorf::Xor16 { seed, - block_length, + block_length: fingerprints.len() / 3, fingerprints, }), XorFingerprints::Xor32(fingerprints) => Box::new(xorf::Xor32 { seed, - block_length, + block_length: fingerprints.len() / 3, fingerprints, }), }; From c294258dc92637ccefd89a887ab197fdce5fdc33 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 23:32:50 +0800 Subject: [PATCH 07/16] refactor(xor): separate input collection from immutable filters --- CHANGELOG.md | 2 +- CONTRIBUTING.md | 2 +- benchmarks/xor/build.rs | 22 +- benchmarks/xor/query_shards.rs | 4 +- datasketches/src/codec/family.rs | 4 +- datasketches/src/xor/filter.rs | 191 +++++++----------- datasketches/src/xor/mod.rs | 26 ++- tests-integration/tests/serde_tests/xor.rs | 105 ++++------ tests-integration/tests/xor_test/filter.rs | 50 ++--- .../xor_test/{parts.rs => fingerprints.rs} | 33 +-- tests-integration/tests/xor_test/main.rs | 3 +- 11 files changed, 181 insertions(+), 261 deletions(-) rename tests-integration/tests/xor_test/{parts.rs => fingerprints.rs} (82%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fc9c254..a8684704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All significant changes to this project will be documented in this file. ### New features * Add `HllUnion::serialize` to serialize union results directly. -* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints, construction from values with automatic deduplication or borrowed distinct precomputed hashes, bounded seeded retries and compact temporary storage, optional cardinality metadata for imported filters, standard serialization for all three widths (Java-compatible for 8/16-bit filters), and validated imports for application-defined storage. +* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes, use `XorFilterBuilder` to collect and deduplicate values or hashes, or import a seed and fingerprints with `from_fingerprints`. Serialization uses version 2 without an item count; readers must support the new version and fingerprint width. ### Improvements diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4a06fb04..dab6a116 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -192,7 +192,7 @@ cargo x prepare-testdata java If no language is specified, all languages are prepared. These directories are not stored in Git. Run the command before the first test run and again whenever the pinned TCK revision changes. It requires network access and replaces the selected generated directories. -The pinned TCK revision does not yet contain XOR filter snapshots. Until it does, `serde_tests/xor.rs` retains small hexadecimal fixtures generated from the merged Java implementation, with the source revision and inputs documented alongside them. Keep these separate from the synchronized `*_generated_files` directories; replace them with TCK fixtures when available. +The pinned TCK revision does not yet contain XOR filter snapshots. Until it does, `serde_tests/xor.rs` retains small hexadecimal fixtures generated from the Java implementation, with the source revision and inputs documented alongside them. Keep these separate from the synchronized `*_generated_files` directories; replace them with TCK fixtures when available. ## Code of Conduct diff --git a/benchmarks/xor/build.rs b/benchmarks/xor/build.rs index f203a2fc..d3bcc153 100644 --- a/benchmarks/xor/build.rs +++ b/benchmarks/xor/build.rs @@ -20,6 +20,7 @@ use std::hint::black_box; use benchmarks::Bencher; use benchmarks::Runner; use datasketches::xor::XorFilter; +use datasketches::xor::XorFilterBuilder; use datasketches::xor::XorFilterType; use crate::xor::hashes; @@ -43,10 +44,9 @@ fn xorf_distinct_8(bencher: Bencher, cardinality: usize) { fn deduplicate_8(bencher: Bencher, rows: usize) { let keys = repeated_hashes(rows); bencher.bench_local(|| { - let mut distinct = keys.clone(); - distinct.sort_unstable(); - distinct.dedup(); - black_box(XorFilter::from_hashes(&distinct, XorFilterType::Xor8, 0).unwrap()) + let mut builder = XorFilterBuilder::new(XorFilterType::Xor8, 0); + builder.update_hashes(keys.iter().copied()); + black_box(builder.build().unwrap()) }); } @@ -75,10 +75,9 @@ fn xorf_distinct_16(bencher: Bencher, cardinality: usize) { fn deduplicate_16(bencher: Bencher, rows: usize) { let keys = repeated_hashes(rows); bencher.bench_local(|| { - let mut distinct = keys.clone(); - distinct.sort_unstable(); - distinct.dedup(); - black_box(XorFilter::from_hashes(&distinct, XorFilterType::Xor16, 0).unwrap()) + let mut builder = XorFilterBuilder::new(XorFilterType::Xor16, 0); + builder.update_hashes(keys.iter().copied()); + black_box(builder.build().unwrap()) }); } @@ -107,10 +106,9 @@ fn xorf_distinct_32(bencher: Bencher, cardinality: usize) { fn deduplicate_32(bencher: Bencher, rows: usize) { let keys = repeated_hashes(rows); bencher.bench_local(|| { - let mut distinct = keys.clone(); - distinct.sort_unstable(); - distinct.dedup(); - black_box(XorFilter::from_hashes(&distinct, XorFilterType::Xor32, 0).unwrap()) + let mut builder = XorFilterBuilder::new(XorFilterType::Xor32, 0); + builder.update_hashes(keys.iter().copied()); + black_box(builder.build().unwrap()) }); } diff --git a/benchmarks/xor/query_shards.rs b/benchmarks/xor/query_shards.rs index 013ad126..2df7c63b 100644 --- a/benchmarks/xor/query_shards.rs +++ b/benchmarks/xor/query_shards.rs @@ -30,7 +30,7 @@ const SHARDS: usize = 32; const QUERIES: usize = 256; // Storage formats can choose a fingerprint width at runtime. Give both peers -// this dispatch, and import identical payloads without cardinality metadata. +// this dispatch, and import identical payloads. enum Reference { Xor8(xorf::Xor8), Xor16(xorf::Xor16), @@ -61,7 +61,7 @@ impl Reference { Self::Xor16(f) => (f.seed, XorFingerprints::Xor16(f.fingerprints)), Self::Xor32(f) => (f.seed, XorFingerprints::Xor32(f.fingerprints)), }; - XorFilter::from_parts(seed, fingerprints, None).unwrap() + XorFilter::from_fingerprints(seed, fingerprints).unwrap() } } diff --git a/datasketches/src/codec/family.rs b/datasketches/src/codec/family.rs index 06a8f3d1..d613eec0 100644 --- a/datasketches/src/codec/family.rs +++ b/datasketches/src/codec/family.rs @@ -130,8 +130,8 @@ impl Family { pub const XORFILTER: Family = Family { id: 22, name: "XORFILTER", - min_pre_longs: 3, - max_pre_longs: 3, + min_pre_longs: 2, + max_pre_longs: 2, }; } diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 1e91f7a4..0d872ad5 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -29,8 +29,8 @@ use crate::error::Error; use crate::hash::XxHash64; const NUM_HASHES: u8 = 3; -const SERIAL_VERSION: u8 = 1; -const PREAMBLE_BYTES: usize = 3 * size_of::(); +const SERIAL_VERSION: u8 = 2; +const PREAMBLE_BYTES: usize = 2 * size_of::(); const HASH_SEED: u64 = 0; const LOAD_FACTOR: f64 = 1.23; const CAPACITY_OFFSET: u64 = 32; @@ -100,6 +100,64 @@ impl XorFingerprints { } } +/// A collector of values or precomputed hashes for building an [`XorFilter`]. +/// +/// Retains hashes until [`build`](Self::build), which removes duplicates and consumes the builder. +#[derive(Debug)] +pub struct XorFilterBuilder { + ty: XorFilterType, + seed: u64, + hashes: Vec, +} + +impl XorFilterBuilder { + /// Creates a builder with the given fingerprint width and construction seed. + /// + /// The `seed` selects a deterministic sequence of construction attempts; it does not change + /// the hash function used for values. + pub fn new(ty: XorFilterType, seed: u64) -> Self { + Self { + ty, + seed, + hashes: Vec::new(), + } + } + + /// Adds a value hashed by xxHash64 with seed `0`. + /// + /// Query the resulting filter with [`XorFilter::contains`]. + pub fn update(&mut self, item: &T) { + self.update_hash(hash_item(item)); + } + + /// Adds a precomputed 64-bit hash. + /// + /// Query the resulting filter with [`XorFilter::contains_hash`] using the same hash function. + #[inline] + pub fn update_hash(&mut self, hash: u64) { + self.hashes.push(hash); + } + + /// Adds a batch of precomputed 64-bit hashes. + /// + /// Query the resulting filter with [`XorFilter::contains_hash`] using the same hash function. + pub fn update_hashes(&mut self, hashes: impl IntoIterator) { + self.hashes.extend(hashes); + } + + /// Consumes the builder and creates a filter from the distinct hashes. + /// + /// # Errors + /// + /// Returns an error under the size and construction-attempt limits of + /// [`XorFilter::from_hashes`]. + pub fn build(mut self) -> Result { + self.hashes.sort_unstable(); + self.hashes.dedup(); + XorFilter::from_hashes(&self.hashes, self.ty, self.seed) + } +} + /// An immutable xor filter for probabilistic set membership. /// /// A query that returns `false` proves that the item was not in the input set. A query that returns @@ -114,44 +172,11 @@ impl XorFingerprints { pub struct XorFilter { // Cache the derived length to avoid division on every query. segment_length: usize, - num_items: Option, seed: u64, fingerprints: XorFingerprints, } impl XorFilter { - /// Creates a filter from values hashed by xxHash64 with seed `0`. - /// - /// Removes duplicate hashes before construction. The construction `seed` selects a - /// deterministic sequence of attempts; it does not change the hash function used for values. - /// Pass borrowed values to retain them after construction. - /// - /// # Errors - /// - /// Returns an error under the size and construction-attempt limits of - /// [`from_hashes`](Self::from_hashes). Duplicate values are removed before construction. - /// - /// # Examples - /// - /// ``` - /// use datasketches::xor::XorFilter; - /// use datasketches::xor::XorFilterType; - /// - /// let filter = XorFilter::from_values(["apple", "banana"], XorFilterType::Xor8, 0)?; - /// assert!(filter.contains("apple")); - /// # Ok::<(), datasketches::error::Error>(()) - /// ``` - pub fn from_values( - items: impl IntoIterator, - ty: XorFilterType, - seed: u64, - ) -> Result { - let mut hashes: Vec<_> = items.into_iter().map(|item| hash_item(&item)).collect(); - hashes.sort_unstable(); - hashes.dedup(); - Self::from_hashes(&hashes, ty, seed) - } - /// Creates a filter from distinct precomputed 64-bit hashes. /// /// Duplicate hashes return an error. @@ -249,24 +274,20 @@ impl XorFilter { Ok(Self { segment_length, - num_items: Some(num_items), seed: construction_seed, fingerprints, }) } - /// Imports a filter from its raw fields without requiring a DataSketches serialization header. + /// Imports a filter from its construction seed and fingerprint slots. /// /// The caller must preserve the original construction seed and fingerprint order, and query - /// with the original input hashes. A supplied item count must match the distinct input hashes. - /// A valid layout alone cannot establish these invariants. - /// Pass `None` when the storage format omits the item count. Standard serialization requires - /// a known count. The item count does not affect membership queries. + /// with the original input hashes. A valid layout alone cannot establish these invariants. /// /// # Errors /// - /// Returns an error unless the payload has three equally sized, nonempty segments and a - /// supplied item count fits the capacity. Segment lengths above `u32::MAX` + /// Returns an error unless the payload has three equally sized, nonempty segments. + /// Segment lengths above `u32::MAX` /// are rejected because the index reduction uses 32-bit hash words. /// /// # Examples @@ -276,30 +297,19 @@ impl XorFilter { /// use datasketches::xor::XorFilterType; /// /// let filter = XorFilter::from_hashes(&[10, 20, 30], XorFilterType::Xor8, 0)?; - /// let imported = XorFilter::from_parts(filter.seed(), filter.fingerprints().clone(), None)?; + /// let imported = XorFilter::from_fingerprints(filter.seed(), filter.fingerprints().clone())?; /// assert!(imported.contains_hash(20)); - /// assert_eq!(imported.num_items(), None); /// # Ok::<(), datasketches::error::Error>(()) /// ``` - pub fn from_parts( - seed: u64, - fingerprints: XorFingerprints, - num_items: Option, - ) -> Result { + pub fn from_fingerprints(seed: u64, fingerprints: XorFingerprints) -> Result { let slots = fingerprints.len(); if slots == 0 || slots % 3 != 0 || slots / 3 > u32::MAX as usize { return Err(Error::invalid_argument( "invalid xor filter segment or payload length", )); } - if num_items.is_some_and(|count| count > slots) { - return Err(Error::invalid_argument( - "item count exceeds xor filter capacity", - )); - } Ok(Self { segment_length: slots / 3, - num_items, seed, fingerprints, }) @@ -342,18 +352,6 @@ impl XorFilter { } } - /// Returns whether the input set was empty, or `None` if imported without an item count. - /// - /// Fingerprint contents cannot reliably establish whether the original set was empty. - pub fn is_empty(&self) -> Option { - self.num_items.map(|count| count == 0) - } - - /// Returns the number of distinct input hashes, or `None` if imported without a count. - pub fn num_items(&self) -> Option { - self.num_items - } - /// Returns the fingerprint representation. pub fn filter_type(&self) -> XorFilterType { match &self.fingerprints { @@ -373,27 +371,20 @@ impl XorFilter { size_of::() + self.fingerprints.byte_len() } - /// Serializes the filter using DataSketches family `22`, version `1`. + /// Serializes the filter using DataSketches family `22`, version `2`. /// /// Supports 8-, 16-, and 32-bit fingerprints, recorded in the header's fingerprint width - /// field. Reserved fields are written as zero; integers and fingerprints are little-endian. - /// The 8- and 16-bit images are compatible with Java when input hashing agrees. Readers - /// must support the encoded fingerprint width; implementations limited to 8/16 bits cannot - /// read 32-bit images. + /// field. Integers and fingerprints are little-endian. Readers must support version `2` + /// and the encoded fingerprint width; implementations limited to 8/16 bits cannot read + /// 32-bit images. Version `1` images are not supported. /// /// # Errors /// - /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) for an unknown - /// item count, or an item count or segment length above `i32::MAX`. - /// The total number of slots must also fit `i32::MAX` so that Java can index all three - /// segments; this is an additional interoperability limit, not a header field. Construction - /// and raw parts allow larger capacities, so successful construction does not guarantee - /// standard serialization. Use raw parts for other storage formats. + /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) if the fingerprint + /// slot count exceeds `i32::MAX` or the serialized size overflows `usize`. Construction and + /// imports allow larger capacities than the standard format. pub fn serialize(&self) -> Result, Error> { - let count = self - .num_items - .ok_or_else(|| Error::invalid_argument("standard xor format requires an item count"))?; - if count > i32::MAX as usize || self.fingerprints.len() > i32::MAX as usize { + if self.fingerprints.len() > i32::MAX as usize { return Err(Error::invalid_argument( "xor filter exceeds standard format limits", )); @@ -406,13 +397,9 @@ impl XorFilter { bytes.write_u8(Family::XORFILTER.min_pre_longs); bytes.write_u8(SERIAL_VERSION); bytes.write_u8(Family::XORFILTER.id); - bytes.write_u8(0); // flags bytes.write_u8(self.filter_type().bits_per_fingerprint()); - bytes.write_u8(NUM_HASHES); - bytes.write_u16_le(0); // unused - bytes.write_u64_le(self.seed); bytes.write_i32_le(self.segment_length as i32); - bytes.write_i32_le(count as i32); + bytes.write_u64_le(self.seed); match &self.fingerprints { XorFingerprints::Xor8(fingerprints) => bytes.write(fingerprints), @@ -434,7 +421,7 @@ impl XorFilter { /// Deserializes an owned filter from bytes. /// /// Accepts 8-, 16-, and 32-bit images within the limits of [`serialize`](Self::serialize). - /// Reserved fields and trailing bytes are ignored. + /// Trailing bytes are ignored. /// /// # Errors /// @@ -450,12 +437,9 @@ impl XorFilter { .read_u8() .map_err(insufficient_data("serial_version"))?; let family_id = cursor.read_u8().map_err(insufficient_data("family_id"))?; - let _flags = cursor.read_u8().map_err(insufficient_data("flags"))?; let bits_per_fingerprint = cursor .read_u8() .map_err(insufficient_data("bits_per_fingerprint"))?; - let num_hashes = cursor.read_u8().map_err(insufficient_data("num_hashes"))?; - let _unused = cursor.read_u16_le().map_err(insufficient_data("unused"))?; Family::XORFILTER.validate_id(family_id)?; ensure_serial_version_is(SERIAL_VERSION, serial_version)?; @@ -473,13 +457,6 @@ impl XorFilter { )); } }; - if num_hashes != NUM_HASHES { - return Err(Error::deserial(format!( - "invalid number of hashes: expected {NUM_HASHES}, got {num_hashes}" - ))); - } - - let seed = cursor.read_u64_le().map_err(insufficient_data("seed"))?; let segment_length = cursor .read_i32_le() .map_err(insufficient_data("segment_length"))?; @@ -490,15 +467,7 @@ impl XorFilter { } let segment_length = segment_length as usize; - let num_items = cursor - .read_i32_le() - .map_err(insufficient_data("num_items"))?; - if num_items < 0 { - return Err(Error::deserial(format!( - "invalid item count: expected a non-negative value, got {num_items}" - ))); - } - let num_items = num_items as usize; + let seed = cursor.read_u64_le().map_err(insufficient_data("seed"))?; let capacity = segment_length .checked_mul(usize::from(NUM_HASHES)) @@ -509,11 +478,6 @@ impl XorFilter { i32::MAX ))); } - if num_items > capacity { - return Err(Error::deserial(format!( - "invalid item count: capacity is {capacity}, got {num_items}" - ))); - } let fingerprint_bytes = capacity .checked_mul(ty.bytes_per_fingerprint()) @@ -562,7 +526,6 @@ impl XorFilter { Ok(Self { segment_length, - num_items: Some(num_items), seed, fingerprints, }) diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index 88445405..d754f248 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -22,16 +22,16 @@ //! performs exactly three fingerprint reads per query. Construction needs temporary space linear //! in the number of distinct hashes. //! -//! [`XorFilter::from_values`] hashes ordinary Rust values with xxHash64. Call -//! [`XorFilter::from_hashes`] with distinct hashes -//! when hashes have already been computed and must not be hashed again. +//! [`XorFilterBuilder`] collects values or precomputed hashes and removes duplicates when built. +//! Use [`XorFilter::from_hashes`] when the input hashes are already distinct, or +//! [`XorFilter::from_fingerprints`] to import an existing filter. //! //! # Cross-language hashing //! -//! Standard serialization supports 8-, 16-, and 32-bit fingerprints using family `22`, version `1`. -//! Interoperability requires the reader to support the encoded fingerprint width. The 8- and -//! 16-bit images match Java's format; 32-bit images extend the same layout with little-endian -//! `u32` slots and cannot be read by implementations limited to 8/16 bits. +//! Standard serialization supports 8-, 16-, and 32-bit fingerprints using family `22`, version `2`. +//! Interoperability requires the reader to support this version and the encoded fingerprint width. +//! Version `1` images are not supported. The format stores the seed, segment layout, and +//! fingerprints, without an item count. //! Construction may choose a different valid fingerprint array from another implementation; //! byte-for-byte equality of independently constructed filters is not guaranteed. Rust's //! [`Hash`](std::hash::Hash) implementations do not always encode values like other languages. Use @@ -46,17 +46,20 @@ //! //! # Application-defined storage //! -//! [`XorFilter::from_parts`] imports a construction seed and typed fingerprint +//! [`XorFilter::from_fingerprints`] imports a construction seed and typed fingerprint //! slots without adding metadata to an existing disk format. Accessors expose these fields for -//! serialization. Imports may omit the item count, which does not affect membership queries. +//! serialization. //! //! # Examples //! //! ``` -//! use datasketches::xor::XorFilter; +//! use datasketches::xor::XorFilterBuilder; //! use datasketches::xor::XorFilterType; //! -//! let filter = XorFilter::from_values(["apple", "banana"], XorFilterType::Xor8, 0).unwrap(); +//! let mut builder = XorFilterBuilder::new(XorFilterType::Xor8, 0); +//! builder.update("apple"); +//! builder.update("banana"); +//! let filter = builder.build().unwrap(); //! //! assert!(filter.contains(&"apple")); //! ``` @@ -69,5 +72,6 @@ mod filter; pub use self::filter::XorFilter; +pub use self::filter::XorFilterBuilder; pub use self::filter::XorFilterType; pub use self::filter::XorFingerprints; diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index f4347afb..6ed00203 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -17,28 +17,20 @@ use datasketches::error::ErrorKind; use datasketches::xor::XorFilter; +use datasketches::xor::XorFilterBuilder; use datasketches::xor::XorFilterType; use datasketches::xor::XorFingerprints; -// Generated from datasketches-java commit 1034fb33b6e013334ff870f130b9c388cae5dfa9. +// Generated from datasketches-java commit a038a8b298e3580da73f4c8ee9d0c8e33f77a607. // XorFilterBuilder.create([1, 2, 3, Long.MIN_VALUE], bits, 42).toByteArray(). const JAVA_XOR8_IMAGE: &str = concat!( - "0301160008030000956eeb2f2632d7bd0c00000004000000", - "00000000000000000000c500000000000000000000000000dd000000040000000b000000" + "020216080c000000956eeb2f2632d7bd", + "000000000000000400dd0000000000000000ce0000c50000000000000000000000000000" ); const JAVA_XOR16_IMAGE: &str = concat!( - "0301160010030000", - "956eeb2f2632d7bd", - "0c00000004000000", - "0000000000000000", - "0000000000000000", - "00000000c5b60000", - "0000000000000000", - "0000000000000000", - "0000000000000000", - "dd6e000000000000", - "04e8000000000000", - "0b23000000000000", + "020216100c000000956eeb2f2632d7bd", + "000000000000000000000000000004e80000dd6e00000000000000000000000000000000", + "ce9500000000c5b600000000000000000000000000000000000000000000000000000000", ); #[test] @@ -50,19 +42,16 @@ fn java_images_round_trip_byte_for_byte() { (XorFilterType::Xor16, JAVA_XOR16_IMAGE), ] { let expected = decode_hex(image); - let built = XorFilter::from_values(values, ty, 42).unwrap(); + let mut builder = XorFilterBuilder::new(ty, 42); + for value in values { + builder.update(&value); + } + let built = builder.build().unwrap(); let restored = XorFilter::deserialize(&expected).unwrap(); assert_eq!(restored.serialize().unwrap(), expected); - // Peelability does not depend on peeling order, so both builders select the same - // construction seed and layout. Fingerprint slots may still differ by peeling order. - assert_eq!( - built.serialize().unwrap()[..24], - expected[..24], - "{ty:?} header differs from Java" - ); + assert_eq!(built.serialize().unwrap(), expected); assert_eq!(restored.filter_type(), ty); - assert_eq!(restored.num_items(), Some(values.len())); for value in values { assert!(built.contains(&value)); assert!( @@ -123,14 +112,14 @@ fn invalid_preamble_fields_are_rejected() { .unwrap(); for (offset, value) in [ - (0, 2), - (0, 4), - (1, 2), + (0, 1), + (0, 3), + (1, 1), + (1, 3), (2, 21), - (4, 7), - (4, 24), - (4, 64), - (5, 4), + (3, 7), + (3, 24), + (3, 64), ] { let mut corrupted = valid.clone(); corrupted[offset] = value; @@ -149,16 +138,7 @@ fn unsafe_lengths_are_rejected_before_indexing() { for segment_length in [0_i32, -1, i32::MAX] { let mut corrupted = valid.clone(); - corrupted[16..20].copy_from_slice(&segment_length.to_le_bytes()); - assert_eq!( - XorFilter::deserialize(&corrupted).unwrap_err().kind(), - ErrorKind::InvalidData - ); - } - - for num_items in [-1_i32, i32::MAX] { - let mut corrupted = valid.clone(); - corrupted[20..24].copy_from_slice(&num_items.to_le_bytes()); + corrupted[4..8].copy_from_slice(&segment_length.to_le_bytes()); assert_eq!( XorFilter::deserialize(&corrupted).unwrap_err().kind(), ErrorKind::InvalidData @@ -197,39 +177,27 @@ fn decode_hex(input: &str) -> Vec { #[test] fn java_utf8_and_raw_byte_inputs_match() { use datasketches::hash::value::raw_bytes; - // The same merged Java implementation, using update(String) and update(byte[]). + // The same Java revision, using update(String) and update(byte[]). let images = [ ( XorFilterType::Xor8, - "0301160008030000956eeb2f2632d7bd0c0000000400000000000000000000000000000000df00000000000000000000cc0000003000000091000000", + "020216080c000000956eeb2f2632d7bdccdf00000000000000ef0091000000000000000000000000000000000000000000000000", ), ( XorFilterType::Xor16, - "0301160010030000956eeb2f2632d7bd0c000000040000000000000000000000000000000000000000000000000000000000df500000000000000000000000000000000000000000ccb5000000000000301200000000000091b8000000000000", + "020216100c000000956eeb2f2632d7bdccb5df500000000000000000000000000000ef42000091b8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", ), ]; for (kind, image) in images { - let built = XorFilter::from_values( - [ - raw_bytes::from_slice(b"hello"), - raw_bytes::from_slice("数据🔎".as_bytes()), - raw_bytes::from_slice(&[]), - raw_bytes::from_slice(&[0, 1, 255]), - ], - kind, - 42, - ) - .unwrap(); + let mut builder = XorFilterBuilder::new(kind, 42); + for bytes in [b"hello".as_slice(), "数据🔎".as_bytes(), &[], &[0, 1, 255]] { + builder.update(&raw_bytes::from_slice(bytes)); + } + let built = builder.build().unwrap(); let expected = decode_hex(image); let restored = XorFilter::deserialize(&expected).unwrap(); assert_eq!(restored.serialize().unwrap(), expected); - // Peelability does not depend on peeling order, so both builders select the same - // construction seed and layout. Fingerprint slots may still differ by peeling order. - assert_eq!( - built.serialize().unwrap()[..24], - expected[..24], - "{kind:?} header differs from Java" - ); + assert_eq!(built.serialize().unwrap(), expected); for bytes in [b"hello".as_slice(), "数据🔎".as_bytes(), &[], &[0, 1, 255]] { assert!(built.contains(&raw_bytes::from_slice(bytes))); assert!(restored.contains(&raw_bytes::from_slice(bytes))); @@ -240,17 +208,15 @@ fn java_utf8_and_raw_byte_inputs_match() { #[test] fn xor32_image_preserves_full_width_little_endian_slots() { // Hand-authored layout fixture, independent of the construction algorithm: - // family 22/version 1, width 32, three one-slot segments, one item. + // family 22/version 2, width 32, three one-slot segments. let bytes = decode_hex(concat!( - "0301160020030000", + "0202162001000000", "efcdab8967452301", - "0100000001000000", "67452301efcdab89ffffffff", )); - let filter = XorFilter::from_parts( + let filter = XorFilter::from_fingerprints( 0x0123_4567_89ab_cdef, XorFingerprints::Xor32(vec![0x0123_4567, 0x89ab_cdef, u32::MAX].into_boxed_slice()), - Some(1), ) .unwrap(); assert_eq!(filter.serialize().unwrap(), bytes); @@ -258,7 +224,7 @@ fn xor32_image_preserves_full_width_little_endian_slots() { } #[test] -fn raw_parts_preserve_an_application_storage_layout() { +fn fingerprints_preserve_an_application_storage_layout() { use datasketches::codec::SketchSlice; // Synthetic storage fixtures with public xorf 0.13.0 payloads for [0, 1, 2, u64::MAX]. // This test-only envelope omits cardinality: magic, width:u8, segment:u32, seed:u64, @@ -300,11 +266,10 @@ fn raw_parts_preserve_an_application_storage_layout() { } _ => unreachable!(), }; - let filter = XorFilter::from_parts(seed, payload, None).unwrap(); + let filter = XorFilter::from_fingerprints(seed, payload).unwrap(); for hash in [0, 1, 2, u64::MAX] { assert!(filter.contains_hash(hash)); } - assert_eq!(filter.num_items(), None); let seed = filter.seed(); let payload = filter.fingerprints(); diff --git a/tests-integration/tests/xor_test/filter.rs b/tests-integration/tests/xor_test/filter.rs index 15e7e93f..26ff979d 100644 --- a/tests-integration/tests/xor_test/filter.rs +++ b/tests-integration/tests/xor_test/filter.rs @@ -16,11 +16,16 @@ // under the License. use datasketches::xor::XorFilter; +use datasketches::xor::XorFilterBuilder; use datasketches::xor::XorFilterType; #[test] fn values_have_no_false_negatives() { - let filter = XorFilter::from_values(0..10_000_u64, XorFilterType::Xor8, 8123).unwrap(); + let mut builder = XorFilterBuilder::new(XorFilterType::Xor8, 8123); + for value in 0..10_000_u64 { + builder.update(&value); + } + let filter = builder.build().unwrap(); for value in 0..10_000_u64 { assert!(filter.contains(&value), "false negative for {value}"); @@ -46,32 +51,29 @@ fn precomputed_hashes_have_no_false_negatives() { } #[test] -fn borrowed_values_can_be_reused() { - let values = vec!["datasketches".to_string(), "xor filter".to_string()]; - let first = XorFilter::from_values(&values, XorFilterType::Xor16, 42).unwrap(); - let second = XorFilter::from_values(&values, XorFilterType::Xor16, 42).unwrap(); - assert_eq!(first, second); - for value in &values { - assert!(first.contains(value)); +fn builder_deduplicates_values_and_hashes() { + for ty in [ + XorFilterType::Xor8, + XorFilterType::Xor16, + XorFilterType::Xor32, + ] { + let mut values = XorFilterBuilder::new(ty, 0); + let mut hashes = XorFilterBuilder::new(ty, 0); + for value in [7_u64, 7, 11, 11, 11] { + values.update(&value); + hashes.update_hash(value); + } + hashes.update_hashes([13, 7, 13]); + let filter = values.build().unwrap(); + assert!(filter.contains(&7_u64)); + assert!(filter.contains(&11_u64)); + assert_eq!( + hashes.build().unwrap(), + XorFilter::from_hashes(&[7, 11, 13], ty, 0).unwrap() + ); } } -#[test] -fn duplicates_are_removed_before_construction() { - let filter = XorFilter::from_values([7_u64, 7, 11, 11, 11], XorFilterType::Xor8, 0).unwrap(); - assert_eq!(filter.num_items(), Some(2)); - assert!(filter.contains(&7_u64)); - assert!(filter.contains(&11_u64)); -} - -#[test] -fn empty_filter_is_well_formed() { - let filter = XorFilter::from_hashes(&[], XorFilterType::Xor16, 0).unwrap(); - - assert_eq!(filter.is_empty(), Some(true)); - assert_eq!(filter.num_items(), Some(0)); -} - #[test] fn false_positive_rates_follow_fingerprint_width() { const NUM_ITEMS: u64 = 50_000; diff --git a/tests-integration/tests/xor_test/parts.rs b/tests-integration/tests/xor_test/fingerprints.rs similarity index 82% rename from tests-integration/tests/xor_test/parts.rs rename to tests-integration/tests/xor_test/fingerprints.rs index fabd0c77..c8436826 100644 --- a/tests-integration/tests/xor_test/parts.rs +++ b/tests-integration/tests/xor_test/fingerprints.rs @@ -22,7 +22,7 @@ use datasketches::xor::XorFingerprints; use xorf::Filter; #[test] -fn raw_parts_interoperate_with_xorf_in_both_directions() { +fn fingerprints_interoperate_with_xorf_in_both_directions() { let keys: Vec<_> = (0..16_384u64) .map(|n| n.wrapping_mul(0x9e3779b97f4a7c15)) .collect(); @@ -45,19 +45,16 @@ fn raw_parts_interoperate_with_xorf_in_both_directions() { (f.seed, XorFingerprints::Xor32(f.fingerprints)) } }; - let imported = XorFilter::from_parts(seed, payload, None).unwrap(); - assert_eq!(imported.num_items(), None); - assert_eq!(imported.is_empty(), None); + let imported = XorFilter::from_fingerprints(seed, payload).unwrap(); assert_eq!( - imported.serialize().unwrap_err().kind(), - ErrorKind::InvalidArgument + XorFilter::deserialize(&imported.serialize().unwrap()).unwrap(), + imported ); for &key in &keys { assert!(imported.contains_hash(key)); } let built = XorFilter::from_hashes(&keys, kind, 0).unwrap(); - assert_eq!(built.num_items(), Some(keys.len())); let seed = built.seed(); let payload = built.fingerprints().clone(); // Compare all answers, including false positives, after exporting the fingerprints. @@ -85,10 +82,10 @@ fn raw_parts_interoperate_with_xorf_in_both_directions() { } #[test] -fn raw_parts_reject_invalid_layouts_and_counts() { - for (length, count) in [(0, None), (2, None), (4, None), (3, Some(4))] { +fn fingerprints_reject_invalid_layouts() { + for length in [0, 2, 4] { assert_eq!( - XorFilter::from_parts(0, XorFingerprints::Xor8(vec![0; length].into()), count) + XorFilter::from_fingerprints(0, XorFingerprints::Xor8(vec![0; length].into())) .unwrap_err() .kind(), ErrorKind::InvalidArgument @@ -108,7 +105,6 @@ fn distinct_construction_is_bounded_and_does_not_deduplicate() { assert!(error.message().contains("duplicate hashes")); for keys in [vec![], vec![0], vec![u64::MAX], vec![0, u64::MAX]] { let f = XorFilter::from_hashes(&keys, kind, 0).unwrap(); - assert_eq!(f.num_items(), Some(keys.len())); for key in keys { assert!(f.contains_hash(key)); } @@ -150,7 +146,7 @@ fn construction_recovers_after_an_unpeelable_first_attempt() { } #[test] -fn item_count_does_not_change_queries() { +fn empty_input_uses_fingerprint_queries() { for kind in [ XorFilterType::Xor8, XorFilterType::Xor16, @@ -158,19 +154,12 @@ fn item_count_does_not_change_queries() { ] { let empty = XorFilter::from_hashes(&[], kind, 0).unwrap(); let zero_hash = empty.seed().wrapping_neg(); - assert_eq!(empty.is_empty(), Some(true)); // This query produces a false positive even though the input set is empty. assert!(empty.contains_hash(zero_hash)); - let unknown = - XorFilter::from_parts(empty.seed(), empty.fingerprints().clone(), None).unwrap(); - assert_eq!(unknown.is_empty(), None); - assert_eq!(unknown.num_items(), None); + let imported = + XorFilter::from_fingerprints(empty.seed(), empty.fingerprints().clone()).unwrap(); for hash in (0..1_000).chain([zero_hash]) { - assert_eq!(unknown.contains_hash(hash), empty.contains_hash(hash)); + assert_eq!(imported.contains_hash(hash), empty.contains_hash(hash)); } - assert_eq!( - unknown.serialize().unwrap_err().kind(), - ErrorKind::InvalidArgument - ); } } diff --git a/tests-integration/tests/xor_test/main.rs b/tests-integration/tests/xor_test/main.rs index 656a5053..34b03598 100644 --- a/tests-integration/tests/xor_test/main.rs +++ b/tests-integration/tests/xor_test/main.rs @@ -16,5 +16,4 @@ // under the License. mod filter; - -mod parts; +mod fingerprints; From 8a10a5f02546e64c07df1a9cdf7a2ab1bd33ca9c Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 5 Nov 2026 23:43:09 +0800 Subject: [PATCH 08/16] fix(xor): retain the unreleased serialization version --- CHANGELOG.md | 2 +- datasketches/src/xor/filter.rs | 9 ++++----- datasketches/src/xor/mod.rs | 7 +++---- tests-integration/tests/serde_tests/xor.rs | 18 +++++++++--------- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8684704..da294e18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All significant changes to this project will be documented in this file. ### New features * Add `HllUnion::serialize` to serialize union results directly. -* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes, use `XorFilterBuilder` to collect and deduplicate values or hashes, or import a seed and fingerprints with `from_fingerprints`. Serialization uses version 2 without an item count; readers must support the new version and fingerprint width. +* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes, use `XorFilterBuilder` to collect and deduplicate values or hashes, or import a seed and fingerprints with `from_fingerprints`. Serialization stores no item count; readers must support the encoded fingerprint width. ### Improvements diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 0d872ad5..190e9cdf 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -29,7 +29,7 @@ use crate::error::Error; use crate::hash::XxHash64; const NUM_HASHES: u8 = 3; -const SERIAL_VERSION: u8 = 2; +const SERIAL_VERSION: u8 = 1; const PREAMBLE_BYTES: usize = 2 * size_of::(); const HASH_SEED: u64 = 0; const LOAD_FACTOR: f64 = 1.23; @@ -371,12 +371,11 @@ impl XorFilter { size_of::() + self.fingerprints.byte_len() } - /// Serializes the filter using DataSketches family `22`, version `2`. + /// Serializes the filter using DataSketches family `22`, version `1`. /// /// Supports 8-, 16-, and 32-bit fingerprints, recorded in the header's fingerprint width - /// field. Integers and fingerprints are little-endian. Readers must support version `2` - /// and the encoded fingerprint width; implementations limited to 8/16 bits cannot read - /// 32-bit images. Version `1` images are not supported. + /// field. Integers and fingerprints are little-endian. Readers must support the encoded + /// fingerprint width; implementations limited to 8/16 bits cannot read 32-bit images. /// /// # Errors /// diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index d754f248..97ae417d 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -28,10 +28,9 @@ //! //! # Cross-language hashing //! -//! Standard serialization supports 8-, 16-, and 32-bit fingerprints using family `22`, version `2`. -//! Interoperability requires the reader to support this version and the encoded fingerprint width. -//! Version `1` images are not supported. The format stores the seed, segment layout, and -//! fingerprints, without an item count. +//! Standard serialization supports 8-, 16-, and 32-bit fingerprints using family `22`, version `1`. +//! Interoperability requires the reader to support the encoded fingerprint width. The format +//! stores the seed, segment layout, and fingerprints, without an item count. //! Construction may choose a different valid fingerprint array from another implementation; //! byte-for-byte equality of independently constructed filters is not guaranteed. Rust's //! [`Hash`](std::hash::Hash) implementations do not always encode values like other languages. Use diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index 6ed00203..b758f8f9 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -21,14 +21,14 @@ use datasketches::xor::XorFilterBuilder; use datasketches::xor::XorFilterType; use datasketches::xor::XorFingerprints; -// Generated from datasketches-java commit a038a8b298e3580da73f4c8ee9d0c8e33f77a607. +// Generated from datasketches-java commit c179c5bc00d5aadaed8c68ce11d14d812abe08d6. // XorFilterBuilder.create([1, 2, 3, Long.MIN_VALUE], bits, 42).toByteArray(). const JAVA_XOR8_IMAGE: &str = concat!( - "020216080c000000956eeb2f2632d7bd", + "020116080c000000956eeb2f2632d7bd", "000000000000000400dd0000000000000000ce0000c50000000000000000000000000000" ); const JAVA_XOR16_IMAGE: &str = concat!( - "020216100c000000956eeb2f2632d7bd", + "020116100c000000956eeb2f2632d7bd", "000000000000000000000000000004e80000dd6e00000000000000000000000000000000", "ce9500000000c5b600000000000000000000000000000000000000000000000000000000", ); @@ -114,8 +114,8 @@ fn invalid_preamble_fields_are_rejected() { for (offset, value) in [ (0, 1), (0, 3), - (1, 1), - (1, 3), + (1, 0), + (1, 2), (2, 21), (3, 7), (3, 24), @@ -181,11 +181,11 @@ fn java_utf8_and_raw_byte_inputs_match() { let images = [ ( XorFilterType::Xor8, - "020216080c000000956eeb2f2632d7bdccdf00000000000000ef0091000000000000000000000000000000000000000000000000", + "020116080c000000956eeb2f2632d7bdccdf00000000000000ef0091000000000000000000000000000000000000000000000000", ), ( XorFilterType::Xor16, - "020216100c000000956eeb2f2632d7bdccb5df500000000000000000000000000000ef42000091b8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "020116100c000000956eeb2f2632d7bdccb5df500000000000000000000000000000ef42000091b8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", ), ]; for (kind, image) in images { @@ -208,9 +208,9 @@ fn java_utf8_and_raw_byte_inputs_match() { #[test] fn xor32_image_preserves_full_width_little_endian_slots() { // Hand-authored layout fixture, independent of the construction algorithm: - // family 22/version 2, width 32, three one-slot segments. + // family 22/version 1, width 32, three one-slot segments. let bytes = decode_hex(concat!( - "0202162001000000", + "0201162001000000", "efcdab8967452301", "67452301efcdab89ffffffff", )); From 1dd2c25ffe67d7890680523d49b5c3806cfd3404 Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 12:19:29 +0800 Subject: [PATCH 09/16] feat(xor): accept batches of values in the filter builder --- CHANGELOG.md | 2 +- README.md | 2 +- datasketches/src/xor/filter.rs | 66 ++++++++++------------ datasketches/src/xor/mod.rs | 10 ++-- tests-integration/tests/serde_tests/xor.rs | 4 +- tests-integration/tests/xor_test/filter.rs | 11 ++-- 6 files changed, 42 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da294e18..102bed14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All significant changes to this project will be documented in this file. ### New features * Add `HllUnion::serialize` to serialize union results directly. -* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes, use `XorFilterBuilder` to collect and deduplicate values or hashes, or import a seed and fingerprints with `from_fingerprints`. Serialization stores no item count; readers must support the encoded fingerprint width. +* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes or import a seed and fingerprints with `from_fingerprints`. `XorFilterBuilder` accepts values or hashes individually or in batches and removes duplicates when built. Serialization stores no item count; readers must support the encoded fingerprint width. ### Improvements diff --git a/README.md b/README.md index 37a86a31..40c31310 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ See the [API documentation](https://docs.rs/datasketches) for configuration, acc The minimum supported Rust version is 1.86.0. The crate currently supports little-endian targets only. -Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). XOR additionally uses fixtures generated from the merged Java implementation until TCK snapshots are available. Its standard format supports 8-, 16-, and 32-bit fingerprints; 32-bit images require a reader that supports that width. Formats without an item count use the raw-parts API. +Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). XOR also uses Java fixtures until TCK snapshots are available. Its standard format supports 8-, 16-, and 32-bit fingerprints; readers must support the encoded width. Use `XorFilter::from_fingerprints` to import a seed and fingerprints from another format. Serialization compatibility does not imply that an ordinary Rust `Hash` implementation produces the same update bytes across languages. When sketches must represent the same inputs across implementations, use `hash::value::{raw_bytes, canonical_float, sign_extend, natural_extend}` (and the constructors within those modules) to select matching input encodings. When interoperating with an implementation that skips empty strings, skip them before updating here as well. diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 190e9cdf..499dcdda 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -103,44 +103,46 @@ impl XorFingerprints { /// A collector of values or precomputed hashes for building an [`XorFilter`]. /// /// Retains hashes until [`build`](Self::build), which removes duplicates and consumes the builder. +/// Query values with [`XorFilter::contains`] and precomputed hashes with +/// [`XorFilter::contains_hash`]. #[derive(Debug)] pub struct XorFilterBuilder { - ty: XorFilterType, + filter_type: XorFilterType, seed: u64, hashes: Vec, } impl XorFilterBuilder { - /// Creates a builder with the given fingerprint width and construction seed. + /// Creates a builder with the given fingerprint width and initial construction seed. /// - /// The `seed` selects a deterministic sequence of construction attempts; it does not change - /// the hash function used for values. - pub fn new(ty: XorFilterType, seed: u64) -> Self { + /// The `seed` determines which layouts to try during construction. Values are hashed + /// independently of it. + pub fn new(filter_type: XorFilterType, seed: u64) -> Self { Self { - ty, + filter_type, seed, hashes: Vec::new(), } } - /// Adds a value hashed by xxHash64 with seed `0`. - /// - /// Query the resulting filter with [`XorFilter::contains`]. + /// Adds a value. + #[inline] pub fn update(&mut self, item: &T) { self.update_hash(hash_item(item)); } /// Adds a precomputed 64-bit hash. - /// - /// Query the resulting filter with [`XorFilter::contains_hash`] using the same hash function. #[inline] pub fn update_hash(&mut self, hash: u64) { self.hashes.push(hash); } + /// Adds a batch of values. + pub fn update_values(&mut self, items: impl IntoIterator) { + self.update_hashes(items.into_iter().map(|item| hash_item(&item))); + } + /// Adds a batch of precomputed 64-bit hashes. - /// - /// Query the resulting filter with [`XorFilter::contains_hash`] using the same hash function. pub fn update_hashes(&mut self, hashes: impl IntoIterator) { self.hashes.extend(hashes); } @@ -154,7 +156,7 @@ impl XorFilterBuilder { pub fn build(mut self) -> Result { self.hashes.sort_unstable(); self.hashes.dedup(); - XorFilter::from_hashes(&self.hashes, self.ty, self.seed) + XorFilter::from_hashes(&self.hashes, self.filter_type, self.seed) } } @@ -164,10 +166,6 @@ impl XorFilterBuilder { /// `true` may be a false positive, with probability determined by [`XorFilterType`], even for an /// empty input set. Values cannot be added after construction; rebuild the filter when the set /// changes. -/// -/// Ordinary values passed to [`contains`](Self::contains) are reduced to 64-bit hashes with -/// xxHash64 and seed `0`. Use [`contains_hash`](Self::contains_hash) only with the same hashes that -/// were supplied to [`from_hashes`](Self::from_hashes). #[derive(Debug, Clone, PartialEq, Eq)] pub struct XorFilter { // Cache the derived length to avoid division on every query. @@ -179,10 +177,9 @@ pub struct XorFilter { impl XorFilter { /// Creates a filter from distinct precomputed 64-bit hashes. /// - /// Duplicate hashes return an error. - /// Query with [`contains_hash`](Self::contains_hash) using hashes from the same source. - /// The construction `seed` selects a deterministic sequence of attempts; input order - /// does not affect the result. + /// The initial `seed` determines which layouts to try during construction. The same set of + /// hashes, filter type, and seed produce the same filter regardless of input order. + /// Query with [`contains_hash`](Self::contains_hash) using the same hash function. /// /// # Errors /// @@ -191,7 +188,7 @@ impl XorFilter { /// exceed `isize::MAX` bytes, or no peelable construction is found within 100 attempts. /// Valid input can also exhaust the attempts. A successful build may still exceed the /// limits of [`serialize`](Self::serialize). - pub fn from_hashes(keys: &[u64], ty: XorFilterType, seed: u64) -> Result { + pub fn from_hashes(keys: &[u64], filter_type: XorFilterType, seed: u64) -> Result { let num_items = keys.len(); let capacity = compute_capacity(num_items)?; let segment_length = capacity / usize::from(NUM_HASHES); @@ -248,7 +245,7 @@ impl XorFilter { drop(count); drop(queue); - let fingerprints = match ty { + let fingerprints = match filter_type { XorFilterType::Xor8 => XorFingerprints::Xor8(assign_fingerprints( capacity, segment_length, @@ -279,16 +276,14 @@ impl XorFilter { }) } - /// Imports a filter from its construction seed and fingerprint slots. + /// Imports a filter from its stored seed and fingerprint slots. /// - /// The caller must preserve the original construction seed and fingerprint order, and query - /// with the original input hashes. A valid layout alone cannot establish these invariants. + /// Pass the original filter's [`seed`](Self::seed), which can differ from the initial + /// construction seed. Preserve the fingerprint order and query with the original input hashes. /// /// # Errors /// - /// Returns an error unless the payload has three equally sized, nonempty segments. - /// Segment lengths above `u32::MAX` - /// are rejected because the index reduction uses 32-bit hash words. + /// Returns an error unless the payload has three equal segments of `1..=u32::MAX` slots each. /// /// # Examples /// @@ -331,8 +326,7 @@ impl XorFilter { /// Returns `true` if a precomputed 64-bit hash is possibly in the input set. /// - /// This method bypasses xxHash64. The caller must use the same hash value during construction - /// and queries; mixing raw values with precomputed hashes can introduce false negatives. + /// Use the same hash function for construction and queries. // Keep the width dispatch in the caller's hot loop, including across crates. #[inline(always)] pub fn contains_hash(&self, hash: u64) -> bool { @@ -361,7 +355,7 @@ impl XorFilter { } } - /// Returns the construction seed stored with the filter. + /// Returns the seed paired with these fingerprints for queries and imports. pub fn seed(&self) -> u64 { self.seed } @@ -446,7 +440,7 @@ impl XorFilter { Family::XORFILTER.min_pre_longs..=Family::XORFILTER.max_pre_longs, preamble_longs, )?; - let ty = match bits_per_fingerprint { + let filter_type = match bits_per_fingerprint { 8 => XorFilterType::Xor8, 16 => XorFilterType::Xor16, 32 => XorFilterType::Xor32, @@ -479,7 +473,7 @@ impl XorFilter { } let fingerprint_bytes = capacity - .checked_mul(ty.bytes_per_fingerprint()) + .checked_mul(filter_type.bytes_per_fingerprint()) .ok_or_else(|| Error::deserial("xor filter fingerprint size overflow"))?; if cursor.remaining().len() < fingerprint_bytes { return Err(Error::insufficient_data_of( @@ -491,7 +485,7 @@ impl XorFilter { )); } - let fingerprints = match ty { + let fingerprints = match filter_type { XorFilterType::Xor8 => { let mut values = vec![0_u8; capacity].into_boxed_slice(); cursor diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index 97ae417d..b47d6aeb 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -45,9 +45,8 @@ //! //! # Application-defined storage //! -//! [`XorFilter::from_fingerprints`] imports a construction seed and typed fingerprint -//! slots without adding metadata to an existing disk format. Accessors expose these fields for -//! serialization. +//! [`XorFilter::from_fingerprints`] restores a filter from its stored [`seed`](XorFilter::seed) +//! and [`fingerprints`](XorFilter::fingerprints). //! //! # Examples //! @@ -56,11 +55,10 @@ //! use datasketches::xor::XorFilterType; //! //! let mut builder = XorFilterBuilder::new(XorFilterType::Xor8, 0); -//! builder.update("apple"); -//! builder.update("banana"); +//! builder.update_values(["apple", "banana"]); //! let filter = builder.build().unwrap(); //! -//! assert!(filter.contains(&"apple")); +//! assert!(filter.contains("apple")); //! ``` //! //! # References diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index b758f8f9..2a9aa35a 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -43,9 +43,7 @@ fn java_images_round_trip_byte_for_byte() { ] { let expected = decode_hex(image); let mut builder = XorFilterBuilder::new(ty, 42); - for value in values { - builder.update(&value); - } + builder.update_values(values.iter()); let built = builder.build().unwrap(); let restored = XorFilter::deserialize(&expected).unwrap(); diff --git a/tests-integration/tests/xor_test/filter.rs b/tests-integration/tests/xor_test/filter.rs index 26ff979d..2deec7fb 100644 --- a/tests-integration/tests/xor_test/filter.rs +++ b/tests-integration/tests/xor_test/filter.rs @@ -59,17 +59,16 @@ fn builder_deduplicates_values_and_hashes() { ] { let mut values = XorFilterBuilder::new(ty, 0); let mut hashes = XorFilterBuilder::new(ty, 0); - for value in [7_u64, 7, 11, 11, 11] { - values.update(&value); - hashes.update_hash(value); - } - hashes.update_hashes([13, 7, 13]); + values.update(&7_u64); + values.update_values([11_u64, 7, 11]); + hashes.update_hash(7); + hashes.update_hashes([11, 7, 11]); let filter = values.build().unwrap(); assert!(filter.contains(&7_u64)); assert!(filter.contains(&11_u64)); assert_eq!( hashes.build().unwrap(), - XorFilter::from_hashes(&[7, 11, 13], ty, 0).unwrap() + XorFilter::from_hashes(&[7, 11], ty, 0).unwrap() ); } } From 21126c16a6199548eeff1e98976458777f1353af Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 12:28:41 +0800 Subject: [PATCH 10/16] docs(xor): express false-positive probabilities as powers of two --- datasketches/src/xor/filter.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 499dcdda..b9acc1a6 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -46,10 +46,9 @@ const SPLITMIX_MUL2: u64 = 0x94d0_49bb_1331_11eb; /// Fingerprint representation used by an [`XorFilter`]. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum XorFilterType { - /// Uses 8-bit fingerprints and has an expected false-positive probability of about `1 / 256`. + /// Uses 8-bit fingerprints and has an expected false-positive probability of about `2^-8`. Xor8, - /// Uses 16-bit fingerprints and has an expected false-positive probability of about - /// `1 / 65_536`. + /// Uses 16-bit fingerprints and has an expected false-positive probability of about `2^-16`. Xor16, /// Uses 32-bit fingerprints and has an expected false-positive probability of about `2^-32`. Xor32, From b8306a0bf490daeab63d26421778152487a20c30 Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 12:46:50 +0800 Subject: [PATCH 11/16] fix(xor): clarify construction and fingerprint layout errors --- datasketches/src/xor/filter.rs | 43 +++++++++++++------ .../tests/xor_test/fingerprints.rs | 12 +++--- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index b9acc1a6..d06f5f97 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -64,7 +64,11 @@ impl XorFilterType { } const fn bytes_per_fingerprint(self) -> usize { - (self.bits_per_fingerprint() / 8) as usize + match self { + Self::Xor8 => 1, + Self::Xor16 => 2, + Self::Xor32 => 4, + } } } @@ -150,8 +154,7 @@ impl XorFilterBuilder { /// /// # Errors /// - /// Returns an error under the size and construction-attempt limits of - /// [`XorFilter::from_hashes`]. + /// Returns an error if the set of distinct hashes is too large or no valid layout is found. pub fn build(mut self) -> Result { self.hashes.sort_unstable(); self.hashes.dedup(); @@ -182,11 +185,11 @@ impl XorFilter { /// /// # Errors /// - /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) if hashes contain - /// duplicates, the fingerprint slot count exceeds `u32::MAX`, a scratch allocation would - /// exceed `isize::MAX` bytes, or no peelable construction is found within 100 attempts. - /// Valid input can also exhaust the attempts. A successful build may still exceed the - /// limits of [`serialize`](Self::serialize). + /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) in these cases: + /// + /// - The input contains duplicate hashes. + /// - The input is too large to build a filter. + /// - Construction cannot find a valid layout. For distinct hashes, try another `seed`. pub fn from_hashes(keys: &[u64], filter_type: XorFilterType, seed: u64) -> Result { let num_items = keys.len(); let capacity = compute_capacity(num_items)?; @@ -232,9 +235,9 @@ impl XorFilter { } if stack_size != num_items { - return Err(Error::invalid_argument(format!( - "xor filter construction failed after {MAX_CONSTRUCTION_ATTEMPTS} attempts" - )) + return Err(Error::invalid_argument( + "xor filter construction could not find a valid layout", + ) .with_context("num_items", num_items)); } @@ -297,13 +300,25 @@ impl XorFilter { /// ``` pub fn from_fingerprints(seed: u64, fingerprints: XorFingerprints) -> Result { let slots = fingerprints.len(); - if slots == 0 || slots % 3 != 0 || slots / 3 > u32::MAX as usize { + if slots == 0 { return Err(Error::invalid_argument( - "invalid xor filter segment or payload length", + "xor filter fingerprint array must not be empty", )); } + if slots % 3 != 0 { + return Err(Error::invalid_argument(format!( + "xor filter fingerprint slot count must be a multiple of 3, got {slots}" + ))); + } + let segment_length = slots / 3; + if segment_length > u32::MAX as usize { + return Err(Error::invalid_argument(format!( + "xor filter segment length must be at most {}, got {segment_length}", + u32::MAX + ))); + } Ok(Self { - segment_length: slots / 3, + segment_length, seed, fingerprints, }) diff --git a/tests-integration/tests/xor_test/fingerprints.rs b/tests-integration/tests/xor_test/fingerprints.rs index c8436826..b5d000cb 100644 --- a/tests-integration/tests/xor_test/fingerprints.rs +++ b/tests-integration/tests/xor_test/fingerprints.rs @@ -83,13 +83,11 @@ fn fingerprints_interoperate_with_xorf_in_both_directions() { #[test] fn fingerprints_reject_invalid_layouts() { - for length in [0, 2, 4] { - assert_eq!( - XorFilter::from_fingerprints(0, XorFingerprints::Xor8(vec![0; length].into())) - .unwrap_err() - .kind(), - ErrorKind::InvalidArgument - ); + for (length, reason) in [(0, "empty"), (2, "multiple of 3"), (4, "multiple of 3")] { + let error = XorFilter::from_fingerprints(0, XorFingerprints::Xor8(vec![0; length].into())) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + assert!(error.message().contains(reason), "{error}"); } } From 68a65b478030c37713eb5e6a677d790a19fb2ec6 Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 13:02:09 +0800 Subject: [PATCH 12/16] ci: run macOS tests on macos-15 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecd4d8f3..ff4ca5d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: needs: msrv strategy: matrix: - os: [ ubuntu-24.04, macos-14, windows-2022 ] + os: [ ubuntu-24.04, macos-15, windows-2022 ] rust-version: ${{ fromJson(needs.msrv.outputs.rust-versions) }} runs-on: ${{ matrix.os }} steps: From b199d72ebc272f0f2e508c3b7938591a9ca9d943 Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 14:13:15 +0800 Subject: [PATCH 13/16] refactor(xor): keep fingerprints private and decode Xor8 in one copy --- CHANGELOG.md | 2 +- Cargo.lock | 1 - README.md | 2 +- benchmarks/xor/query_shards.rs | 15 +- datasketches/src/xor/filter.rs | 111 +++--------- datasketches/src/xor/mod.rs | 15 +- tests-integration/Cargo.toml | 1 - tests-integration/tests/serde_tests/xor.rs | 84 +-------- tests-integration/tests/xor_test/filter.rs | 47 +++++ .../tests/xor_test/fingerprints.rs | 163 ------------------ tests-integration/tests/xor_test/main.rs | 1 - 11 files changed, 79 insertions(+), 363 deletions(-) delete mode 100644 tests-integration/tests/xor_test/fingerprints.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 102bed14..8482c947 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All significant changes to this project will be documented in this file. ### New features * Add `HllUnion::serialize` to serialize union results directly. -* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes or import a seed and fingerprints with `from_fingerprints`. `XorFilterBuilder` accepts values or hashes individually or in batches and removes duplicates when built. Serialization stores no item count; readers must support the encoded fingerprint width. +* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes or restore a filter with `XorFilter::deserialize`. `XorFilterBuilder` accepts values or hashes individually or in batches and removes duplicates when built. Serialization stores no item count; readers must support the encoded fingerprint width. ### Improvements diff --git a/Cargo.lock b/Cargo.lock index ccc8f1a9..3cbfdf62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -735,7 +735,6 @@ dependencies = [ "googletest", "insta", "quickcheck", - "xorf", ] [[package]] diff --git a/README.md b/README.md index 40c31310..f07f058e 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ See the [API documentation](https://docs.rs/datasketches) for configuration, acc The minimum supported Rust version is 1.86.0. The crate currently supports little-endian targets only. -Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). XOR also uses Java fixtures until TCK snapshots are available. Its standard format supports 8-, 16-, and 32-bit fingerprints; readers must support the encoded width. Use `XorFilter::from_fingerprints` to import a seed and fingerprints from another format. +Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). XOR also uses Java fixtures until TCK snapshots are available. Its standard format supports 8-, 16-, and 32-bit fingerprints; readers must support the encoded width. Serialization compatibility does not imply that an ordinary Rust `Hash` implementation produces the same update bytes across languages. When sketches must represent the same inputs across implementations, use `hash::value::{raw_bytes, canonical_float, sign_extend, natural_extend}` (and the constructors within those modules) to select matching input encodings. When interoperating with an implementation that skips empty strings, skip them before updating here as well. diff --git a/benchmarks/xor/query_shards.rs b/benchmarks/xor/query_shards.rs index 2df7c63b..d0f91901 100644 --- a/benchmarks/xor/query_shards.rs +++ b/benchmarks/xor/query_shards.rs @@ -21,7 +21,6 @@ use benchmarks::Bencher; use benchmarks::Runner; use datasketches::xor::XorFilter; use datasketches::xor::XorFilterType; -use datasketches::xor::XorFingerprints; use xorf::Filter; use crate::xor::hashes_with_offset; @@ -29,8 +28,7 @@ use crate::xor::hashes_with_offset; const SHARDS: usize = 32; const QUERIES: usize = 256; -// Storage formats can choose a fingerprint width at runtime. Give both peers -// this dispatch, and import identical payloads. +// Both implementations dispatch on fingerprint width at runtime. enum Reference { Xor8(xorf::Xor8), Xor16(xorf::Xor16), @@ -54,15 +52,6 @@ impl Reference { Self::Xor32(filter) => filter.contains(&key), } } - - fn into_datasketches(self) -> XorFilter { - let (seed, fingerprints) = match self { - Self::Xor8(f) => (f.seed, XorFingerprints::Xor8(f.fingerprints)), - Self::Xor16(f) => (f.seed, XorFingerprints::Xor16(f.fingerprints)), - Self::Xor32(f) => (f.seed, XorFingerprints::Xor32(f.fingerprints)), - }; - XorFilter::from_fingerprints(seed, fingerprints).unwrap() - } } fn query( @@ -116,7 +105,7 @@ pub fn register(runner: &mut Runner) { b, cardinality, present, - |keys| Reference::new(keys, kind).into_datasketches(), + |keys| XorFilter::from_hashes(keys, kind, 1).unwrap(), XorFilter::contains_hash, ) }), diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index d06f5f97..e8bd2025 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -72,20 +72,14 @@ impl XorFilterType { } } -/// Owned fingerprint slots for importing and exporting an application-defined format. -/// -/// Slots are ordered as three consecutive, equally sized segments. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum XorFingerprints { - /// 8-bit fingerprint slots. +enum Fingerprints { Xor8(Box<[u8]>), - /// 16-bit fingerprint slots. Xor16(Box<[u16]>), - /// 32-bit fingerprint slots. Xor32(Box<[u32]>), } -impl XorFingerprints { +impl Fingerprints { fn len(&self) -> usize { match self { Self::Xor8(values) => values.len(), @@ -173,7 +167,7 @@ pub struct XorFilter { // Cache the derived length to avoid division on every query. segment_length: usize, seed: u64, - fingerprints: XorFingerprints, + fingerprints: Fingerprints, } impl XorFilter { @@ -248,21 +242,21 @@ impl XorFilter { drop(queue); let fingerprints = match filter_type { - XorFilterType::Xor8 => XorFingerprints::Xor8(assign_fingerprints( + XorFilterType::Xor8 => Fingerprints::Xor8(assign_fingerprints( capacity, segment_length, &stack_hash, &stack_index, |hash| fingerprint(hash) as u8, )), - XorFilterType::Xor16 => XorFingerprints::Xor16(assign_fingerprints( + XorFilterType::Xor16 => Fingerprints::Xor16(assign_fingerprints( capacity, segment_length, &stack_hash, &stack_index, |hash| fingerprint(hash) as u16, )), - XorFilterType::Xor32 => XorFingerprints::Xor32(assign_fingerprints( + XorFilterType::Xor32 => Fingerprints::Xor32(assign_fingerprints( capacity, segment_length, &stack_hash, @@ -278,57 +272,6 @@ impl XorFilter { }) } - /// Imports a filter from its stored seed and fingerprint slots. - /// - /// Pass the original filter's [`seed`](Self::seed), which can differ from the initial - /// construction seed. Preserve the fingerprint order and query with the original input hashes. - /// - /// # Errors - /// - /// Returns an error unless the payload has three equal segments of `1..=u32::MAX` slots each. - /// - /// # Examples - /// - /// ``` - /// use datasketches::xor::XorFilter; - /// use datasketches::xor::XorFilterType; - /// - /// let filter = XorFilter::from_hashes(&[10, 20, 30], XorFilterType::Xor8, 0)?; - /// let imported = XorFilter::from_fingerprints(filter.seed(), filter.fingerprints().clone())?; - /// assert!(imported.contains_hash(20)); - /// # Ok::<(), datasketches::error::Error>(()) - /// ``` - pub fn from_fingerprints(seed: u64, fingerprints: XorFingerprints) -> Result { - let slots = fingerprints.len(); - if slots == 0 { - return Err(Error::invalid_argument( - "xor filter fingerprint array must not be empty", - )); - } - if slots % 3 != 0 { - return Err(Error::invalid_argument(format!( - "xor filter fingerprint slot count must be a multiple of 3, got {slots}" - ))); - } - let segment_length = slots / 3; - if segment_length > u32::MAX as usize { - return Err(Error::invalid_argument(format!( - "xor filter segment length must be at most {}, got {segment_length}", - u32::MAX - ))); - } - Ok(Self { - segment_length, - seed, - fingerprints, - }) - } - - /// Returns the fingerprint slots for application-defined serialization. - pub fn fingerprints(&self) -> &XorFingerprints { - &self.fingerprints - } - /// Returns `true` if an item is possibly in the input set. /// /// A `false` result means the item was definitely absent; a `true` result may be a false @@ -348,32 +291,26 @@ impl XorFilter { let [h0, h1, h2] = indexes(mixed, self.segment_length); match &self.fingerprints { - XorFingerprints::Xor8(fingerprints) => { + Fingerprints::Xor8(fingerprints) => { fingerprint(mixed) as u8 == fingerprints[h0] ^ fingerprints[h1] ^ fingerprints[h2] } - XorFingerprints::Xor16(fingerprints) => { + Fingerprints::Xor16(fingerprints) => { fingerprint(mixed) as u16 == fingerprints[h0] ^ fingerprints[h1] ^ fingerprints[h2] } - XorFingerprints::Xor32(fingerprints) => { + Fingerprints::Xor32(fingerprints) => { fingerprint(mixed) as u32 == fingerprints[h0] ^ fingerprints[h1] ^ fingerprints[h2] } } } - /// Returns the fingerprint representation. - pub fn filter_type(&self) -> XorFilterType { + fn filter_type(&self) -> XorFilterType { match &self.fingerprints { - XorFingerprints::Xor8(_) => XorFilterType::Xor8, - XorFingerprints::Xor16(_) => XorFilterType::Xor16, - XorFingerprints::Xor32(_) => XorFilterType::Xor32, + Fingerprints::Xor8(_) => XorFilterType::Xor8, + Fingerprints::Xor16(_) => XorFilterType::Xor16, + Fingerprints::Xor32(_) => XorFilterType::Xor32, } } - /// Returns the seed paired with these fingerprints for queries and imports. - pub fn seed(&self) -> u64 { - self.seed - } - /// Returns the estimated in-memory size of the filter in bytes. pub fn estimated_size(&self) -> usize { size_of::() + self.fingerprints.byte_len() @@ -388,8 +325,8 @@ impl XorFilter { /// # Errors /// /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) if the fingerprint - /// slot count exceeds `i32::MAX` or the serialized size overflows `usize`. Construction and - /// imports allow larger capacities than the standard format. + /// slot count exceeds `i32::MAX` or the serialized size overflows `usize`. Construction allows + /// larger capacities than the standard format. pub fn serialize(&self) -> Result, Error> { if self.fingerprints.len() > i32::MAX as usize { return Err(Error::invalid_argument( @@ -409,13 +346,13 @@ impl XorFilter { bytes.write_u64_le(self.seed); match &self.fingerprints { - XorFingerprints::Xor8(fingerprints) => bytes.write(fingerprints), - XorFingerprints::Xor16(fingerprints) => { + Fingerprints::Xor8(fingerprints) => bytes.write(fingerprints), + Fingerprints::Xor16(fingerprints) => { for &fingerprint in fingerprints.iter() { bytes.write_u16_le(fingerprint); } } - XorFingerprints::Xor32(fingerprints) => { + Fingerprints::Xor32(fingerprints) => { for &fingerprint in fingerprints.iter() { bytes.write_u32_le(fingerprint); } @@ -500,13 +437,7 @@ impl XorFilter { } let fingerprints = match filter_type { - XorFilterType::Xor8 => { - let mut values = vec![0_u8; capacity].into_boxed_slice(); - cursor - .read_exact(&mut values) - .map_err(insufficient_data("fingerprints"))?; - XorFingerprints::Xor8(values) - } + XorFilterType::Xor8 => Fingerprints::Xor8(cursor.remaining()[..capacity].into()), XorFilterType::Xor16 => { let mut values = Vec::with_capacity(capacity); for _ in 0..capacity { @@ -516,7 +447,7 @@ impl XorFilter { .map_err(insufficient_data("fingerprints"))?, ); } - XorFingerprints::Xor16(values.into_boxed_slice()) + Fingerprints::Xor16(values.into_boxed_slice()) } XorFilterType::Xor32 => { let mut values = Vec::with_capacity(capacity); @@ -527,7 +458,7 @@ impl XorFilter { .map_err(insufficient_data("fingerprints"))?, ); } - XorFingerprints::Xor32(values.into_boxed_slice()) + Fingerprints::Xor32(values.into_boxed_slice()) } }; diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index b47d6aeb..03dd786b 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -19,12 +19,11 @@ //! //! An XOR filter is built from the complete input set and cannot be updated afterward. Compared //! with a Bloom filter at the same false-positive probability, it generally needs less space and -//! performs exactly three fingerprint reads per query. Construction needs temporary space linear -//! in the number of distinct hashes. +//! performs exactly three fingerprint reads per query. //! -//! [`XorFilterBuilder`] collects values or precomputed hashes and removes duplicates when built. -//! Use [`XorFilter::from_hashes`] when the input hashes are already distinct, or -//! [`XorFilter::from_fingerprints`] to import an existing filter. +//! [`XorFilterBuilder`] buffers one hash per input until [`build`](XorFilterBuilder::build) +//! removes duplicates. Constructing the layout then needs additional space proportional to the +//! number of distinct hashes. Use [`XorFilter::from_hashes`] for already distinct hashes. //! //! # Cross-language hashing //! @@ -43,11 +42,6 @@ //! skips empty strings; callers must do so explicitly here. External hashes (for example XXH3) //! must use the precomputed-hash APIs throughout; Java's typed query API will not match them. //! -//! # Application-defined storage -//! -//! [`XorFilter::from_fingerprints`] restores a filter from its stored [`seed`](XorFilter::seed) -//! and [`fingerprints`](XorFilter::fingerprints). -//! //! # Examples //! //! ``` @@ -71,4 +65,3 @@ mod filter; pub use self::filter::XorFilter; pub use self::filter::XorFilterBuilder; pub use self::filter::XorFilterType; -pub use self::filter::XorFingerprints; diff --git a/tests-integration/Cargo.toml b/tests-integration/Cargo.toml index 78d9eecd..44a802e6 100644 --- a/tests-integration/Cargo.toml +++ b/tests-integration/Cargo.toml @@ -23,7 +23,6 @@ edition.workspace = true rust-version.workspace = true [dev-dependencies] -xorf = { version = "=0.13.0", default-features = false } datasketches = { workspace = true, features = [ "bloom", "countmin", diff --git a/tests-integration/tests/serde_tests/xor.rs b/tests-integration/tests/serde_tests/xor.rs index 2a9aa35a..de8dbe5d 100644 --- a/tests-integration/tests/serde_tests/xor.rs +++ b/tests-integration/tests/serde_tests/xor.rs @@ -19,7 +19,6 @@ use datasketches::error::ErrorKind; use datasketches::xor::XorFilter; use datasketches::xor::XorFilterBuilder; use datasketches::xor::XorFilterType; -use datasketches::xor::XorFingerprints; // Generated from datasketches-java commit c179c5bc00d5aadaed8c68ce11d14d812abe08d6. // XorFilterBuilder.create([1, 2, 3, Long.MIN_VALUE], bits, 42).toByteArray(). @@ -49,7 +48,6 @@ fn java_images_round_trip_byte_for_byte() { let restored = XorFilter::deserialize(&expected).unwrap(); assert_eq!(restored.serialize().unwrap(), expected); assert_eq!(built.serialize().unwrap(), expected); - assert_eq!(restored.filter_type(), ty); for value in values { assert!(built.contains(&value)); assert!( @@ -210,85 +208,9 @@ fn xor32_image_preserves_full_width_little_endian_slots() { let bytes = decode_hex(concat!( "0201162001000000", "efcdab8967452301", - "67452301efcdab89ffffffff", + "67452301efcdab899c5f4186", )); - let filter = XorFilter::from_fingerprints( - 0x0123_4567_89ab_cdef, - XorFingerprints::Xor32(vec![0x0123_4567, 0x89ab_cdef, u32::MAX].into_boxed_slice()), - ) - .unwrap(); + let filter = XorFilter::deserialize(&bytes).unwrap(); + assert!(filter.contains_hash(0)); assert_eq!(filter.serialize().unwrap(), bytes); - assert_eq!(XorFilter::deserialize(&bytes).unwrap(), filter); -} - -#[test] -fn fingerprints_preserve_an_application_storage_layout() { - use datasketches::codec::SketchSlice; - // Synthetic storage fixtures with public xorf 0.13.0 payloads for [0, 1, 2, u64::MAX]. - // This test-only envelope omits cardinality: magic, width:u8, segment:u32, seed:u64, - // and little-endian slots. It is not a production storage format. - let images = [ - concat!( - "54455354080c000000c15c0289ec2d0a91ad000000000000000000ca000000000000000000dc00300000000000000000", - "0000000000", - ), - concat!( - "54455354100c000000c15c0289ec2d0a91aded000000000000000000000000000000000000ca18000000000000000000", - "000000000000000000dcb9000030fc0000000000000000000000000000000000000000000000000000", - ), - concat!( - "54455354200c000000c15c0289ec2d0a91adedfc84000000000000000000000000000000000000000000000000000000", - "000000000000000000ca187b8e0000000000000000000000000000000000000000000000000000000000000000000000", - "00dcb969ee0000000030fcf3490000000000000000000000000000000000000000000000000000000000000000000000", - "0000000000000000000000000000000000", - ), - ]; - for image in images { - let bytes = decode_hex(image); - let mut cursor = SketchSlice::new(&bytes); - assert_eq!(&bytes[..4], b"TEST"); - let mut magic = [0; 4]; - cursor.read_exact(&mut magic).unwrap(); - let width = cursor.read_u8().unwrap(); - let segment = cursor.read_u32_le().unwrap() as usize; - let seed = cursor.read_u64_le().unwrap(); - let slots = cursor.remaining().len() / usize::from(width / 8); - assert_eq!(segment * 3, slots); - let payload = match width { - 8 => XorFingerprints::Xor8(cursor.remaining().into()), - 16 => { - XorFingerprints::Xor16((0..slots).map(|_| cursor.read_u16_le().unwrap()).collect()) - } - 32 => { - XorFingerprints::Xor32((0..slots).map(|_| cursor.read_u32_le().unwrap()).collect()) - } - _ => unreachable!(), - }; - let filter = XorFilter::from_fingerprints(seed, payload).unwrap(); - for hash in [0, 1, 2, u64::MAX] { - assert!(filter.contains_hash(hash)); - } - - let seed = filter.seed(); - let payload = filter.fingerprints(); - let segment = match payload { - XorFingerprints::Xor8(values) => values.len() / 3, - XorFingerprints::Xor16(values) => values.len() / 3, - XorFingerprints::Xor32(values) => values.len() / 3, - }; - let mut encoded = b"TEST".to_vec(); - encoded.push(width); - encoded.extend((segment as u32).to_le_bytes()); - encoded.extend(seed.to_le_bytes()); - match payload { - XorFingerprints::Xor8(values) => encoded.extend_from_slice(values), - XorFingerprints::Xor16(values) => { - encoded.extend(values.iter().flat_map(|v| v.to_le_bytes())) - } - XorFingerprints::Xor32(values) => { - encoded.extend(values.iter().flat_map(|v| v.to_le_bytes())) - } - } - assert_eq!(encoded, bytes); - } } diff --git a/tests-integration/tests/xor_test/filter.rs b/tests-integration/tests/xor_test/filter.rs index 2deec7fb..780ac2d5 100644 --- a/tests-integration/tests/xor_test/filter.rs +++ b/tests-integration/tests/xor_test/filter.rs @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +use datasketches::error::ErrorKind; use datasketches::xor::XorFilter; use datasketches::xor::XorFilterBuilder; use datasketches::xor::XorFilterType; @@ -107,3 +108,49 @@ fn false_positive_rates_follow_fingerprint_width() { ); assert!(false_positives16 < false_positives8); } + +#[test] +fn distinct_construction_is_bounded_and_does_not_deduplicate() { + for kind in [ + XorFilterType::Xor8, + XorFilterType::Xor16, + XorFilterType::Xor32, + ] { + let error = XorFilter::from_hashes(&[1, 2, 1], kind, 0).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + assert!(error.message().contains("duplicate hashes")); + for keys in [vec![], vec![0], vec![u64::MAX], vec![0, u64::MAX]] { + let f = XorFilter::from_hashes(&keys, kind, 0).unwrap(); + for key in keys { + assert!(f.contains_hash(key)); + } + } + } +} + +#[test] +fn construction_recovers_after_an_unpeelable_first_attempt() { + // This distinct set needs two attempts with base seed 1. All fingerprint widths + // share the peeling graph, so each must reset the failed attempt's accumulators. + let keys: Vec<_> = (917_504..917_504 + 16_384u64) + .map(|i| { + let mut z = i.wrapping_add(0x9e3779b97f4a7c15); + z = (z ^ (z >> 30)).wrapping_mul(0xbf58476d1ce4e5b9); + z = (z ^ (z >> 27)).wrapping_mul(0x94d049bb133111eb); + z ^ (z >> 31) + }) + .collect(); + for kind in [ + XorFilterType::Xor8, + XorFilterType::Xor16, + XorFilterType::Xor32, + ] { + let filter = XorFilter::from_hashes(&keys, kind, 1).unwrap(); + for &key in &keys { + assert!(filter.contains_hash(key)); + } + let mut reversed = keys.clone(); + reversed.reverse(); + assert_eq!(filter, XorFilter::from_hashes(&reversed, kind, 1).unwrap()); + } +} diff --git a/tests-integration/tests/xor_test/fingerprints.rs b/tests-integration/tests/xor_test/fingerprints.rs deleted file mode 100644 index b5d000cb..00000000 --- a/tests-integration/tests/xor_test/fingerprints.rs +++ /dev/null @@ -1,163 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -use datasketches::error::ErrorKind; -use datasketches::xor::XorFilter; -use datasketches::xor::XorFilterType; -use datasketches::xor::XorFingerprints; -use xorf::Filter; - -#[test] -fn fingerprints_interoperate_with_xorf_in_both_directions() { - let keys: Vec<_> = (0..16_384u64) - .map(|n| n.wrapping_mul(0x9e3779b97f4a7c15)) - .collect(); - for kind in [ - XorFilterType::Xor8, - XorFilterType::Xor16, - XorFilterType::Xor32, - ] { - let (seed, payload) = match kind { - XorFilterType::Xor8 => { - let f = xorf::Xor8::from(&keys); - (f.seed, XorFingerprints::Xor8(f.fingerprints)) - } - XorFilterType::Xor16 => { - let f = xorf::Xor16::from(&keys); - (f.seed, XorFingerprints::Xor16(f.fingerprints)) - } - XorFilterType::Xor32 => { - let f = xorf::Xor32::from(&keys); - (f.seed, XorFingerprints::Xor32(f.fingerprints)) - } - }; - let imported = XorFilter::from_fingerprints(seed, payload).unwrap(); - assert_eq!( - XorFilter::deserialize(&imported.serialize().unwrap()).unwrap(), - imported - ); - for &key in &keys { - assert!(imported.contains_hash(key)); - } - - let built = XorFilter::from_hashes(&keys, kind, 0).unwrap(); - let seed = built.seed(); - let payload = built.fingerprints().clone(); - // Compare all answers, including false positives, after exporting the fingerprints. - let reference: Box> = match payload { - XorFingerprints::Xor8(fingerprints) => Box::new(xorf::Xor8 { - seed, - block_length: fingerprints.len() / 3, - fingerprints, - }), - XorFingerprints::Xor16(fingerprints) => Box::new(xorf::Xor16 { - seed, - block_length: fingerprints.len() / 3, - fingerprints, - }), - XorFingerprints::Xor32(fingerprints) => Box::new(xorf::Xor32 { - seed, - block_length: fingerprints.len() / 3, - fingerprints, - }), - }; - for key in keys.iter().copied().chain(0..100_000) { - assert_eq!(built.contains_hash(key), reference.contains(&key)); - } - } -} - -#[test] -fn fingerprints_reject_invalid_layouts() { - for (length, reason) in [(0, "empty"), (2, "multiple of 3"), (4, "multiple of 3")] { - let error = XorFilter::from_fingerprints(0, XorFingerprints::Xor8(vec![0; length].into())) - .unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); - assert!(error.message().contains(reason), "{error}"); - } -} - -#[test] -fn distinct_construction_is_bounded_and_does_not_deduplicate() { - for kind in [ - XorFilterType::Xor8, - XorFilterType::Xor16, - XorFilterType::Xor32, - ] { - let error = XorFilter::from_hashes(&[1, 2, 1], kind, 0).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); - assert!(error.message().contains("duplicate hashes")); - for keys in [vec![], vec![0], vec![u64::MAX], vec![0, u64::MAX]] { - let f = XorFilter::from_hashes(&keys, kind, 0).unwrap(); - for key in keys { - assert!(f.contains_hash(key)); - } - } - } -} - -#[test] -fn construction_recovers_after_an_unpeelable_first_attempt() { - // This distinct set needs two attempts with base seed 1. All fingerprint widths - // share the peeling graph, so each must reset the failed attempt's accumulators. - let keys: Vec<_> = (917_504..917_504 + 16_384u64) - .map(|i| { - let mut z = i.wrapping_add(0x9e3779b97f4a7c15); - z = (z ^ (z >> 30)).wrapping_mul(0xbf58476d1ce4e5b9); - z = (z ^ (z >> 27)).wrapping_mul(0x94d049bb133111eb); - z ^ (z >> 31) - }) - .collect(); - let first_seed = XorFilter::from_hashes(&[], XorFilterType::Xor8, 1) - .unwrap() - .seed(); - let reference = xorf::Xor8::from(&keys); - assert_ne!(reference.seed, first_seed); - for kind in [ - XorFilterType::Xor8, - XorFilterType::Xor16, - XorFilterType::Xor32, - ] { - let filter = XorFilter::from_hashes(&keys, kind, 1).unwrap(); - assert_eq!(filter.seed(), reference.seed); - for &key in &keys { - assert!(filter.contains_hash(key)); - } - let mut reversed = keys.clone(); - reversed.reverse(); - assert_eq!(filter, XorFilter::from_hashes(&reversed, kind, 1).unwrap()); - } -} - -#[test] -fn empty_input_uses_fingerprint_queries() { - for kind in [ - XorFilterType::Xor8, - XorFilterType::Xor16, - XorFilterType::Xor32, - ] { - let empty = XorFilter::from_hashes(&[], kind, 0).unwrap(); - let zero_hash = empty.seed().wrapping_neg(); - // This query produces a false positive even though the input set is empty. - assert!(empty.contains_hash(zero_hash)); - let imported = - XorFilter::from_fingerprints(empty.seed(), empty.fingerprints().clone()).unwrap(); - for hash in (0..1_000).chain([zero_hash]) { - assert_eq!(imported.contains_hash(hash), empty.contains_hash(hash)); - } - } -} diff --git a/tests-integration/tests/xor_test/main.rs b/tests-integration/tests/xor_test/main.rs index 34b03598..2b29745d 100644 --- a/tests-integration/tests/xor_test/main.rs +++ b/tests-integration/tests/xor_test/main.rs @@ -16,4 +16,3 @@ // under the License. mod filter; -mod fingerprints; From afe37b85d031b3f3dad53a46f96d3bdad59ec7d6 Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 17:13:07 +0800 Subject: [PATCH 14/16] docs(xor): focus filter documentation on membership --- CHANGELOG.md | 2 +- datasketches/src/xor/filter.rs | 13 ++++--------- datasketches/src/xor/mod.rs | 8 ++------ 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1e0ced3..1960df55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All significant changes to this project will be documented in this file. ### New features * Add `HllUnion::serialize` to serialize union results directly. -* Add immutable `XorFilter` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints. Build from distinct precomputed hashes or restore a filter with `XorFilter::deserialize`. `XorFilterBuilder` accepts values or hashes individually or in batches. Inputs may repeat. Serialization stores no item count; readers must support the encoded fingerprint width. +* Add `XorFilter` and `XorFilterBuilder` behind the `xor` feature for immutable membership queries on values or precomputed hashes. Filters support 8-, 16-, and 32-bit fingerprints and can be serialized for later queries. ### Improvements diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 36b18ef5..fb8e0096 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -99,8 +99,6 @@ impl Fingerprints { /// An [`XorFilter`] builder for values or precomputed hashes. /// -/// Inputs may repeat. Temporary memory use can grow linearly with the total input count. -/// /// Query values with [`XorFilter::contains`] and precomputed hashes with /// [`XorFilter::contains_hash`]. #[derive(Debug)] @@ -145,11 +143,11 @@ impl XorFilterBuilder { self.hashes.extend(hashes); } - /// Consumes the builder and creates a filter from the distinct hashes. + /// Consumes the builder and creates the filter. /// /// # Errors /// - /// Returns an error if the set of distinct hashes is too large or no valid layout is found. + /// Returns an error if the input is too large or no valid layout is found. pub fn build(mut self) -> Result { self.hashes.sort_unstable(); self.hashes.dedup(); @@ -180,11 +178,8 @@ impl XorFilter { /// /// # Errors /// - /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) in these cases: - /// - /// - The input contains duplicate hashes. - /// - The input is too large to build a filter. - /// - Construction cannot find a valid layout. For distinct hashes, try another `seed`. + /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) for invalid or + /// oversized input, or if no valid layout is found. In the latter case, try another `seed`. pub fn from_hashes(keys: &[u64], filter_type: XorFilterType, seed: u64) -> Result { let num_items = keys.len(); let capacity = compute_capacity(num_items)?; diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index 88ac4408..01218a98 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -17,12 +17,8 @@ //! XOR filters for immutable probabilistic set membership. //! -//! An XOR filter is built from the complete input set and cannot be updated afterward. Compared -//! with a Bloom filter at the same false-positive probability, it generally needs less space and -//! performs exactly three fingerprint reads per query. -//! -//! [`XorFilterBuilder`] accepts values or precomputed hashes, including duplicates. -//! Use [`XorFilter::from_hashes`] when the hashes are already distinct. +//! Use [`XorFilterBuilder`] to build a filter from values or precomputed hashes. A negative +//! query result rules out membership. A positive result may be a false positive. //! //! # Cross-language hashing //! From 2d14ff08cc2a6591fe198c9181e7bef07a21123c Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 17:31:30 +0800 Subject: [PATCH 15/16] docs(xor): trim serialization notes --- CONTRIBUTING.md | 2 -- README.md | 2 +- datasketches/src/xor/filter.rs | 7 +------ datasketches/src/xor/mod.rs | 12 +++--------- 4 files changed, 5 insertions(+), 18 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dab6a116..50b1ded8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -192,8 +192,6 @@ cargo x prepare-testdata java If no language is specified, all languages are prepared. These directories are not stored in Git. Run the command before the first test run and again whenever the pinned TCK revision changes. It requires network access and replaces the selected generated directories. -The pinned TCK revision does not yet contain XOR filter snapshots. Until it does, `serde_tests/xor.rs` retains small hexadecimal fixtures generated from the Java implementation, with the source revision and inputs documented alongside them. Keep these separate from the synchronized `*_generated_files` directories; replace them with TCK fixtures when available. - ## Code of Conduct We expect all community members to follow our [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html). diff --git a/README.md b/README.md index f07f058e..0e38d6b5 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ See the [API documentation](https://docs.rs/datasketches) for configuration, acc The minimum supported Rust version is 1.86.0. The crate currently supports little-endian targets only. -Supported serialization formats are tested with fixtures produced by Apache DataSketches Java, C++, and Go through the [DataSketches TCK](https://github.com/apache/datasketches-tck). XOR also uses Java fixtures until TCK snapshots are available. Its standard format supports 8-, 16-, and 32-bit fingerprints; readers must support the encoded width. +Cross-language serialization tests use Java, C++, and Go fixtures from the [DataSketches TCK](https://github.com/apache/datasketches-tck). Serialization compatibility does not imply that an ordinary Rust `Hash` implementation produces the same update bytes across languages. When sketches must represent the same inputs across implementations, use `hash::value::{raw_bytes, canonical_float, sign_extend, natural_extend}` (and the constructors within those modules) to select matching input encodings. When interoperating with an implementation that skips empty strings, skip them before updating here as well. diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index fb8e0096..7c94d146 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -312,11 +312,7 @@ impl XorFilter { size_of::() + self.fingerprints.byte_len() } - /// Serializes the filter using DataSketches family `22`, version `1`. - /// - /// Supports 8-, 16-, and 32-bit fingerprints, recorded in the header's fingerprint width - /// field. Integers and fingerprints are little-endian. Readers must support the encoded - /// fingerprint width; implementations limited to 8/16 bits cannot read 32-bit images. + /// Serializes the filter in the DataSketches format. /// /// # Errors /// @@ -360,7 +356,6 @@ impl XorFilter { /// Deserializes an owned filter from bytes. /// - /// Accepts 8-, 16-, and 32-bit images within the limits of [`serialize`](Self::serialize). /// Trailing bytes are ignored. /// /// # Errors diff --git a/datasketches/src/xor/mod.rs b/datasketches/src/xor/mod.rs index 01218a98..3f146744 100644 --- a/datasketches/src/xor/mod.rs +++ b/datasketches/src/xor/mod.rs @@ -22,15 +22,9 @@ //! //! # Cross-language hashing //! -//! Standard serialization supports 8-, 16-, and 32-bit fingerprints using family `22`, version `1`. -//! Interoperability requires the reader to support the encoded fingerprint width. The format -//! stores the seed, segment layout, and fingerprints, without an item count. -//! Construction may choose a different valid fingerprint array from another implementation; -//! byte-for-byte equality of independently constructed filters is not guaranteed. Rust's -//! [`Hash`](std::hash::Hash) implementations do not always encode values like other languages. Use -//! the strategies in [`crate::hash::value`] when the input hashes must match another DataSketches -//! implementation. A filter built through the precomputed-hash APIs remains portable only when -//! every reader uses the same external hash function. +//! Rust's [`Hash`](std::hash::Hash) may encode a value differently from other languages. Use +//! [`crate::hash::value`] to match the input encoding of another DataSketches implementation. +//! For precomputed hashes, use the same hash function when building and querying the filter. //! //! Java hashes integer scalars as 64-bit little-endian values and strings as UTF-8 bytes with //! xxHash64 seed `0`. Use `u64`/`i64` and [`crate::hash::value::raw_bytes`] accordingly. Java From fd087885cd362b18075f643cea2defb55a6d2edb Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 6 Nov 2026 17:51:53 +0800 Subject: [PATCH 16/16] docs(xor): clarify that from_hashes rejects duplicates --- datasketches/src/xor/filter.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/datasketches/src/xor/filter.rs b/datasketches/src/xor/filter.rs index 7c94d146..3be436f0 100644 --- a/datasketches/src/xor/filter.rs +++ b/datasketches/src/xor/filter.rs @@ -178,8 +178,11 @@ impl XorFilter { /// /// # Errors /// - /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) for invalid or - /// oversized input, or if no valid layout is found. In the latter case, try another `seed`. + /// Returns [`InvalidArgument`](crate::error::ErrorKind::InvalidArgument) in these cases: + /// + /// - The input contains duplicate hashes. + /// - The input is too large to build a filter. + /// - Construction cannot find a valid layout. For distinct hashes, try another `seed`. pub fn from_hashes(keys: &[u64], filter_type: XorFilterType, seed: u64) -> Result { let num_items = keys.len(); let capacity = compute_capacity(num_items)?;