From 185ef1f26b7433b82b50cb7ecbcd6fe8b1e05d6b Mon Sep 17 00:00:00 2001 From: Taewoon Kim Date: Tue, 3 Nov 2026 08:22:34 +0900 Subject: [PATCH 1/2] Native: record ids go to the engine as a parameter, not in the statement text Port of humemai/arcadedb-embedded-python#529 (branch perf/native-rid-param, never merged there) onto current main. On the native build a record id was pasted into the SQL text, so each traversal step and each lookup_by_rid was a new statement text. The engine caches a parsed statement by its text (arcadedb.sqlStatementCache, 300 entries) and its literal parameterizer keeps a literal in a FROM target, so 25 records meant 25 cache entries. The id now goes over as a parameter (the RID the codec already tags as R): lookup_by_rid (SELECT FROM ?), Vertex.get_out_edges/get_in_edges/get_both_edges, Vertex.new_edge, Edge.get_in/get_out, save() (UPDATE :rid MERGE :row) and delete() (DELETE FROM :rid). Id validation and error messages are unchanged. test_native_statement_texts.py counts the distinct statement texts (native only, not collected on JPype); the new behaviour tests in test_graph_api.py run on both builds. Untimed counts only: a timed A/B decides it before merge. Refs #19 Co-Authored-By: Claude Opus 5.5 --- .../pages/how-to/databases-and-queries.adoc | 2 + src/arcadedb/_native/database.py | 10 +- src/arcadedb/_native/records.py | 26 ++--- tests/conftest.py | 2 + tests/test_graph_api.py | 97 +++++++++++++++++++ tests/test_native_statement_texts.py | 94 ++++++++++++++++++ 6 files changed, 216 insertions(+), 15 deletions(-) create mode 100644 tests/test_native_statement_texts.py diff --git a/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc b/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc index 543d1c0..c3d0535 100644 --- a/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc +++ b/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 a record id never goes into a 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. `db.lookup_by_rid()` and the record wrappers (`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. + [source,python,subs="attributes+"] ---- row = db.query("sql", "SELECT @rid AS r, name FROM User LIMIT 1").first() diff --git a/src/arcadedb/_native/database.py b/src/arcadedb/_native/database.py index 3c876fe..45e7c1b 100644 --- a/src/arcadedb/_native/database.py +++ b/src/arcadedb/_native/database.py @@ -15,6 +15,7 @@ from ..type_conversion import json_bulk_dumps from . import interrupts, lib, needs_jvm from .bulk import insert_rows_columnar +from .codec import RID from .codec import _prep as _prep_row from .codec import encode_params, json_native, loads, row_default @@ -681,10 +682,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/src/arcadedb/_native/records.py b/src/arcadedb/_native/records.py index 6d81502..f790ade 100644 --- a/src/arcadedb/_native/records.py +++ b/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,7 +150,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() @@ -155,12 +157,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 @@ -168,8 +170,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"]: @@ -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: diff --git a/tests/conftest.py b/tests/conftest.py index c5961c2..d15d9ed 100644 --- a/tests/conftest.py +++ b/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") ): diff --git a/tests/test_graph_api.py b/tests/test_graph_api.py index 3b54991..7ba4264 100644 --- a/tests/test_graph_api.py +++ b/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] diff --git a/tests/test_native_statement_texts.py b/tests/test_native_statement_texts.py new file mode 100644 index 0000000..b36cae7 --- /dev/null +++ b/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 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) + 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 From 3256b013e5433a3f0a4a2fd4e5fc32da8626665b Mon Sep 17 00:00:00 2001 From: Taewoon Kim Date: Tue, 3 Nov 2026 10:08:04 +0900 Subject: [PATCH 2/2] Native: keep lookup_by_rid's literal id; the parameter path measured 2 % slower there First timed A/B (ab.py, laptop, native wheel of main vs this PR, 12 clean pairs): lookup_rid_us 1.022 [1.019, 1.025] (slower), out_edges_us 0.792 [0.790, 0.794], edge_ends_us 0.792 [0.789, 0.794], save_us 0.805 [0.801, 0.809] (faster). Guards flat, A/A clean. Narrow the PR to the paths that got faster; lookup_by_rid returns to main's text. database.py is now identical to main. Co-Authored-By: Claude Opus 5.5 --- .../ROOT/pages/how-to/databases-and-queries.adoc | 2 +- src/arcadedb/_native/database.py | 10 +++------- tests/test_native_statement_texts.py | 8 +++++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc b/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc index c3d0535..3b372fd 100644 --- a/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc +++ b/docs/modules/ROOT/pages/how-to/databases-and-queries.adoc @@ -243,7 +243,7 @@ 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 a record id never goes into a 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. `db.lookup_by_rid()` and the record wrappers (`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. +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+"] ---- diff --git a/src/arcadedb/_native/database.py b/src/arcadedb/_native/database.py index 45e7c1b..3c876fe 100644 --- a/src/arcadedb/_native/database.py +++ b/src/arcadedb/_native/database.py @@ -15,7 +15,6 @@ from ..type_conversion import json_bulk_dumps from . import interrupts, lib, needs_jvm from .bulk import insert_rows_columnar -from .codec import RID from .codec import _prep as _prep_row from .codec import encode_params, json_native, loads, row_default @@ -682,13 +681,10 @@ 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() - text = str(rid) - if not _RID_RE.fullmatch(text): + if not _RID_RE.fullmatch(str(rid)): raise ArcadeDBError(f"Failed to lookup RID '{rid}': not a record id") - # 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() + rid_sql = f"SELECT FROM {rid}" # nosec B608 - rid matched _RID_RE above + row = self.query("sql", rid_sql).first() if row is None: raise ArcadeDBError(f"Failed to lookup RID '{rid}': record not found") return row.get_element() diff --git a/tests/test_native_statement_texts.py b/tests/test_native_statement_texts.py index b36cae7..cfe42d5 100644 --- a/tests/test_native_statement_texts.py +++ b/tests/test_native_statement_texts.py @@ -2,7 +2,8 @@ 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. +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 @@ -66,7 +67,8 @@ def test_a_step_has_one_text_for_every_record(temp_db_path, texts): for vertex in vertices[10:20]: vertex.delete() writes = list(texts) - assert len(looked_up) == count and len(set(looked_up)) == 1 + # 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() @@ -74,7 +76,7 @@ def test_a_step_has_one_text_for_every_record(temp_db_path, texts): # 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: + for text in steps + ends + writes: assert not RID_IN_TEXT.search(text), text