Features/py sec indices (#1875)

* fix issue with secondary index not overwriting the prop * fix the issue with time secondary index tuple not working for properties addition * add tests for add node secondary index * add more tests * impl py secondary index * add and fix tests --------- Co-authored-by: Shivam Kapoor <[email protected]>
Pometry · Nov 29, 2024 · a8813ad · a8813ad
1 parent e018f20
commit a8813ad
Show file tree

Hide file tree

Showing 9 changed files with 840 additions and 50 deletions.
diff --git a/python/tests/test_graphdb/test_graphdb.py b/python/tests/test_graphdb/test_graphdb.py
@@ -457,10 +457,10 @@ def test_graph_properties():
     assert g.properties["prop 5"] == {"x": 1, "y": "ok"}
 
     props = {"prop 4": 11, "prop 5": "world", "prop 6": False}
-    g.add_property(1, props)
+    g.add_properties(1, props)
 
     props = {"prop 6": True}
-    g.add_property(2, props)
+    g.add_properties(2, props)
 
     def history_test(key, value):
         if value is None:
@@ -2054,7 +2054,7 @@ def check(g):
         assert mg.nodes.collect()[0].name == "1"
 
         props = {"prop 4": 11, "prop 5": "world", "prop 6": False}
-        mg.add_property(1, props)
+        mg.add_properties(1, props)
 
         props = {"prop 1": 1, "prop 2": "hi", "prop 3": True}
         mg.add_constant_properties(props)
@@ -2755,10 +2755,10 @@ def check(g):
 
 def test_unique_temporal_properties():
     g = Graph()
-    g.add_property(1, {"name": "tarzan"})
-    g.add_property(2, {"name": "tarzan2"})
-    g.add_property(3, {"name": "tarzan2"})
-    g.add_property(2, {"salary": "1000"})
+    g.add_properties(1, {"name": "tarzan"})
+    g.add_properties(2, {"name": "tarzan2"})
+    g.add_properties(3, {"name": "tarzan2"})
+    g.add_properties(2, {"salary": "1000"})
     g.add_constant_properties({"type": "character"})
     g.add_edge(1, 1, 2, properties={"status": "open"})
     g.add_edge(2, 1, 2, properties={"status": "open"})
@@ -3057,6 +3057,162 @@ def test_edge_layer_properties():
     assert g.edge("A", "B").properties == {"greeting": "namaste"}
 
 
+def test_add_node_properties_ordered_by_secondary_index():
+    g = Graph()
+    g.add_node(1, "A", properties={"prop": 1}, secondary_index=3)
+    g.add_node(1, "A", properties={"prop": 2}, secondary_index=2)
+    g.add_node(1, "A", properties={"prop": 3}, secondary_index=1)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 3), (1, 2), (1, 1)]
+
+
+def test_add_node_properties_overwritten_for_same_secondary_index():
+    g = Graph()
+    g.add_node(1, "A", properties={"prop": 1}, secondary_index=1)
+    g.add_node(1, "A", properties={"prop": 2}, secondary_index=1)
+    g.add_node(1, "A", properties={"prop": 3}, secondary_index=1)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 3)]
+
+    g = Graph()
+    g.add_node(1, "A", properties={"prop": 1}, secondary_index=1)
+    g.add_node(1, "A", properties={"prop": 2}, secondary_index=2)
+    g.add_node(1, "A", properties={"prop": 3}, secondary_index=2)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 1), (1, 3)]
+
+
+def test_create_node_properties_ordered_by_secondary_index():
+    g = Graph()
+    g.create_node(1, "A", properties={"prop": 1}, secondary_index=3)
+    g.add_node(1, "A", properties={"prop": 2}, secondary_index=2)
+    g.add_node(1, "A", properties={"prop": 3}, secondary_index=1)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 3), (1, 2), (1, 1)]
+
+
+def test_create_node_properties_overwritten_for_same_secondary_index():
+    g = Graph()
+    g.create_node(1, "A", properties={"prop": 1}, secondary_index=1)
+    g.add_node(1, "A", properties={"prop": 2}, secondary_index=1)
+    g.add_node(1, "A", properties={"prop": 3}, secondary_index=1)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 3)]
+
+    g = Graph()
+    g.create_node(1, "A", properties={"prop": 1}, secondary_index=1)
+    g.add_node(1, "A", properties={"prop": 2}, secondary_index=2)
+    g.add_node(1, "A", properties={"prop": 3}, secondary_index=2)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 1), (1, 3)]
+
+
+def test_add_edge_properties_ordered_by_secondary_index():
+    g = Graph()
+    g.add_edge(1, "A", "B", properties={"prop": 1}, secondary_index=3)
+    g.add_edge(1, "A", "B", properties={"prop": 2}, secondary_index=2)
+    g.add_edge(1, "A", "B", properties={"prop": 3}, secondary_index=1)
+
+    assert g.edge("A", "B").properties.temporal.get("prop").items() == [(1, 3), (1, 2), (1, 1)]
+
+
+def test_add_edge_properties_overwritten_for_same_secondary_index():
+    g = Graph()
+    g.add_edge(1, "A", "B", properties={"prop": 1}, secondary_index=1)
+    g.add_edge(1, "A", "B", properties={"prop": 2}, secondary_index=1)
+    g.add_edge(1, "A", "B", properties={"prop": 3}, secondary_index=1)
+
+    assert g.edge("A", "B").properties.temporal.get("prop").items() == [(1, 3)]
+
+    g = Graph()
+    g.add_edge(1, "A", "B", properties={"prop": 1}, secondary_index=1)
+    g.add_edge(1, "A", "B", properties={"prop": 2}, secondary_index=2)
+    g.add_edge(1, "A", "B", properties={"prop": 3}, secondary_index=2)
+
+    assert g.edge("A", "B").properties.temporal.get("prop").items() == [(1, 1), (1, 3)]
+
+
+def test_add_properties_properties_ordered_by_secondary_index():
+    g = Graph()
+    g.add_properties(1, properties={"prop": 1}, secondary_index=3)
+    g.add_properties(1, properties={"prop": 2}, secondary_index=2)
+    g.add_properties(1, properties={"prop": 3}, secondary_index=1)
+
+    assert g.properties.temporal.get("prop").items() == [(1, 3), (1, 2), (1, 1)]
+
+
+def test_add_properties_properties_overwritten_for_same_secondary_index():
+    g = Graph()
+    g.add_properties(1, properties={"prop": 1}, secondary_index=1)
+    g.add_properties(1, properties={"prop": 2}, secondary_index=1)
+    g.add_properties(1, properties={"prop": 3}, secondary_index=1)
+
+    assert g.properties.temporal.get("prop").items() == [(1, 3)]
+
+    g = Graph()
+    g.add_properties(1, properties={"prop": 1}, secondary_index=1)
+    g.add_properties(1, properties={"prop": 2}, secondary_index=2)
+    g.add_properties(1, properties={"prop": 3}, secondary_index=2)
+
+    assert g.properties.temporal.get("prop").items() == [(1, 1), (1, 3)]
+
+
+def test_node_add_updates_properties_ordered_by_secondary_index():
+    g = Graph()
+    g.add_node(1, "A")
+    g.node("A").add_updates(1, properties={"prop": 1}, secondary_index=3)
+    g.node("A").add_updates(1, properties={"prop": 2}, secondary_index=2)
+    g.node("A").add_updates(1, properties={"prop": 3}, secondary_index=1)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 3), (1, 2), (1, 1)]
+
+
+def test_node_add_updates_properties_overwritten_for_same_secondary_index():
+    g = Graph()
+    g.add_node(1, "A")
+    g.node("A").add_updates(1, properties={"prop": 1}, secondary_index=1)
+    g.node("A").add_updates(1, properties={"prop": 2}, secondary_index=1)
+    g.node("A").add_updates(1, properties={"prop": 3}, secondary_index=1)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 3)]
+
+    g = Graph()
+    g.add_node(1, "A")
+    g.node("A").add_updates(1, properties={"prop": 1}, secondary_index=1)
+    g.node("A").add_updates(1, properties={"prop": 2}, secondary_index=2)
+    g.node("A").add_updates(1, properties={"prop": 3}, secondary_index=2)
+
+    assert g.node("A").properties.temporal.get("prop").items() == [(1, 1), (1, 3)]
+
+
+def test_edge_add_updates_properties_ordered_by_secondary_index():
+    g = Graph()
+    g.add_edge(1, "A", "B")
+    g.edge("A", "B").add_updates(1, properties={"prop": 1}, secondary_index=3)
+    g.edge("A", "B").add_updates(1, properties={"prop": 2}, secondary_index=2)
+    g.edge("A", "B").add_updates(1, properties={"prop": 3}, secondary_index=1)
+
+    assert g.edge("A", "B").properties.temporal.get("prop").items() == [(1, 3), (1, 2), (1, 1)]
+
+
+def test_edge_add_updates_properties_overwritten_for_same_secondary_index():
+    g = Graph()
+    g.add_edge(1, "A", "B")
+    g.edge("A", "B").add_updates(1, properties={"prop": 1}, secondary_index=1)
+    g.edge("A", "B").add_updates(1, properties={"prop": 2}, secondary_index=1)
+    g.edge("A", "B").add_updates(1, properties={"prop": 3}, secondary_index=1)
+
+    assert g.edge("A", "B").properties.temporal.get("prop").items() == [(1, 3)]
+
+    g = Graph()
+    g.add_edge(1, "A", "B")
+    g.edge("A", "B").add_updates(1, properties={"prop": 1}, secondary_index=1)
+    g.edge("A", "B").add_updates(1, properties={"prop": 2}, secondary_index=2)
+    g.edge("A", "B").add_updates(1, properties={"prop": 3}, secondary_index=2)
+
+    assert g.edge("A", "B").properties.temporal.get("prop").items() == [(1, 1), (1, 3)]
+
+
 @fixture
 def datadir(tmpdir, request):
     filename = request.module.__file__

diff --git a/raphtory/src/core/entities/properties/tcell.rs b/raphtory/src/core/entities/properties/tcell.rs
@@ -27,14 +27,16 @@ impl<A: Clone + Debug + PartialEq + Send + Sync> TCell<A> {
             TCell::Empty => {
                 *self = TCell::TCell1(t, value);
             }
-            TCell::TCell1(t0, _) => {
+            TCell::TCell1(t0, v) => {
                 if &t != t0 {
                     if let TCell::TCell1(t0, value0) = std::mem::take(self) {
                         let mut svm = SVM::new();
                         svm.insert(t, value);
                         svm.insert(t0, value0);
                         *self = TCell::TCellCap(svm)
                     }
+                } else {
+                    *v = value
                 }
             }
             TCell::TCellCap(svm) => {
@@ -183,7 +185,10 @@ mod tcell_tests {
         let mut tcell = TCell::new(TimeIndexEntry::start(1), "Pometry");
         tcell.set(TimeIndexEntry::start(1), "Pometry Inc.");
 
-        assert_eq!(tcell.iter_t().collect::<Vec<_>>(), vec![(1, &"Pometry")]);
+        assert_eq!(
+            tcell.iter_t().collect::<Vec<_>>(),
+            vec![(1, &"Pometry Inc.")]
+        );
     }
 
     #[test]

diff --git a/raphtory/src/core/entities/properties/tprop.rs b/raphtory/src/core/entities/properties/tprop.rs
@@ -430,7 +430,7 @@ mod tprop_tests {
 
         assert_eq!(
             tprop.iter_t().collect::<Vec<_>>(),
-            vec![(1, "Pometry".into())]
+            vec![(1, "Pometry Inc.".into())]
         );
     }
 

diff --git a/raphtory/src/db/api/mutation/property_addition_ops.rs b/raphtory/src/db/api/mutation/property_addition_ops.rs
@@ -1,5 +1,5 @@
 use crate::{
-    core::utils::{errors::GraphError, time::TryIntoTime},
+    core::utils::errors::GraphError,
     db::api::mutation::{
         internal::{InternalAdditionOps, InternalPropertyAdditionOps},
         TryIntoInputTime,
@@ -9,7 +9,7 @@ use crate::{
 use super::{time_from_input, CollectProperties};
 
 pub trait PropertyAdditionOps {
-    fn add_properties<T: TryIntoTime, PI: CollectProperties>(
+    fn add_properties<T: TryIntoInputTime, PI: CollectProperties>(
         &self,
         t: T,
         props: PI,