Indexing, part 2

* Separate domain & codomain indexes * Fix some bugs in indexing that were breaking things at runtime * Implement AdaptiveRadixTree index strategy. Still has some bugs, so not turned on for anything at this point.
rdaum · Feb 19, 2024 · 2e72ca8 · 2e72ca8
1 parent 1aa5674
commit 2e72ca8
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 44 deletions.
diff --git a/crates/db/src/odb/object_relations.rs b/crates/db/src/odb/object_relations.rs
@@ -35,46 +35,30 @@ pub enum WorldStateRelation {
         DomainType = "Integer",
         CodomainType = "Integer",
         SecondaryIndexed = "true",
-        IndexType = "AdaptiveRadixTree",
-        SecondaryIndexType = "AdaptiveRadixTree",
+        IndexType = "Hash",
+        SecondaryIndexType = "Hash",
     ))]
     ObjectParent = 0,
     /// Object<->Location
     #[strum(props(
         DomainType = "Integer",
         CodomainType = "Integer",
         SecondaryIndexed = "true",
-        IndexType = "AdaptiveRadixTree",
-        SecondaryIndexType = "AdaptiveRadixTree",
+        IndexType = "Hash",
+        SecondaryIndexType = "Hash",
     ))]
     ObjectLocation = 1,
     /// Object->Flags (BitEnum<ObjFlag>)
-    #[strum(props(
-        DomainType = "Integer",
-        CodomainType = "Bytes",
-        IndexType = "AdaptiveRadixTree"
-    ))]
+    #[strum(props(DomainType = "Integer", CodomainType = "Bytes", IndexType = "Hash"))]
     ObjectFlags = 2,
     /// Object->Name
-    #[strum(props(
-        DomainType = "Integer",
-        CodomainType = "String",
-        IndexType = "AdaptiveRadixTree"
-    ))]
+    #[strum(props(DomainType = "Integer", CodomainType = "String", IndexType = "Hash"))]
     ObjectName = 3,
     /// Object->Owner
-    #[strum(props(
-        DomainType = "Integer",
-        CodomainType = "Integer",
-        IndexType = "AdaptiveRadixTree"
-    ))]
+    #[strum(props(DomainType = "Integer", CodomainType = "Integer", IndexType = "Hash"))]
     ObjectOwner = 4,
     /// Object->Verbs (Verbdefs)
-    #[strum(props(
-        DomainType = "Integer",
-        CodomainType = "Bytes",
-        IndexType = "AdaptiveRadixTree"
-    ))]
+    #[strum(props(DomainType = "Integer", CodomainType = "Bytes", IndexType = "Hash"))]
     ObjectVerbs = 5,
     /// Verb UUID->VerbProgram (Binary)
     #[strum(props(DomainType = "Bytes", CodomainType = "Bytes", IndexType = "Hash"))]

diff --git a/crates/db/src/rdb/base_relation.rs b/crates/db/src/rdb/base_relation.rs
@@ -12,6 +12,7 @@
 // this program. If not, see <https://www.gnu.org/licenses/>.
 //
 
+use log::error;
 use std::collections::HashSet;
 
 use moor_values::util::SliceRef;
@@ -39,11 +40,11 @@ use crate::rdb::{IndexType, RelationError, RelationId, RelationInfo};
 pub struct BaseRelation {
     pub(crate) id: RelationId,
 
+    pub(crate) info: RelationInfo,
+
     /// The last successful committer's tx timestamp
     pub(crate) ts: u64,
 
-    pub(crate) unique_domain: bool,
-
     /// All the tuples in this relation. Indexed by their TupleId, so we can map back from the values in the indexes.
     tuples: im::HashMap<TupleId, TupleRef>,
 
@@ -81,24 +82,33 @@ impl BaseRelation {
             tuples: Default::default(),
             domain_index,
             codomain_index,
-            unique_domain,
+            info: relation_info,
         }
     }
 
-    /// Establish indexes for a tuple initial-loaded from secondary storage. Basically a, "trust us,
+    /// Establish indexes & storage for a tuple initial-loaded from secondary storage. Basically a, "trust us,
     /// this exists" move.
-    pub(crate) fn index_tuple(&mut self, mut tuple: TupleRef) {
+    pub(crate) fn load_tuple(&mut self, mut tuple: TupleRef) {
         // Reset timestamp to 0, since this is a tuple initial-loaded from secondary storage.
         tuple.update_timestamp(0);
 
-        self.domain_index
-            .index_tuple(&tuple.domain(), tuple.id())
-            .expect("Indexing failed");
+        let d_result = self.domain_index.index_tuple(&tuple.domain(), tuple.id());
+        if let Err(e) = d_result {
+            error!(
+                "Domain indexing failed on load for tuple {:?} in relation {:?}: {:?}",
+                tuple, self.info.name, e
+            );
+            return;
+        }
 
         if let Some(codomain_index) = &mut self.codomain_index {
-            codomain_index
-                .index_tuple(&tuple.codomain(), tuple.id())
-                .expect("Indexing failed");
+            let c_result = codomain_index.index_tuple(&tuple.codomain(), tuple.id());
+            if let Err(e) = c_result {
+                error!(
+                    "Codomain indexing failed on load for tuple {:?} in relation {:?}: {:?}",
+                    tuple, self.info.name, e
+                );
+            }
         }
 
         // Add the tuple to the relation.

diff --git a/crates/db/src/rdb/index/art_index.rs b/crates/db/src/rdb/index/art_index.rs
@@ -124,6 +124,9 @@ impl Index for ArtArrayIndex {
                 self.index.insert_k(&attr_key, set);
             }
             Some(ref mut v) => {
+                if self.unique && !v.is_empty() {
+                    return Err(RelationError::UniqueConstraintViolation);
+                }
                 v.insert(tuple_id);
             }
         }
@@ -139,6 +142,9 @@ impl Index for ArtArrayIndex {
 
         keys.remove(&tuple_id);
 
+        if self.unique && !keys.is_empty() {
+            return Err(RelationError::UniqueConstraintViolation);
+        }
         Ok(())
     }
 

diff --git a/crates/db/src/rdb/index/hash_index.rs b/crates/db/src/rdb/index/hash_index.rs
@@ -66,9 +66,9 @@ impl Index for HashIndex {
 
     fn seek(
         &self,
-        domain: &SliceRef,
+        attr: &SliceRef,
     ) -> Result<Box<dyn Iterator<Item = TupleId> + '_>, RelationError> {
-        let Some(set) = self.index.get(domain) else {
+        let Some(set) = self.index.get(attr) else {
             return Ok(Box::new(Iter {
                 iter: Box::new(std::iter::empty()),
             }));
@@ -95,6 +95,9 @@ impl Index for HashIndex {
         if !tuples.remove(&tuple_id) {
             return Err(RelationError::TupleNotFound);
         }
+        if self.unique && !tuples.is_empty() {
+            return Err(RelationError::UniqueConstraintViolation);
+        }
         Ok(())
     }
 

diff --git a/crates/db/src/rdb/index/im_hash_index.rs b/crates/db/src/rdb/index/im_hash_index.rs
@@ -65,9 +65,9 @@ impl Index for ImHashIndex {
 
     fn seek(
         &self,
-        domain: &SliceRef,
+        attr: &SliceRef,
     ) -> Result<Box<dyn Iterator<Item = TupleId> + '_>, RelationError> {
-        let Some(set) = self.index.get(domain) else {
+        let Some(set) = self.index.get(attr) else {
             return Ok(Box::new(Iter {
                 iter: Box::new(std::iter::empty()),
             }));
@@ -94,6 +94,9 @@ impl Index for ImHashIndex {
         tuples
             .remove(&tuple_id)
             .ok_or(RelationError::TupleNotFound)?;
+        if self.unique && !tuples.is_empty() {
+            return Err(RelationError::UniqueConstraintViolation);
+        }
         Ok(())
     }
 

diff --git a/crates/db/src/rdb/paging/cold_storage.rs b/crates/db/src/rdb/paging/cold_storage.rs
@@ -121,7 +121,7 @@ impl ColdStorage {
             for page_tuple_ids in relation_tuple_ids {
                 for tuple_id in page_tuple_ids {
                     let relation = &mut relations[relation_id.0];
-                    relation.index_tuple(tuple_id);
+                    relation.load_tuple(tuple_id);
                     restored_count += 1;
                 }
             }

diff --git a/crates/db/src/rdb/tx/transaction.rs b/crates/db/src/rdb/tx/transaction.rs
@@ -285,7 +285,7 @@ impl<'a> CommitSet<'a> {
                             .expect("failed to seek for constraints check");
                         let mut replacements = im::HashSet::new();
                         for t in results_canonical {
-                            if canonical.unique_domain && t.ts() > tuple.ts() {
+                            if canonical.info.unique_domain && t.ts() > tuple.ts() {
                                 return Err(CommitError::UniqueConstraintViolation);
                             }
                             // Check the timestamp on the upstream value, if it's newer than the read-timestamp,
@@ -337,7 +337,7 @@ impl<'a> CommitSet<'a> {
                             // Someone got here first and deleted the tuple we're trying to delete.
                             // If this is a tuple with a unique constraint, we need to check if there's a
                             // newer tuple with the same domain value.
-                            if canonical.unique_domain {
+                            if canonical.info.unique_domain {
                                 let results_canonical = canonical
                                     .seek_by_domain(tuple.domain())
                                     .expect("failed to seek for constraints check");

diff --git a/crates/db/src/rdb/tx/working_set.rs b/crates/db/src/rdb/tx/working_set.rs
@@ -569,12 +569,11 @@ impl WorkingSet {
         let old_tuples = db.with_relation(relation_id, |relation| {
             let tuples = relation.seek_by_domain(domain.clone())?;
 
-            if relation.unique_domain {
+            if relation.info.unique_domain {
                 if tuples.is_empty() {
                     return Err(RelationError::TupleNotFound);
                 }
                 if tuples.len() > 1 {
-                    error!("Ambiguous tuple in base");
                     return Err(RelationError::AmbiguousTuple);
                 }
             }