From be79048f2926f3f7cebc0ae6b6d8bdd495368612 Mon Sep 17 00:00:00 2001 From: StevenBtw Date: Sun, 4 Oct 2026 19:56:04 +0200 Subject: [PATCH 1/3] updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d7562ba8..e4d95b3ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to Grafeo, for future reference (and enjoyment). ### Fixed +- **`compact()` turned missing properties into empty values** ([#542](https://github.com/GrafeoDB/grafeo/issues/542)): a node or edge without a property that others with the same label or edge type have read `''`, `0`, `0.0` or `false` for it, `keys()` listed it, `IS NULL` did not match it and filters such as `= 0` did. Missing properties now stay missing, also after a reopen; databases compacted by 0.5.44 or older keep the values they stored. - **Writes during a commit could land in the middle of it** ([#548](https://github.com/GrafeoDB/grafeo/issues/548)): a direct write, or another transaction writing what the committing one wrote, could hide the committed value from point-in-time reads, remove it on rollback or mark an uncommitted value as committed; a transaction that began during a commit could miss it, and a query outside a transaction could see part of it; and after a crash, two commits could be replayed in the wrong order. A commit now completes before anything that comes after it. ## [0.5.44] - 2026-10-04 From c0e4242ffa36fcc6551fe3e6b77dffc3cd1eacdd Mon Sep 17 00:00:00 2001 From: StevenBtw Date: Sun, 4 Oct 2026 20:01:32 +0200 Subject: [PATCH 2/3] compact() keeps missing properties missing (#542) Compact columns record which rows have a value; reads, searches and zone maps skip the others, and the CompactStore section (v4) stores it. v1 to v3 sections still load. --- .../grafeo-core/src/graph/compact/builder.rs | 319 ++++++------------ .../grafeo-core/src/graph/compact/column.rs | 290 +++++++++++++++- .../src/graph/compact/node_table.rs | 21 +- .../src/graph/compact/rel_table.rs | 8 +- .../grafeo-core/src/graph/compact/section.rs | 175 ++++++++-- .../grafeo-core/src/graph/compact/zone_map.rs | 46 ++- .../tests/compact_missing_properties.rs | 145 ++++++++ docs/user-guide/compact-store.md | 9 +- 8 files changed, 747 insertions(+), 266 deletions(-) create mode 100644 crates/grafeo-engine/tests/compact_missing_properties.rs diff --git a/crates/grafeo-core/src/graph/compact/builder.rs b/crates/grafeo-core/src/graph/compact/builder.rs index 1657c14b6..be00796f8 100644 --- a/crates/grafeo-core/src/graph/compact/builder.rs +++ b/crates/grafeo-core/src/graph/compact/builder.rs @@ -10,7 +10,7 @@ use grafeo_common::utils::hash::{FxHashMap, FxHashSet}; use thiserror::Error; use super::CompactStore; -use super::column::ColumnCodec; +use super::column::{ColumnCodec, CompactColumn}; use super::csr::CsrAdjacency; use super::id::MAX_TABLE_ID; use super::node_table::NodeTable; @@ -77,7 +77,7 @@ pub enum CompactStoreError { /// Builder for node table columns. Obtained through [`CompactStoreBuilder::node_table`]. pub struct NodeTableBuilder { label: ArcStr, - columns: Vec<(PropertyKey, ColumnCodec)>, + columns: Vec<(PropertyKey, CompactColumn)>, zone_maps: Vec<(PropertyKey, ZoneMap)>, len: Option, length_mismatch: Option<(usize, usize)>, @@ -117,7 +117,7 @@ impl NodeTableBuilder { self.zone_maps.push((PropertyKey::new(name), zone_map)); self.columns - .push((PropertyKey::new(name), ColumnCodec::BitPacked(bp))); + .push((PropertyKey::new(name), ColumnCodec::BitPacked(bp).into())); self } @@ -136,7 +136,7 @@ impl NodeTableBuilder { self.zone_maps.push((PropertyKey::new(name), zone_map)); self.columns - .push((PropertyKey::new(name), ColumnCodec::Dict(dict))); + .push((PropertyKey::new(name), ColumnCodec::Dict(dict).into())); self } @@ -162,7 +162,7 @@ impl NodeTableBuilder { // No meaningful zone map for vector columns. self.columns.push(( PropertyKey::new(name), - ColumnCodec::int8_vector(data, dimensions), + ColumnCodec::int8_vector(data, dimensions).into(), )); self } @@ -178,14 +178,14 @@ impl NodeTableBuilder { self.zone_maps.push((PropertyKey::new(name), zone_map)); self.columns - .push((PropertyKey::new(name), ColumnCodec::Bitmap(bv))); + .push((PropertyKey::new(name), ColumnCodec::Bitmap(bv).into())); self } /// Adds a pre-built column codec (for advanced use). pub fn column(&mut self, name: &str, codec: ColumnCodec) -> &mut Self { self.record_len(codec.len()); - self.columns.push((PropertyKey::new(name), codec)); + self.columns.push((PropertyKey::new(name), codec.into())); self } @@ -213,7 +213,7 @@ pub struct RelTableBuilder { dst_label: ArcStr, edges: Vec<(u32, u32)>, backward: bool, - properties: Vec<(PropertyKey, ColumnCodec)>, + properties: Vec<(PropertyKey, CompactColumn)>, } impl RelTableBuilder { @@ -248,7 +248,7 @@ impl RelTableBuilder { pub fn column_bitpacked(&mut self, name: &str, values: &[u64], bits: u8) -> &mut Self { let bp = BitPackedInts::pack_with_bits(values, bits); self.properties - .push((PropertyKey::new(name), ColumnCodec::BitPacked(bp))); + .push((PropertyKey::new(name), ColumnCodec::BitPacked(bp).into())); self } } @@ -410,15 +410,15 @@ impl CompactStoreBuilder { let col_defs: Vec = ntb .columns .iter() - .map(|(key, codec)| { - let col_type = infer_column_type(codec); + .map(|(key, column)| { + let col_type = infer_column_type(column.codec()); ColumnDef::new(key.as_str(), col_type) }) .collect(); let schema = TableSchema::new(ntb.label.as_str(), table_id, col_defs); - let columns: FxHashMap = ntb.columns.into_iter().collect(); + let columns: FxHashMap = ntb.columns.into_iter().collect(); let zone_maps: FxHashMap = ntb.zone_maps.into_iter().collect(); @@ -427,7 +427,12 @@ impl CompactStoreBuilder { // when a column is empty; otherwise one entry per block. let block_zone_maps: FxHashMap> = columns .iter() - .map(|(key, codec)| (key.clone(), super::zone_map::compute_block_zone_maps(codec))) + .map(|(key, column)| { + ( + key.clone(), + super::zone_map::compute_block_zone_maps(column), + ) + }) .collect(); let table = NodeTable::from_columns_with_block_stats( @@ -513,8 +518,8 @@ impl CompactStoreBuilder { let property_col_defs: Vec = rtb .properties .iter() - .map(|(key, codec)| { - let col_type = infer_column_type(codec); + .map(|(key, column)| { + let col_type = infer_column_type(column.codec()); ColumnDef::new(key.as_str(), col_type) }) .collect(); @@ -527,7 +532,7 @@ impl CompactStoreBuilder { property_col_defs, ); - let properties: FxHashMap = + let properties: FxHashMap = rtb.properties.into_iter().collect(); let table = RelTable::new(schema, fwd, bwd, properties, src_table_id, dst_table_id); @@ -628,23 +633,6 @@ fn compute_zone_map_u64(values: &[u64]) -> ZoneMap { } } -/// Computes a zone map from signed i64 values (RawI64 column). -/// -/// Produces `Value::Int64` min/max, which flows naturally into `compare_values` -/// and yields correct signed ordering in predicate pushdown. -fn compute_zone_map_i64(values: &[i64]) -> ZoneMap { - let Some(&min) = values.iter().min() else { - return ZoneMap::new(); - }; - let max = *values.iter().max().expect("non-empty after min check"); - ZoneMap { - min: Some(Value::Int64(min)), - max: Some(Value::Int64(max)), - null_count: 0, - row_count: values.len(), - } -} - /// Computes a zone map from string values (dict column). fn compute_zone_map_strings(values: &[&str]) -> ZoneMap { let Some(&min) = values.iter().min() else { @@ -816,101 +804,12 @@ pub fn from_graph_store( // Ensure row count is set even when there are no properties. t.record_len(node_count); for (key, values) in props_map { - let inferred = infer_type_from_values(values); - match inferred { - InferredType::BitPacked => { - let u64_values: Vec = values - .iter() - .map(|v| match v { - // reason: ID encoding: i64 <-> u64 for bit-packed storage - #[allow(clippy::cast_sign_loss)] - Value::Int64(n) => *n as u64, - _ => 0, - }) - .collect(); - let bp = BitPackedInts::pack(&u64_values); - let zone_map = compute_zone_map_u64(&u64_values); - t.zone_maps.push((key.clone(), zone_map)); - t.columns.push((key.clone(), ColumnCodec::BitPacked(bp))); - t.record_len(u64_values.len()); - } - InferredType::RawI64 => { - let i64_values: Vec = values - .iter() - .map(|v| match v { - Value::Int64(n) => *n, - _ => 0, - }) - .collect(); - let zone_map = compute_zone_map_i64(&i64_values); - t.zone_maps.push((key.clone(), zone_map)); - t.columns - .push((key.clone(), ColumnCodec::raw_i64(i64_values))); - t.record_len(values.len()); - } - InferredType::Float64 => { - let f64_values: Vec = values - .iter() - .map(|v| match v { - Value::Float64(f) => *f, - Value::Int64(n) => *n as f64, - _ => 0.0, - }) - .collect(); - t.columns - .push((key.clone(), ColumnCodec::float64(f64_values))); - t.record_len(values.len()); - } - InferredType::Float32Vector { dimensions } => { - let mut flat: Vec = - Vec::with_capacity(values.len() * dimensions as usize); - for v in values { - match v { - Value::Vector(vec) => flat.extend_from_slice(vec), - _ => { - flat.extend(std::iter::repeat_n( - 0.0f32, - usize::from(dimensions), - )); - } - } - } - t.columns - .push((key.clone(), ColumnCodec::float32_vector(flat, dimensions))); - t.record_len(values.len()); - } - InferredType::Bitmap => { - let bool_values: Vec = values - .iter() - .map(|v| matches!(v, Value::Bool(true))) - .collect(); - let bv = BitVector::from_bools(&bool_values); - let zone_map = compute_zone_map_bool(&bool_values); - t.zone_maps.push((key.clone(), zone_map)); - t.columns.push((key.clone(), ColumnCodec::Bitmap(bv))); - t.record_len(bool_values.len()); - } - InferredType::Dict => { - let str_values: Vec = values - .iter() - .map(|v| match v { - Value::Null => String::new(), - Value::String(s) => s.to_string(), - other => format!("{other}"), - }) - .collect(); - let str_refs: Vec<&str> = str_values.iter().map(String::as_str).collect(); - let mut dict_builder = DictionaryBuilder::new(); - for s in &str_refs { - dict_builder.add(s); - } - let dict = dict_builder.build(); - let zone_map = compute_zone_map_strings(&str_refs); - t.zone_maps.push((key.clone(), zone_map)); - t.columns.push((key.clone(), ColumnCodec::Dict(dict))); - t.record_len(str_values.len()); - } + let (column, zone_map) = encode_column(values); + if let Some(zone_map) = zone_map { + t.zone_maps.push((key.clone(), zone_map)); } + t.record_len(column.len()); + t.columns.push((key.clone(), column)); } t }); @@ -984,86 +883,8 @@ pub fn from_graph_store( // Add edge property columns. if let Some(props) = edge_props { for (key, values) in props { - let inferred = infer_type_from_values(values); - match inferred { - InferredType::BitPacked => { - let u64_values: Vec = values - .iter() - .map(|v| match v { - // reason: ID encoding: i64 <-> u64 for bit-packed storage - #[allow(clippy::cast_sign_loss)] - Value::Int64(n) => *n as u64, - _ => 0, - }) - .collect(); - let bp = BitPackedInts::pack(&u64_values); - r.properties.push((key.clone(), ColumnCodec::BitPacked(bp))); - } - InferredType::RawI64 => { - let i64_values: Vec = values - .iter() - .map(|v| match v { - Value::Int64(n) => *n, - _ => 0, - }) - .collect(); - r.properties - .push((key.clone(), ColumnCodec::raw_i64(i64_values))); - } - InferredType::Float64 => { - let f64_values: Vec = values - .iter() - .map(|v| match v { - Value::Float64(f) => *f, - Value::Int64(n) => *n as f64, - _ => 0.0, - }) - .collect(); - r.properties - .push((key.clone(), ColumnCodec::float64(f64_values))); - } - InferredType::Float32Vector { dimensions } => { - let mut flat: Vec = - Vec::with_capacity(values.len() * dimensions as usize); - for v in values { - match v { - Value::Vector(vec) => flat.extend_from_slice(vec), - _ => flat.extend(std::iter::repeat_n( - 0.0f32, - usize::from(dimensions), - )), - } - } - r.properties.push(( - key.clone(), - ColumnCodec::float32_vector(flat, dimensions), - )); - } - InferredType::Bitmap => { - let bool_values: Vec = values - .iter() - .map(|v| matches!(v, Value::Bool(true))) - .collect(); - let bv = BitVector::from_bools(&bool_values); - r.properties.push((key.clone(), ColumnCodec::Bitmap(bv))); - } - InferredType::Dict => { - let str_values: Vec = values - .iter() - .map(|v| match v { - Value::Null => String::new(), - Value::String(s) => s.to_string(), - other => format!("{other}"), - }) - .collect(); - let mut dict_builder = DictionaryBuilder::new(); - for s in &str_values { - dict_builder.add(s); - } - let dict = dict_builder.build(); - r.properties.push((key.clone(), ColumnCodec::Dict(dict))); - } - } + let (column, _) = encode_column(values); + r.properties.push((key.clone(), column)); } } @@ -1224,6 +1045,90 @@ pub fn from_graph_store_preserving_ids( Ok(compact) } +/// Encodes one property column for [`from_graph_store`]: the values in the +/// codec their type calls for, and which rows have a value (a node or edge +/// without the property holds `Value::Null` here, and its row is marked +/// missing instead of holding the codec's empty value). Returns the column +/// and, for the types that keep one, its zone map. +fn encode_column(values: &[Value]) -> (CompactColumn, Option) { + let present: Vec = values.iter().map(|v| !matches!(v, Value::Null)).collect(); + let (codec, zoned) = match infer_type_from_values(values) { + InferredType::BitPacked => { + let u64_values: Vec = values + .iter() + .map(|v| match v { + #[allow( + clippy::cast_sign_loss, + reason = "infer_type_from_values picks BitPacked only when no Int64 is negative" + )] + Value::Int64(n) => *n as u64, + _ => 0, + }) + .collect(); + ( + ColumnCodec::BitPacked(BitPackedInts::pack(&u64_values)), + true, + ) + } + InferredType::RawI64 => { + let i64_values: Vec = values + .iter() + .map(|v| match v { + Value::Int64(n) => *n, + _ => 0, + }) + .collect(); + (ColumnCodec::raw_i64(i64_values), true) + } + InferredType::Float64 => { + let f64_values: Vec = values + .iter() + .map(|v| match v { + Value::Float64(f) => *f, + Value::Int64(n) => *n as f64, + _ => 0.0, + }) + .collect(); + (ColumnCodec::float64(f64_values), false) + } + InferredType::Float32Vector { dimensions } => { + let mut flat: Vec = Vec::with_capacity(values.len() * dimensions as usize); + for v in values { + match v { + Value::Vector(vec) => flat.extend_from_slice(vec), + _ => flat.extend(std::iter::repeat_n(0.0f32, usize::from(dimensions))), + } + } + (ColumnCodec::float32_vector(flat, dimensions), false) + } + InferredType::Bitmap => { + let bool_values: Vec = values + .iter() + .map(|v| matches!(v, Value::Bool(true))) + .collect(); + ( + ColumnCodec::Bitmap(BitVector::from_bools(&bool_values)), + true, + ) + } + InferredType::Dict => { + let mut dict_builder = DictionaryBuilder::new(); + for v in values { + // A missing value takes the empty string; `present` masks it. + match v { + Value::Null => dict_builder.add(""), + Value::String(s) => dict_builder.add(s.as_str()), + other => dict_builder.add(&format!("{other}")), + }; + } + (ColumnCodec::Dict(dict_builder.build()), true) + } + }; + let column = CompactColumn::with_present(codec, BitVector::from_bools(&present)); + let zone_map = zoned.then(|| super::zone_map::compute_zone_map(&column)); + (column, zone_map) +} + /// Infers the columnar encoding type from a slice of [`Value`]s. /// /// Rules: diff --git a/crates/grafeo-core/src/graph/compact/column.rs b/crates/grafeo-core/src/graph/compact/column.rs index 0c20ef2ba..7bfd5dc1c 100644 --- a/crates/grafeo-core/src/graph/compact/column.rs +++ b/crates/grafeo-core/src/graph/compact/column.rs @@ -1078,7 +1078,7 @@ impl ColumnCodec { let stats: &[super::zone_map::ZoneMap] = match stats_hint { Some(hint) if hint.len() == metas.len() => hint, _ => { - computed = super::zone_map::compute_block_zone_maps(self); + computed = super::zone_map::compute_codec_block_zone_maps(self); &computed } }; @@ -1669,6 +1669,211 @@ impl ColumnCodec { } } +// ── Columns with missing values ───────────────────────────────── + +/// A property column of a compact table: the encoded values and which rows +/// have one. +/// +/// A codec holds a value for every row, so a row whose node or edge does not +/// have the property holds the codec's empty value (`0`, `""`, `false`); +/// `present` masks those rows out. It is `None` when every row has a value. +#[derive(Debug, Clone)] +pub struct CompactColumn { + codec: ColumnCodec, + present: Option, +} + +impl From for CompactColumn { + fn from(codec: ColumnCodec) -> Self { + Self::new(codec) + } +} + +impl CompactColumn { + /// A column where every row has a value. + #[must_use] + pub fn new(codec: ColumnCodec) -> Self { + Self { + codec, + present: None, + } + } + + /// A column where only the rows set in `present` (one bit per row) have + /// a value. + #[must_use] + pub fn with_present(codec: ColumnCodec, present: BitVector) -> Self { + debug_assert_eq!(present.len(), codec.len(), "one presence bit per row"); + let present = (present.count_zeros() > 0).then_some(present); + Self { codec, present } + } + + /// The encoded values, including the empty values of missing rows. + #[must_use] + pub fn codec(&self) -> &ColumnCodec { + &self.codec + } + + /// Which rows have a value, or `None` when every row has one. + #[must_use] + pub fn present(&self) -> Option<&BitVector> { + self.present.as_ref() + } + + /// Whether row `index` has a value. + #[inline] + #[must_use] + pub fn has_value(&self, index: usize) -> bool { + self.present + .as_ref() + .is_none_or(|present| present.get(index) == Some(true)) + } + + /// The value at row `index`, or `None` when the row has none or `index` + /// is out of bounds. + #[inline] + #[must_use] + pub fn get(&self, index: usize) -> Option { + if self.has_value(index) { + self.codec.get(index) + } else { + None + } + } + + /// The raw `u64` at row `index` of a bit-packed column (see + /// [`ColumnCodec::get_raw_u64`]), or `None` when the row has no value. + #[inline] + #[must_use] + pub fn get_raw_u64(&self, index: usize) -> Option { + if self.has_value(index) { + self.codec.get_raw_u64(index) + } else { + None + } + } + + /// Number of rows. + #[must_use] + pub fn len(&self) -> usize { + self.codec.len() + } + + /// Returns `true` if the column has no rows. + #[must_use] + pub fn is_empty(&self) -> bool { + self.codec.is_empty() + } + + /// Number of logical blocks (see [`ColumnCodec::block_count`]). + #[must_use] + pub fn block_count(&self) -> usize { + self.codec.block_count() + } + + /// The rows whose value equals `target` (see [`ColumnCodec::find_eq`]); + /// rows without a value never match. + pub fn find_eq(&self, target: &Value) -> Vec { + self.keep_present(self.codec.find_eq(target)) + } + + /// The rows whose value falls in the range (see + /// [`ColumnCodec::find_in_range`]); rows without a value never match. + pub fn find_in_range( + &self, + min: Option<&Value>, + max: Option<&Value>, + min_inclusive: bool, + max_inclusive: bool, + ) -> Vec { + self.keep_present( + self.codec + .find_in_range(min, max, min_inclusive, max_inclusive), + ) + } + + /// Lazy range scan with block skipping (see [`ColumnCodec::range_iter`]); + /// rows without a value never match. + pub fn range_iter<'a>( + &'a self, + block_zone_maps: Option<&'a [super::zone_map::ZoneMap]>, + min: Option<&'a Value>, + max: Option<&'a Value>, + min_inclusive: bool, + max_inclusive: bool, + ) -> Box + 'a> { + let rows = self + .codec + .range_iter(block_zone_maps, min, max, min_inclusive, max_inclusive); + match &self.present { + None => rows, + Some(_) => Box::new(rows.filter(move |&row| self.has_value(row))), + } + } + + fn keep_present(&self, mut rows: Vec) -> Vec { + if let Some(present) = &self.present { + rows.retain(|&row| present.get(row) == Some(true)); + } + rows + } + + /// Returns an estimate of heap memory used by this column in bytes. + #[must_use] + pub fn heap_bytes(&self) -> usize { + self.codec.heap_bytes() + self.present.as_ref().map_or(0, |p| p.data_bytes().len()) + } + + /// Writes which rows have a value (section format v4): a flag byte, then, + /// when some row has none, the bitmap as a length and LE `u64` words. + /// + /// # Errors + /// + /// Fails if the bitmap does not fit the format's `u32` fields. + pub(crate) fn write_present( + &self, + buf: &mut Vec, + ) -> grafeo_common::utils::error::Result<()> { + match &self.present { + None => buf.push(0), + Some(present) => { + buf.push(1); + write_usize_as_u32(buf, present.len())?; + write_usize_as_u32(buf, present.word_count())?; + buf.extend_from_slice(present.data_bytes()); + } + } + Ok(()) + } + + /// Reads what [`write_present`](Self::write_present) wrote. + pub(crate) fn read_present( + data: &Bytes, + pos: &mut usize, + ) -> Result, &'static str> { + let bytes: &[u8] = data.as_ref(); + let flag = *bytes.get(*pos).ok_or("truncated column presence flag")?; + *pos += 1; + match flag { + 0 => Ok(None), + 1 => { + let bit_len = read_u32_le(bytes, pos)? as usize; + let word_count = read_u32_le(bytes, pos)? as usize; + let need = word_count + .checked_mul(8) + .ok_or("column presence word count overflow")?; + if *pos + need > bytes.len() { + return Err("truncated column presence bitmap"); + } + let storage = data.slice(*pos..*pos + need); + *pos += need; + Ok(Some(BitVector::from_bytes_storage(storage, bit_len))) + } + _ => Err("invalid column presence flag"), + } + } +} + // ── v2 block-index helpers ────────────────────────────────────── /// Per-block metadata in the v2 column index. @@ -3689,7 +3894,7 @@ mod tests { // the existing eager `find_in_range`, plus pruning behavior on // multi-block columns. - use crate::graph::compact::zone_map::compute_block_zone_maps; + use crate::graph::compact::zone_map::compute_codec_block_zone_maps; fn raw_i64_seq(n: i64) -> ColumnCodec { ColumnCodec::raw_i64((0..n).collect()) @@ -3698,7 +3903,7 @@ mod tests { #[test] fn alix_range_iter_matches_find_in_range_full_scan() { let col = raw_i64_seq(50); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Int64(10); let max = Value::Int64(20); @@ -3719,7 +3924,7 @@ mod tests { // Query [1500, 1700] hits block 1 only; blocks 0 and 2 must be // skipped (their zone maps prove disjointedness). let col = raw_i64_seq(3072); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Int64(1500); let max = Value::Int64(1700); @@ -3734,7 +3939,7 @@ mod tests { #[test] fn vincent_range_iter_open_min_bound() { let col = raw_i64_seq(50); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let max = Value::Int64(10); let result: Vec = col .range_iter(Some(&zm), None, Some(&max), false, true) @@ -3746,7 +3951,7 @@ mod tests { #[test] fn jules_range_iter_open_max_bound() { let col = raw_i64_seq(20); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Int64(15); let result: Vec = col .range_iter(Some(&zm), Some(&min), None, true, false) @@ -3772,7 +3977,7 @@ mod tests { #[test] fn butch_range_iter_empty_column_yields_nothing() { let col = raw_i64_seq(0); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Int64(0); let result: Vec = col .range_iter(Some(&zm), Some(&min), None, true, false) @@ -3787,7 +3992,7 @@ mod tests { b.add(s); } let col = ColumnCodec::Dict(b.build()); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::from("b"); let max = Value::from("c"); let from_iter: Vec = col @@ -3802,7 +4007,7 @@ mod tests { // BitPacked stores u64; a negative min bound must not crash and // must match `find_in_range` semantics. let col = ColumnCodec::BitPacked(BitPackedInts::pack(&(0..20u64).collect::>())); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Int64(-5); let max = Value::Int64(10); let from_iter: Vec = col @@ -3815,7 +4020,7 @@ mod tests { #[test] fn beatrix_range_iter_exclusive_bounds() { let col = raw_i64_seq(50); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Int64(10); let max = Value::Int64(20); let result: Vec = col @@ -3830,7 +4035,7 @@ mod tests { // NaN in a Float64 column is not orderable and must never appear // in any range query result. let col = ColumnCodec::float64(vec![1.0, f64::NAN, 2.0, 3.0]); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Float64(0.5); let max = Value::Float64(4.0); let from_iter: Vec = col @@ -3849,7 +4054,7 @@ mod tests { // Iterator order matters for downstream chunking; rows must be // emitted in increasing offset order. let col = raw_i64_seq(2048); - let zm = compute_block_zone_maps(&col); + let zm = compute_codec_block_zone_maps(&col); let min = Value::Int64(500); let max = Value::Int64(1500); let result: Vec = col @@ -3859,4 +4064,65 @@ mod tests { sorted.sort_unstable(); assert_eq!(result, sorted, "iterator output must be sorted ascending"); } + + /// A bit-packed column whose second row has no value; its stored empty + /// value (0) must not show through. + fn column_with_a_missing_row() -> CompactColumn { + let codec = ColumnCodec::BitPacked(BitPackedInts::pack(&[19, 0, 3, 0])); + CompactColumn::with_present(codec, BitVector::from_bools(&[true, false, true, true])) + } + + /// A row without a value reads as missing and never matches a search, + /// also not for the codec's empty value; a stored 0 still does. + #[test] + fn compact_column_masks_rows_without_a_value() { + let column = column_with_a_missing_row(); + assert_eq!(column.get(1), None); + assert_eq!(column.get_raw_u64(1), None); + assert_eq!(column.get(3), Some(Value::Int64(0))); + assert_eq!(column.find_eq(&Value::Int64(0)), vec![3]); + let three = Value::Int64(3); + assert_eq!( + column.find_in_range(None, Some(&three), true, true), + vec![2, 3] + ); + assert_eq!( + column + .range_iter(None, None, Some(&three), true, true) + .collect::>(), + vec![2, 3] + ); + } + + /// A column where every row has a value keeps no bitmap and reads like + /// its codec. + #[test] + fn compact_column_with_every_row_present_keeps_no_bitmap() { + let codec = ColumnCodec::BitPacked(BitPackedInts::pack(&[19, 0])); + let column = CompactColumn::with_present(codec, BitVector::from_bools(&[true, true])); + assert!(column.present().is_none()); + assert_eq!(column.get(1), Some(Value::Int64(0))); + assert_eq!(column.find_eq(&Value::Int64(0)), vec![1]); + } + + /// The whole-column zone map counts a missing row as a null and leaves + /// its empty value out of min and max. + #[test] + fn compact_column_zone_map_skips_missing_rows() { + let zm = crate::graph::compact::zone_map::compute_zone_map(&column_with_a_missing_row()); + assert_eq!(zm.min, Some(Value::Int64(0))); + assert_eq!(zm.max, Some(Value::Int64(19))); + assert_eq!(zm.null_count, 1); + assert_eq!(zm.row_count, 4); + + let codec = ColumnCodec::BitPacked(BitPackedInts::pack(&[19, 0, 3])); + let column = + CompactColumn::with_present(codec, BitVector::from_bools(&[true, false, true])); + let zm = crate::graph::compact::zone_map::compute_zone_map(&column); + assert_eq!( + zm.min, + Some(Value::Int64(3)), + "the missing row's 0 is not a minimum" + ); + } } diff --git a/crates/grafeo-core/src/graph/compact/node_table.rs b/crates/grafeo-core/src/graph/compact/node_table.rs index cfc2c881d..94a6df269 100644 --- a/crates/grafeo-core/src/graph/compact/node_table.rs +++ b/crates/grafeo-core/src/graph/compact/node_table.rs @@ -6,7 +6,7 @@ use grafeo_common::types::{NodeId, PropertyKey, Value}; use grafeo_common::utils::hash::FxHashMap; -use super::column::ColumnCodec; +use super::column::CompactColumn; use super::id::encode_node_id; use super::schema::TableSchema; use super::zone_map::ZoneMap; @@ -14,14 +14,14 @@ use super::zone_map::ZoneMap; /// Per-label columnar storage for nodes. /// /// All nodes sharing a label are stored in a single `NodeTable` with one -/// [`ColumnCodec`] per property. Row offsets are combined with the table ID +/// [`CompactColumn`] per property. Row offsets are combined with the table ID /// via [`encode_node_id`] to produce globally unique [`NodeId`] values. #[derive(Debug)] pub struct NodeTable { /// Schema describing the label, table ID, and column definitions. schema: TableSchema, /// Columns keyed by property name. - columns: FxHashMap, + columns: FxHashMap, /// Per-column min/max statistics for predicate pushdown. zone_maps: FxHashMap, /// Per-block min/max statistics, one entry per logical block in each @@ -51,7 +51,7 @@ impl NodeTable { #[must_use] pub fn from_columns( schema: TableSchema, - columns: FxHashMap, + columns: FxHashMap, zone_maps: FxHashMap, len: usize, ) -> Self { @@ -63,7 +63,7 @@ impl NodeTable { #[must_use] pub fn from_columns_with_block_stats( schema: TableSchema, - columns: FxHashMap, + columns: FxHashMap, zone_maps: FxHashMap, block_zone_maps: FxHashMap>, len: usize, @@ -141,7 +141,7 @@ impl NodeTable { /// /// This is primarily useful for foreign-key columns where the raw encoded ID /// is needed rather than the `Value::Int64` conversion. Returns `None` for - /// non-[`BitPacked`](ColumnCodec::BitPacked) columns or out-of-bounds offsets. + /// non-[`BitPacked`](super::column::ColumnCodec::BitPacked) columns or out-of-bounds offsets. #[must_use] pub fn get_raw_u64(&self, offset: usize, key: &PropertyKey) -> Option { self.columns.get(key)?.get_raw_u64(offset) @@ -155,7 +155,7 @@ impl NodeTable { /// Returns the column codec for a property, if it exists. #[must_use] - pub fn column(&self, key: &PropertyKey) -> Option<&ColumnCodec> { + pub fn column(&self, key: &PropertyKey) -> Option<&CompactColumn> { self.columns.get(key) } @@ -167,7 +167,7 @@ impl NodeTable { /// Returns all columns (for serialization). #[must_use] - pub fn columns(&self) -> &FxHashMap { + pub fn columns(&self) -> &FxHashMap { &self.columns } @@ -203,6 +203,7 @@ impl NodeTable { mod tests { use super::*; use crate::codec::BitPackedInts; + use crate::graph::compact::column::ColumnCodec; use crate::graph::compact::id::decode_node_id; use crate::graph::compact::schema::{ColumnDef, ColumnType}; @@ -224,11 +225,11 @@ mod tests { let mut columns = FxHashMap::default(); columns.insert( PropertyKey::new("rating"), - ColumnCodec::BitPacked(BitPackedInts::pack(&ratings)), + ColumnCodec::BitPacked(BitPackedInts::pack(&ratings)).into(), ); columns.insert( PropertyKey::new("count"), - ColumnCodec::BitPacked(BitPackedInts::pack(&counts)), + ColumnCodec::BitPacked(BitPackedInts::pack(&counts)).into(), ); let mut zone_maps = FxHashMap::default(); diff --git a/crates/grafeo-core/src/graph/compact/rel_table.rs b/crates/grafeo-core/src/graph/compact/rel_table.rs index b4a98041d..7046e73f1 100644 --- a/crates/grafeo-core/src/graph/compact/rel_table.rs +++ b/crates/grafeo-core/src/graph/compact/rel_table.rs @@ -7,7 +7,7 @@ use arcstr::ArcStr; use grafeo_common::types::{EdgeId, NodeId, PropertyKey, Value}; use grafeo_common::utils::hash::FxHashMap; -use super::column::ColumnCodec; +use super::column::CompactColumn; use super::csr::CsrAdjacency; use super::id::{encode_edge_id, encode_node_id}; use super::schema::EdgeSchema; @@ -30,7 +30,7 @@ pub struct RelTable { /// position for each backward edge. bwd: Option, /// Edge properties, keyed by property name, parallel to forward CSR targets. - properties: FxHashMap, + properties: FxHashMap, /// Table ID of the source node table. src_table_id: u16, /// Table ID of the destination node table. @@ -48,7 +48,7 @@ impl RelTable { schema: EdgeSchema, fwd: CsrAdjacency, bwd: Option, - properties: FxHashMap, + properties: FxHashMap, src_table_id: u16, dst_table_id: u16, ) -> Self { @@ -220,7 +220,7 @@ impl RelTable { /// Returns edge property columns (for serialization). #[must_use] - pub fn properties(&self) -> &FxHashMap { + pub fn properties(&self) -> &FxHashMap { &self.properties } diff --git a/crates/grafeo-core/src/graph/compact/section.rs b/crates/grafeo-core/src/graph/compact/section.rs index 88fbb48a1..3047310dd 100644 --- a/crates/grafeo-core/src/graph/compact/section.rs +++ b/crates/grafeo-core/src/graph/compact/section.rs @@ -13,7 +13,7 @@ use grafeo_common::utils::hash::FxHashMap; use parking_lot::RwLock; use super::CompactStore; -use super::column::ColumnCodec; +use super::column::{ColumnCodec, CompactColumn}; use super::csr::CsrAdjacency; use super::node_table::NodeTable; use super::rel_table::RelTable; @@ -25,9 +25,15 @@ use crate::statistics::{EdgeTypeStatistics, LabelStatistics, Statistics}; /// Magic bytes identifying a CompactStore section. const MAGIC: [u8; 4] = *b"GCST"; -/// Current section format version. Phase 2c bumped this from 2 to 3 to -/// embed per-block zone maps in the column index for skip pruning. -const FORMAT_VERSION: u8 = 3; +/// Current section format version: v3 plus, after each column body, which +/// rows have a value (#542). Phase 2c bumped 2 to 3 to embed per-block zone +/// maps in the column index for skip pruning. +const FORMAT_VERSION: u8 = 4; + +/// v3 layout: per-block index with per-block stats, every row has a value. +/// Retained as a read-only compat path; files written by 0.5.42 to 0.5.44 +/// carry this byte (their missing properties already hold empty values). +const FORMAT_VERSION_V3: u8 = 3; /// v2 (Phase 2b) layout: per-block index + bodies, no per-block stats. /// Retained as a read-only compat path for one release. @@ -139,7 +145,7 @@ impl CompactStoreSection { } else { buf.push(0); } - write_codec( + write_column( codec, &mut buf, version, @@ -165,9 +171,14 @@ impl CompactStoreSection { write_len(&mut buf, properties.len())?; for (key, codec) in properties { write_str(&mut buf, key.as_str())?; - // Edge property columns don't track per-block zone maps - // yet; v3 will compute them inline during write. - write_codec(codec, &mut buf, version, None)?; + // Edge property columns don't keep per-block zone maps; v3+ + // computes them during write, from the column when some rows + // have no value (the codec alone would count their empty + // values). + let block_stats = codec + .present() + .map(|_| super::zone_map::compute_block_zone_maps(codec)); + write_column(codec, &mut buf, version, block_stats.as_deref())?; } } // Continue building buf in `serialize()` epilogue. @@ -231,6 +242,21 @@ fn write_codec( } } +/// Writes a column: its codec body (see [`write_codec`]) and, from v4, which +/// rows have a value. +fn write_column( + column: &CompactColumn, + buf: &mut Vec, + version: u8, + block_stats_hint: Option<&[ZoneMap]>, +) -> grafeo_common::utils::error::Result<()> { + write_codec(column.codec(), buf, version, block_stats_hint)?; + if version >= FORMAT_VERSION { + column.write_present(buf)?; + } + Ok(()) +} + impl Section for CompactStoreSection { fn section_type(&self) -> SectionType { SectionType::CompactStore @@ -287,13 +313,42 @@ fn read_codec( FORMAT_VERSION_V2 => ColumnCodec::read_from_v2(data, pos) .map(|c| (c, None)) .map_err(|e| e.to_string()), - FORMAT_VERSION => ColumnCodec::read_from_v3(data, pos) + FORMAT_VERSION_V3 | FORMAT_VERSION => ColumnCodec::read_from_v3(data, pos) .map(|(c, stats)| (c, Some(stats))) .map_err(|e| e.to_string()), _ => Err(format!("unsupported CompactStore version {version}")), } } +/// Reads a column: its codec body (see [`read_codec`]) and, from v4, which +/// rows have a value. Before v4 every row has one. +fn read_column( + data: &Bytes, + pos: &mut usize, + version: u8, +) -> Result<(CompactColumn, Option>), String> { + let (codec, block_stats) = read_codec(data, pos, version)?; + let present = if version >= FORMAT_VERSION { + CompactColumn::read_present(data, pos).map_err(str::to_string)? + } else { + None + }; + let column = match present { + Some(present) if present.len() == codec.len() => { + CompactColumn::with_present(codec, present) + } + Some(present) => { + return Err(format!( + "column presence has {} rows, the column {}", + present.len(), + codec.len() + )); + } + None => CompactColumn::new(codec), + }; + Ok((column, block_stats)) +} + fn deserialize_compact_store(data_bytes: &bytes::Bytes) -> Result { let data: &[u8] = data_bytes.as_ref(); if data.len() < 10 { @@ -324,9 +379,16 @@ fn deserialize_compact_store(data_bytes: &bytes::Bytes) -> Result Result = FxHashMap::default(); + let mut columns: FxHashMap = FxHashMap::default(); let mut zone_maps: FxHashMap = FxHashMap::default(); let mut block_zone_maps: FxHashMap> = FxHashMap::default(); let mut col_defs = Vec::with_capacity(num_cols); @@ -362,14 +424,14 @@ fn deserialize_compact_store(data_bytes: &bytes::Bytes) -> Result Result = FxHashMap::default(); + let mut properties: FxHashMap = FxHashMap::default(); let mut prop_defs = Vec::with_capacity(num_props); for _ in 0..num_props { let key_str = read_string(data, &mut pos)?; let key = PropertyKey::new(&key_str); - let (codec, _block_stats) = read_codec(data_bytes, &mut pos, version) + let (column, _block_stats) = read_column(data_bytes, &mut pos, version) .map_err(|e| format!("edge codec: {e}"))?; - let col_type = infer_column_type_from_codec(&codec); + let col_type = infer_column_type_from_codec(column.codec()); prop_defs.push(ColumnDef::new(&key_str, col_type)); - properties.insert(key, codec); + properties.insert(key, column); } let src_label = table_id_to_label @@ -1149,4 +1211,77 @@ mod tests { Some(&Value::Int64(5)) ); } + + /// A store where one `:P` node and one `:R` edge lack properties that + /// the others have. Returns the store and the ids of the sparse node and + /// edge. + fn store_with_missing_properties() -> (LpgStore, NodeId, EdgeId) { + let store = LpgStore::new().unwrap(); + let full = store.create_node(&["P"]); + let sparse = store.create_node(&["P"]); + store.set_node_property(full, "n", Value::Int64(1)); + store.set_node_property(full, "u", Value::Int64(19)); + store.set_node_property(full, "s", Value::from("Amsterdam")); + store.set_node_property(full, "b", Value::Bool(true)); + store.set_node_property(sparse, "n", Value::Int64(2)); + let full_edge = store.create_edge(full, sparse, "R"); + store.set_edge_property(full_edge, "n", Value::Int64(1)); + store.set_edge_property(full_edge, "w", Value::Int64(3)); + let sparse_edge = store.create_edge(sparse, full, "R"); + store.set_edge_property(sparse_edge, "n", Value::Int64(2)); + (store, sparse, sparse_edge) + } + + /// The current format keeps which rows have a value: after a round trip + /// the sparse node and edge still have no value for what they lack. + #[test] + fn missing_values_survive_a_round_trip() { + let (store, sparse, sparse_edge) = store_with_missing_properties(); + let compact = from_graph_store_preserving_ids(&store).unwrap(); + let bytes = CompactStoreSection::new(Arc::new(compact)) + .serialize() + .unwrap(); + let mut section = CompactStoreSection::empty(); + section.deserialize(&bytes).unwrap(); + let restored = section.store().unwrap(); + + for key in ["u", "s", "b"] { + assert_eq!( + restored.get_node_property(sparse, &PropertyKey::new(key)), + None, + "{key}" + ); + } + assert_eq!( + restored.get_node_property(sparse, &PropertyKey::new("n")), + Some(Value::Int64(2)) + ); + assert_eq!( + restored.get_edge_property(sparse_edge, &PropertyKey::new("w")), + None + ); + } + + /// A v3 section (0.5.44 and older) still loads. It records no missing + /// values, so a missing property reads the empty value stored for it. + #[test] + fn a_v3_section_still_loads() { + let (store, sparse, _) = store_with_missing_properties(); + let compact = from_graph_store_preserving_ids(&store).unwrap(); + let bytes = CompactStoreSection::new(Arc::new(compact)) + .serialize_with_version(FORMAT_VERSION_V3) + .unwrap(); + let mut section = CompactStoreSection::empty(); + section.deserialize(&bytes).unwrap(); + let restored = section.store().unwrap(); + + assert_eq!( + restored.get_node_property(sparse, &PropertyKey::new("n")), + Some(Value::Int64(2)) + ); + assert_eq!( + restored.get_node_property(sparse, &PropertyKey::new("u")), + Some(Value::Int64(0)) + ); + } } diff --git a/crates/grafeo-core/src/graph/compact/zone_map.rs b/crates/grafeo-core/src/graph/compact/zone_map.rs index 34d77edf7..aa784dd9a 100644 --- a/crates/grafeo-core/src/graph/compact/zone_map.rs +++ b/crates/grafeo-core/src/graph/compact/zone_map.rs @@ -189,29 +189,59 @@ fn read_inline_u32(data: &[u8], pos: &mut usize) -> Result { Ok(u32::from_le_bytes(bytes)) } -/// Computes per-block zone maps for a column codec. +/// Computes the zone map of a whole column: min and max over the rows that +/// have a value, the other rows counted as nulls. +#[must_use] +pub fn compute_zone_map(column: &super::column::CompactColumn) -> ZoneMap { + let mut zm = ZoneMap { + row_count: column.len(), + ..ZoneMap::default() + }; + for row in 0..column.len() { + match column.get(row) { + Some(value) => update_block_min_max(&mut zm, &value), + None => zm.null_count += 1, + } + } + zm +} + +/// Computes per-block zone maps for a column. /// /// Walks each block's row range, tracking min/max/null_count for the -/// orderable scalar types (`Int64`, `Float64`, `String`, `Bool`). -/// Vector and list columns produce zone maps with `None` min/max but -/// still track row and null counts. +/// orderable scalar types (`Int64`, `Float64`, `String`, `Bool`). A row +/// without a value counts as a null. Vector and list columns produce zone +/// maps with `None` min/max but still track row and null counts. /// /// Empty columns produce a single zero-row block (matching /// [`ColumnCodec::block_count`](super::column::ColumnCodec::block_count)). #[must_use] -pub fn compute_block_zone_maps(codec: &super::column::ColumnCodec) -> Vec { - let block_count = codec.block_count(); +pub fn compute_block_zone_maps(column: &super::column::CompactColumn) -> Vec { + block_zone_maps(column.block_count(), column.len(), |row| column.get(row)) +} + +/// [`compute_block_zone_maps`] for a bare codec, where every row has a value. +#[must_use] +pub(super) fn compute_codec_block_zone_maps(codec: &super::column::ColumnCodec) -> Vec { + block_zone_maps(codec.block_count(), codec.len(), |row| codec.get(row)) +} + +fn block_zone_maps( + block_count: usize, + len: usize, + get: impl Fn(usize) -> Option, +) -> Vec { let block_rows = crate::codec::DEFAULT_BLOCK_ROWS as usize; let mut result = Vec::with_capacity(block_count); for i in 0..block_count { let start = i * block_rows; - let end = (start + block_rows).min(codec.len()); + let end = (start + block_rows).min(len); let mut zm = ZoneMap { row_count: end - start, ..ZoneMap::default() }; for j in start..end { - match codec.get(j) { + match get(j) { Some(value) => update_block_min_max(&mut zm, &value), None => zm.null_count += 1, } diff --git a/crates/grafeo-engine/tests/compact_missing_properties.rs b/crates/grafeo-engine/tests/compact_missing_properties.rs new file mode 100644 index 000000000..230a05b8f --- /dev/null +++ b/crates/grafeo-engine/tests/compact_missing_properties.rs @@ -0,0 +1,145 @@ +//! After `compact()`, a property that a node or edge does not have stays +//! missing (#542): it reads as null, `keys()` leaves it out, `IS NULL` +//! matches it, and no filter matches it through the column's empty value +//! (`''`, `0`, `0.0`, `false`). + +#![cfg(feature = "compact-store")] + +use grafeo_common::types::Value; +use grafeo_engine::GrafeoDB; + +/// Two `:P` nodes and two `:R` edges of the same label and type: the first +/// of each has a value in every column type, the second only `n`. +fn populate(db: &GrafeoDB) { + db.execute( + "INSERT (:P {n: 1, s: 'Amsterdam', u: 19, i: -3, f: 1.5, b: true, v: vector([3.0, 19.0])}), \ + (:P {n: 2})", + ) + .unwrap(); + db.execute( + "MATCH (a:P {n: 1}), (b:P {n: 2}) \ + INSERT (a)-[:R {n: 1, s: 'Berlin', u: 19, i: -3, f: 2.5, b: true}]->(b), \ + (b)-[:R {n: 2}]->(a)", + ) + .unwrap(); +} + +fn rows(db: &GrafeoDB, query: &str) -> Vec> { + db.execute(query).unwrap().rows().to_vec() +} + +fn keys(names: &[&str]) -> Value { + Value::List( + names + .iter() + .map(|n| Value::from(*n)) + .collect::>() + .into(), + ) +} + +/// Checks every way a missing property could show up as a value. +fn assert_missing_stays_missing(db: &GrafeoDB, when: &str) { + assert_eq!( + rows( + db, + "MATCH (p:P {n: 2}) RETURN p.s, p.u, p.i, p.f, p.b, p.v, keys(p)" + ), + [vec![ + Value::Null, + Value::Null, + Value::Null, + Value::Null, + Value::Null, + Value::Null, + keys(&["n"]) + ]], + "{when}: node values" + ); + assert_eq!( + rows( + db, + "MATCH ()-[r:R {n: 2}]->() RETURN r.s, r.u, r.i, r.f, r.b, keys(r)" + ), + [vec![ + Value::Null, + Value::Null, + Value::Null, + Value::Null, + Value::Null, + keys(&["n"]) + ]], + "{when}: edge values" + ); + for column in ["s", "u", "i", "f", "b", "v"] { + assert_eq!( + rows( + db, + &format!("MATCH (p:P) WHERE p.{column} IS NULL RETURN p.n") + ), + [vec![Value::Int64(2)]], + "{when}: p.{column} IS NULL" + ); + } + for (filter, expected) in [ + ("p.s = ''", 0), + ("p.u = 0", 0), + ("p.u < 1", 0), + ("p.i = 0", 0), + ("p.i < 0", 1), + ("p.f = 0.0", 0), + ("p.b = false", 0), + ] { + assert_eq!( + rows(db, &format!("MATCH (p:P) WHERE {filter} RETURN count(p)")), + [vec![Value::Int64(expected)]], + "{when}: {filter}" + ); + } + for (filter, expected) in [("r.u = 0", 0), ("r.b = false", 0), ("r.s = ''", 0)] { + assert_eq!( + rows( + db, + &format!("MATCH ()-[r:R]->() WHERE {filter} RETURN count(r)") + ), + [vec![Value::Int64(expected)]], + "{when}: {filter}" + ); + } + // The values that are there are untouched. + assert_eq!( + rows(db, "MATCH (p:P {n: 1}) RETURN p.s, p.u, p.i, p.b"), + [vec![ + Value::from("Amsterdam"), + Value::Int64(19), + Value::Int64(-3), + Value::Bool(true) + ]], + "{when}: present values" + ); +} + +#[test] +fn missing_properties_stay_missing_after_compact() { + let mut db = GrafeoDB::new_in_memory(); + populate(&db); + db.compact().unwrap(); + assert_missing_stays_missing(&db, "after compact()"); + db.compact().unwrap(); + assert_missing_stays_missing(&db, "after a second compact()"); +} + +#[cfg(feature = "grafeo-file")] +#[test] +fn missing_properties_stay_missing_after_a_reopen() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("compact.grafeo"); + { + let mut db = GrafeoDB::open(&path).unwrap(); + populate(&db); + db.compact().unwrap(); + db.close().unwrap(); + } + let db = GrafeoDB::open(&path).unwrap(); + assert_missing_stays_missing(&db, "after a reopen"); +} diff --git a/docs/user-guide/compact-store.md b/docs/user-guide/compact-store.md index bcd3171a7..96b72066e 100644 --- a/docs/user-guide/compact-store.md +++ b/docs/user-guide/compact-store.md @@ -203,11 +203,10 @@ stores: vector/text scan and search now fall through both layers. - **Alternative:** assign a canonical "primary" label and store additional labels as a list property instead. - **No disk serialization**: `compact()` operates in memory. To persist a compacted database, use snapshot export (WASM) or save before compacting. -- **Missing properties read as empty values**: after `compact()`, when other entities with the - same label or type have a property that a node or edge lacks, reading it on that node or edge - gives the column's empty value (`''`, `0`, `0.0` or `false`) instead of null, `keys()` lists - it and `IS NULL` does not match it. Give every entity the property before compacting, or avoid relying on its absence - ([#542](https://github.com/GrafeoDB/grafeo/issues/542), planned for 0.6.0). +- **Databases compacted by 0.5.44 or older**: those versions stored a property that a node or + edge lacked as the column's empty value (`''`, `0`, `0.0` or `false`), and such files keep those + values. Since 0.6.0 a missing property stays missing after `compact()` + ([#542](https://github.com/GrafeoDB/grafeo/issues/542)). ## Feature Flag From 5b22203614cbaff01dc7d278b0606e01199f56b5 Mon Sep 17 00:00:00 2001 From: StevenBtw Date: Sun, 4 Oct 2026 20:08:05 +0200 Subject: [PATCH 3/3] review comments from #553 The plain-read commit test waits for its read to finish instead of 300 ms, an assert in a loop names its case, and the pip pinning note gives the exact range. --- .../grafeo-engine/tests/commit_completion.rs | 31 +++++++++++++++---- crates/grafeo-engine/tests/direct_api.rs | 2 +- docs/versioning.md | 2 +- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/grafeo-engine/tests/commit_completion.rs b/crates/grafeo-engine/tests/commit_completion.rs index cddb4ae9b..4bb114724 100644 --- a/crates/grafeo-engine/tests/commit_completion.rs +++ b/crates/grafeo-engine/tests/commit_completion.rs @@ -48,11 +48,22 @@ enum Moment { Stamped, } +/// How long the commit waits for the work it started. +#[derive(Clone, Copy)] +enum Wait { + /// Until the work is done, for work that never waits for the commit: it + /// always runs in the middle of the commit. + UntilDone, + /// 300 ms, for work that should wait for the commit: if it does not, it + /// has that long to land in the middle of the commit. + Briefly, +} + /// Starts `work` on another thread from inside the next commit on this -/// thread, at `moment`, and gives it time to finish there once it runs: work -/// that does not wait for the commit runs in the middle of it. +/// thread, at `moment`, and waits for it as `wait` says. fn during_next_commit_at( moment: Moment, + wait: Wait, work: impl FnOnce() -> T + Send + 'static, ) -> DuringCommit { let slot = Arc::new(Mutex::new(None)); @@ -71,7 +82,14 @@ fn during_next_commit_at( running .recv_timeout(Duration::from_secs(10)) .expect("the worker did not start"); - let _ = finished.recv_timeout(Duration::from_millis(300)); + match wait { + Wait::UntilDone => finished + .recv_timeout(Duration::from_secs(10)) + .expect("the work did not finish during the commit"), + Wait::Briefly => { + let _ = finished.recv_timeout(Duration::from_millis(300)); + } + } *handle_slot.lock().unwrap() = Some(handle); }; match moment { @@ -81,11 +99,12 @@ fn during_next_commit_at( DuringCommit(slot) } -/// [`during_next_commit_at`] right after the commit epoch is assigned. +/// [`during_next_commit_at`] right after the commit epoch is assigned, for +/// work that should wait for the commit. fn during_next_commit( work: impl FnOnce() -> T + Send + 'static, ) -> DuringCommit { - during_next_commit_at(Moment::EpochAssigned, work) + during_next_commit_at(Moment::EpochAssigned, Wait::Briefly, work) } fn by(db: &GrafeoDB, node: NodeId) -> Option { @@ -168,7 +187,7 @@ fn a_plain_read_does_not_see_a_commit_before_it_completes() { session.execute("INSERT (:Doc {id: 1})").unwrap(); let read = { let db = Arc::clone(&db); - during_next_commit_at(Moment::Stamped, move || count(&db)) + during_next_commit_at(Moment::Stamped, Wait::UntilDone, move || count(&db)) }; session.commit().unwrap(); diff --git a/crates/grafeo-engine/tests/direct_api.rs b/crates/grafeo-engine/tests/direct_api.rs index aa76d3e55..204b0b2d7 100644 --- a/crates/grafeo-engine/tests/direct_api.rs +++ b/crates/grafeo-engine/tests/direct_api.rs @@ -231,7 +231,7 @@ fn a_failing_batch_creates_nothing() { assert!( db.find_nodes_by_property("id", &Value::from("a")) .is_empty(), - "expected no nodes" + "with_index: {with_index}" ); let created = db diff --git a/docs/versioning.md b/docs/versioning.md index 7a6880cd1..9c3280411 100644 --- a/docs/versioning.md +++ b/docs/versioning.md @@ -57,4 +57,4 @@ Until 1.0, pin the minor version you test against. For example, to stay on 0.6: | npm | `"@grafeo-db/js": "^0.6.0"` | `>=0.6.0, <0.7.0` | | pip / uv | `grafeo~=0.6.0` | `>=0.6.0, <0.7` | -For pip and uv, write all three parts: `grafeo~=0.6` accepts every version below 1.0. +For pip and uv, write all three parts: `grafeo~=0.6` means `>=0.6, <1.0`, so it also accepts 0.7 and later.