Native: record ids go to the engine as a parameter, not in the statement text

docs/modules/ROOT/pages/how-to/databases-and-queries.adoc

@@ -243,6 +243,8 @@ A query vector for `vectorNeighbors` can be a numpy array or a list. The convers
 
 A record id inside a row value (`SELECT @rid AS r`, a `LINK` property, a list of links such as `out('Knows').@rid`) comes back as an RID object, not as text. `str(rid)` is the text (`"#10:5"`), so is an f-string, and `get_rid()` always returns text or `None`. Two RIDs of the same record are equal and hash alike, so they work in sets and as dict keys, and RIDs sort by bucket, then position. `getBucketId()`, `getPosition()`, `toString()` and `isValid()` work on the object. It goes back in as a query parameter (`WHERE @rid = ?`, or a list passed by name) and to `db.lookup_by_rid()`, as itself or as its text.
 
+On the native build the record wrappers send their record id to the engine as a parameter, not in the statement text. The engine keeps a parsed statement in a cache keyed by its text (`arcadedb.sqlStatementCache`, 300 entries by default) and leaves a record id written in a `FROM` target as part of that text, so each record would be a cache entry of its own. `get_out_edges()`, `get_in_edges()`, `get_both_edges()`, `new_edge()`, `Edge.get_in()`, `Edge.get_out()`, `save()` and `delete()` send the id as a parameter, so every record shares one text. `db.lookup_by_rid()` keeps the id in the statement text: measured, the parameter path was 2 % slower for a lookup, so it stays as it is.
+
 [source,python,subs="attributes+"]
 ----
 row = db.query("sql", "SELECT @rid AS r, name FROM User LIMIT 1").first()

src/arcadedb/_native/records.py

@@ -36,7 +36,10 @@ class _Mixin:
     """What the three native wrappers share (state in ``_db``, ``_type``, ``_rid``, ``_props``, ``_mutable``)."""
 
     _CREATE = "INSERT INTO {t} CONTENT :row"
-    _DELETE = "DELETE FROM {rid}"
+    # the record id goes over as a parameter (a RID, passed back it is a link), never in the text: the engine caches a parsed statement by its
+    # text and keeps a record id in a FROM target as part of it, so a pasted id would make one cache entry per record
+    _UPDATE = "UPDATE :rid MERGE :row"
+    _DELETE = "DELETE FROM :rid"
 
     def __init__(self, database, type_name, rid=None, props=None, mutable=False):
         self._java_document = _java_object()
@@ -116,7 +119,7 @@ def save(self):
             self._adopt(row)
         else:
             db.command(
-                "sql", f"UPDATE {self._rid} MERGE :row", {"row": self._props}
+                "sql", self._UPDATE, {"rid": RID(self._rid), "row": self._props}
             ).close()
         return self
 
@@ -131,7 +134,7 @@ def _adopt(self, row) -> None:
     def delete(self):
         if self._rid is None:
             raise ArcadeDBError("Cannot delete a record that was never saved")
-        self._db.command("sql", self._DELETE.format(rid=self._rid)).close()
+        self._db.command("sql", self._DELETE, {"rid": RID(self._rid)}).close()
 
     def modify(self):
         self._check_open()
@@ -147,29 +150,28 @@ class Document(_Mixin, _BaseDocument):
 
 class Vertex(_Mixin, _BaseVertex):
     _CREATE = "CREATE VERTEX {t} CONTENT :row"
-    _DELETE = "DELETE FROM {rid}"
 
     def new_edge(self, label: str, target: "Vertex", **kwargs) -> "Edge":
         self._check_open()
         if self._rid is None or getattr(target, "_rid", None) is None:
             raise ArcadeDBError(
                 "Both vertices of an edge must be saved before new_edge()"
             )
-        sql = f"CREATE EDGE {_q(label)} FROM {self._rid} TO {target._rid}"
+        sql = f"CREATE EDGE {_q(label)} FROM :src TO :dst"
+        params = {"src": RID(self._rid), "dst": RID(target._rid)}
         if kwargs:
             sql += " CONTENT :row"
-        row = self._db.command(
-            "sql", sql, *(({"row": kwargs},) if kwargs else ())
-        ).first()
+            params["row"] = kwargs
+        row = self._db.command("sql", sql, params).first()
         edge = Edge(self._db, label, mutable=False)
         edge._adopt(row)
         return edge
 
     def _edges(self, fn: str, labels) -> List["Edge"]:
         self._check_open()
         args = ", ".join("'" + label.replace("'", "\\'") + "'" for label in labels)
-        edges_sql = f"SELECT expand({fn}({args})) FROM {self._rid}"  # nosec B608 - fn is a fixed function name, labels are escaped literals, _rid is the record's own id
-        rows = self._db.query("sql", edges_sql)
+        edges_sql = f"SELECT expand({fn}({args})) FROM :rid"  # nosec B608 - fn is a fixed function name, labels are escaped literals, the record id is a parameter
+        rows = self._db.query("sql", edges_sql, {"rid": RID(self._rid)})
         return [_wrap(self._db, row) for row in rows]
 
     def get_out_edges(self, *labels: str) -> List["Edge"]:
@@ -186,8 +188,8 @@ class Edge(_Mixin, _BaseEdge):
 
     def _end(self, fn: str) -> "Vertex":
         self._check_open()
-        end_sql = f"SELECT expand({fn}()) FROM {self._rid}"  # nosec B608 - fn is a fixed function name, _rid is the record's own id
-        row = self._db.query("sql", end_sql).first()
+        end_sql = f"SELECT expand({fn}()) FROM :rid"  # nosec B608 - fn is a fixed function name, the record id is a parameter
+        row = self._db.query("sql", end_sql, {"rid": RID(self._rid)}).first()
         return _wrap(self._db, row)
 
     def get_in(self) -> Vertex:

tests/conftest.py

@@ -56,6 +56,8 @@
     collect_ignore.append("test_native_point_read.py")
     # the vector provider the native glue picks by default (ARCADEDB_VECTOR_PROVIDER, #153), observed on the native library
     collect_ignore.append("test_native_vector_provider.py")
+    # counts the statement texts the native backend hands to the engine (the JPype backend has no such layer)
+    collect_ignore.append("test_native_statement_texts.py")
 if not os.path.isfile(
     os.path.join(os.path.dirname(os.path.dirname(__file__)), "docs", "index.md")
 ):

tests/test_graph_api.py

@@ -5,6 +5,8 @@
 """
 
 import arcadedb
+import pytest
+from arcadedb import ArcadeDBError
 from arcadedb.graph import Document, Edge, Vertex
 
 
@@ -538,3 +540,98 @@ def test_delete_via_wrapper_on_fresh_lookup(temp_db_path):
         # Verify deleted
         items_after = list(db.query("sql", "SELECT FROM `Item`"))
         assert len(items_after) == 0, "Item should be deleted via .delete()"
+
+
+def _chain(db, count):
+    """``count`` Person vertices linked a -> b -> c ... by Knows edges (``w`` = the position of the edge)."""
+    db.command("sql", "CREATE VERTEX TYPE Person")
+    db.command("sql", "CREATE EDGE TYPE Knows")
+    db.command("sql", "CREATE EDGE TYPE Likes")
+    with db.transaction():
+        people = [db.new_vertex("Person").set("n", i).save() for i in range(count)]
+        for i in range(count - 1):
+            people[i].new_edge("Knows", people[i + 1], w=i)
+        people[0].new_edge("Likes", people[-1])
+    return [p.get_rid() for p in people]
+
+
+def test_traversal_steps_over_many_records(temp_db_path):
+    """Edges and ends of every vertex of a chain: the RID of the record each step starts from must not matter."""
+    with arcadedb.create_database(temp_db_path) as db:
+        rids = _chain(db, 12)
+        for i, rid in enumerate(rids):
+            vertex = db.lookup_by_rid(rid)
+            assert isinstance(vertex, Vertex)
+            assert vertex.get("n") == i
+            out_edges = vertex.get_out_edges("Knows")
+            in_edges = vertex.get_in_edges("Knows")
+            assert len(out_edges) == (1 if i < 11 else 0)
+            assert len(in_edges) == (1 if i > 0 else 0)
+            for edge in out_edges:
+                assert isinstance(edge, Edge)
+                assert edge.get_out().get_rid() == rid
+                assert edge.get_in().get_rid() == rids[i + 1]
+                assert edge.get("w") == i
+            for edge in in_edges:
+                assert edge.get_in().get_rid() == rid
+                assert edge.get_out().get_rid() == rids[i - 1]
+            # every edge type, in both directions
+            expected_both = (1 if i < 11 else 0) + (1 if i > 0 else 0)
+            expected_both += (
+                1 if i in (0, 11) else 0
+            )  # the Likes edge joins the first and the last
+            assert len(vertex.get_both_edges()) == expected_both
+        assert len(db.lookup_by_rid(rids[0]).get_out_edges()) == 2
+        assert len(db.lookup_by_rid(rids[0]).get_out_edges("Knows", "Likes")) == 2
+        assert len(db.lookup_by_rid(rids[0]).get_out_edges("Likes")) == 1
+
+
+def test_save_and_delete_by_record_id_for_many_records(temp_db_path):
+    """modify().save() writes to the record the wrapper holds; delete() removes only that record."""
+    with arcadedb.create_database(temp_db_path) as db:
+        rids = _chain(db, 6)
+        with db.transaction():
+            for i, rid in enumerate(rids):
+                db.lookup_by_rid(rid).modify().set("n", 100 + i).save()
+        assert [db.lookup_by_rid(rid).get("n") for rid in rids] == [
+            100 + i for i in range(6)
+        ]
+        with db.transaction():
+            db.lookup_by_rid(rids[2]).delete()
+            db.lookup_by_rid(rids[4]).delete()
+        left = {r.get("n") for r in db.query("sql", "SELECT n FROM Person")}
+        assert left == {100, 101, 103, 105}
+
+
+def test_new_edge_without_and_with_properties_between_saved_vertices(temp_db_path):
+    with arcadedb.create_database(temp_db_path) as db:
+        rids = _chain(db, 3)
+        with db.transaction():
+            a, b = db.lookup_by_rid(rids[0]), db.lookup_by_rid(rids[1])
+            plain = a.new_edge("Likes", b)
+            rich = b.new_edge("Likes", a, since="2020", score=0.5)
+        assert (
+            plain.get_out().get_rid() == rids[0] and plain.get_in().get_rid() == rids[1]
+        )
+        assert (
+            rich.get_out().get_rid() == rids[1] and rich.get_in().get_rid() == rids[0]
+        )
+        assert rich.get("since") == "2020" and rich.get("score") == 0.5
+        assert plain.get("since") is None
+
+
+def test_lookup_by_rid_refusals_name_the_record_id(temp_db_path):
+    """A missing record, a missing bucket and a text that is no record id each raise ArcadeDBError naming the id."""
+    with arcadedb.create_database(temp_db_path) as db:
+        rids = _chain(db, 2)
+        for missing in ("#1:99", "#77:0"):
+            with pytest.raises(
+                ArcadeDBError, match=f"Failed to lookup RID '{missing}'"
+            ):
+                db.lookup_by_rid(missing)
+        for bad in ("bad", "", "#1:0 ", "#1:0; DROP TYPE Person"):
+            with pytest.raises(ArcadeDBError, match="Failed to lookup RID"):
+                db.lookup_by_rid(bad)
+        # the refused texts did nothing to the data
+        assert db.query("sql", "SELECT count(*) AS c FROM Person").first().get("c") == 2
+        assert db.lookup_by_rid(rids[0]).get_rid() == rids[0]

tests/test_native_statement_texts.py

@@ -0,0 +1,96 @@
+"""The statement texts the native backend sends to the engine, on the native backend only (not collected on JPype).
+
+The engine keeps parsed statements in a cache keyed by statement text (``arcadedb.sqlStatementCache``, 300 entries by default), and it
+leaves a record id written in a FROM target as part of the text. A record id pasted into the text therefore makes one cache entry per
+record. The edge getters, ``Edge.get_in``/``get_out``, ``new_edge``, ``save`` and ``delete`` send the record id as a parameter instead, so
+each has one text for every record. ``lookup_by_rid`` keeps the id in the text: a timed A/B found the parameter path 2 % slower there.
+"""
+
+import re
+
+import arcadedb as arcadedb
+import pytest
+from arcadedb._native.database import NativeDatabase
+
+RID_IN_TEXT = re.compile(r"#-?\d+:-?\d+")
+
+
+@pytest.fixture
+def texts(monkeypatch):
+    """Every statement text handed to the engine, in order, while the fixture is active."""
+    seen = []
+    db_class = NativeDatabase
+    original = db_class._exec
+
+    def recording(self, flags, prefix, language, command, args):
+        seen.append(command)
+        return original(self, flags, prefix, language, command, args)
+
+    monkeypatch.setattr(db_class, "_exec", recording)
+    return seen
+
+
+def _graph(db, count):
+    db.command("sql", "CREATE VERTEX TYPE Person")
+    db.command("sql", "CREATE EDGE TYPE Knows")
+    with db.transaction():
+        people = [db.new_vertex("Person").set("n", i).save() for i in range(count)]
+        for i in range(count - 1):
+            people[i].new_edge("Knows", people[i + 1])
+    return [p.get_rid() for p in people]
+
+
+def test_a_step_has_one_text_for_every_record(temp_db_path, texts):
+    count = 25
+    with arcadedb.create_database(temp_db_path) as db:
+        rids = _graph(db, count)
+        del texts[:]
+        vertices = [db.lookup_by_rid(rid) for rid in rids]
+        looked_up = list(texts)
+        del texts[:]
+        for vertex in vertices[:-1]:
+            vertex.get_out_edges("Knows")
+            vertex.get_in_edges()
+            vertex.get_both_edges()
+        steps = list(texts)
+        del texts[:]
+        edges = [v.get_out_edges("Knows")[0] for v in vertices[:-1]]
+        del texts[:]
+        for edge in edges:
+            edge.get_out()
+            edge.get_in()
+        ends = list(texts)
+        del texts[:]
+        with db.transaction():
+            for vertex in vertices[:10]:
+                vertex.modify().set("n", -1).save()
+            for vertex in vertices[10:20]:
+                vertex.delete()
+        writes = list(texts)
+    # lookup_by_rid keeps the literal id in its text (one text per record, as on main): the parameter path measured slower
+    assert len(looked_up) == count and len(set(looked_up)) == count
+    assert (
+        len(steps) == 3 * (count - 1) and len(set(steps)) == 3
+    )  # outE('Knows'), inE(), bothE()
+    assert len(ends) == 2 * (count - 1) and len(set(ends)) == 2  # inV(), outV()
+    # the update and the delete are one text each (the transaction's own statements are not counted)
+    assert len({t for t in writes if t.startswith("UPDATE")}) == 1
+    assert len({t for t in writes if t.startswith("DELETE")}) == 1
+    for text in steps + ends + writes:
+        assert not RID_IN_TEXT.search(text), text
+
+
+def test_new_edge_text_depends_on_the_label_only(temp_db_path, texts):
+    with arcadedb.create_database(temp_db_path) as db:
+        rids = _graph(db, 2)
+        db.command("sql", "CREATE EDGE TYPE Likes")
+        with db.transaction():
+            vertices = [db.lookup_by_rid(rid) for rid in rids]
+            del texts[:]
+            for _ in range(8):
+                vertices[0].new_edge("Likes", vertices[1])
+                vertices[1].new_edge("Likes", vertices[0], w=1)
+            created = list(texts)
+    assert len(created) == 16 and len(set(created)) == 2  # with and without CONTENT
+    for text in created:
+        assert not RID_IN_TEXT.search(text), text