diff --git a/bindings/python/docs/api/database.md b/bindings/python/docs/api/database.md index 683a0036b94..89440f90d2d 100644 --- a/bindings/python/docs/api/database.md +++ b/bindings/python/docs/api/database.md @@ -820,7 +820,9 @@ Lookup a record by its RID. **Parameters:** -- `rid` (str): Record ID string (e.g. "#10:5") +- `rid` (str): Record ID string (e.g. "#10:5"). On the native wheel the id goes to + the engine as a parameter, so every lookup shares one statement text + ([Parameters](../guide/core/queries.md#parameters)) **Returns:** diff --git a/bindings/python/docs/guide/core/queries.md b/bindings/python/docs/guide/core/queries.md index dca023077dd..75eb65c8ea5 100644 --- a/bindings/python/docs/guide/core/queries.md +++ b/bindings/python/docs/guide/core/queries.md @@ -150,6 +150,14 @@ values, raising `arcadedb.sqlStatementCache` and `arcadedb.opencypher.statementCache` keeps more texts parsed. Identifiers (type, property, bucket names) cannot be bound and belong in the text. +A record id is a value too. The engine does not turn a record id written in a +`FROM` target (`SELECT FROM #10:5`) into a parameter, so each record would be a +new cached text. To read a record by its id, call `db.lookup_by_rid(rid)`: on +the native wheel it sends the id as a parameter, so every lookup shares one +statement text. The record wrappers do the same for their own statements +(`get_out_edges()`, `get_in_edges()`, `get_both_edges()`, `new_edge()`, +`Edge.get_in()`, `Edge.get_out()`, `save()` and `delete()`). + ```python # Named parameters (recommended) result = db.query( diff --git a/bindings/python/src/arcadedb_embedded/_native/database.py b/bindings/python/src/arcadedb_embedded/_native/database.py index 68fe220b894..8e75e04c00d 100644 --- a/bindings/python/src/arcadedb_embedded/_native/database.py +++ b/bindings/python/src/arcadedb_embedded/_native/database.py @@ -13,6 +13,7 @@ from ..transactions import TransactionContext from ..type_conversion import json_bulk_dumps from . import lib, not_yet +from .codec import RID from .codec import _default as codec_default from .codec import _prep as _prep_row from .codec import encode_params, json_native, loads @@ -639,10 +640,13 @@ def count_type(self, type_name: str) -> int: def lookup_by_rid(self, rid: str): """The record at ``rid`` as a Document, Vertex or Edge wrapper.""" self._check_not_closed() - if not _RID_RE.fullmatch(str(rid)): + text = str(rid) + if not _RID_RE.fullmatch(text): raise ArcadeDBError(f"Failed to lookup RID '{rid}': not a record id") - rid_sql = f"SELECT FROM {rid}" # nosec B608 - rid matched _RID_RE above - row = self.query("sql", rid_sql).first() + # one statement text for every record id: the id is a parameter (see records.py, _UPDATE), not part of the text + row = self.query( + "sql", "SELECT FROM ?", RID(text if text[0] == "#" else "#" + text) + ).first() if row is None: raise ArcadeDBError(f"Failed to lookup RID '{rid}': record not found") return row.get_element() diff --git a/bindings/python/src/arcadedb_embedded/_native/records.py b/bindings/python/src/arcadedb_embedded/_native/records.py index 7f03d2f2307..86a073d1fa8 100644 --- a/bindings/python/src/arcadedb_embedded/_native/records.py +++ b/bindings/python/src/arcadedb_embedded/_native/records.py @@ -32,7 +32,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() @@ -105,7 +108,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 @@ -120,7 +123,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() @@ -136,7 +139,6 @@ 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() @@ -144,12 +146,12 @@ def new_edge(self, label: str, target: "Vertex", **kwargs) -> "Edge": 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 @@ -157,8 +159,8 @@ def new_edge(self, label: str, target: "Vertex", **kwargs) -> "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"]: @@ -175,8 +177,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: diff --git a/bindings/python/tests/conftest.py b/bindings/python/tests/conftest.py index 77820a36136..53e8a2fbbfa 100644 --- a/bindings/python/tests/conftest.py +++ b/bindings/python/tests/conftest.py @@ -29,6 +29,8 @@ collect_ignore.append("test_native_udf.py") # the native library's start-up output (#477) can only be checked with the native library loaded collect_ignore.append("test_native_startup_runtime.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.isdir(os.path.join(os.path.dirname(os.path.dirname(__file__)), "docs")): # the upstream pull request branch has no docs/ directory collect_ignore.append("test_docs_examples.py") diff --git a/bindings/python/tests/test_graph_api.py b/bindings/python/tests/test_graph_api.py index 07a9f0a057e..09471bcafac 100644 --- a/bindings/python/tests/test_graph_api.py +++ b/bindings/python/tests/test_graph_api.py @@ -5,6 +5,8 @@ """ import arcadedb_embedded as arcadedb +import pytest +from arcadedb_embedded import ArcadeDBError from arcadedb_embedded.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] diff --git a/bindings/python/tests/test_native_statement_texts.py b/bindings/python/tests/test_native_statement_texts.py new file mode 100644 index 00000000000..eae981f8fae --- /dev/null +++ b/bindings/python/tests/test_native_statement_texts.py @@ -0,0 +1,94 @@ +"""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 wrappers send the record id as a parameter instead, so a traversal step or a lookup has one text for every record. +""" + +import re + +import arcadedb_embedded as arcadedb +import pytest +from arcadedb_embedded._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) + assert len(looked_up) == count and len(set(looked_up)) == 1 + 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 looked_up + 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