fix: tx-memo edge cases — tx entities, nesting/exceptions, bounded attr keys

- Memo freshness now requires e <= max_allocatable_entity_id: datoms on
  transaction entities (e >= tx0) can already carry index datoms from
  earlier transactions, so routing them through the empty memo bucket
  dropped the retraction a cardinality-one update needs.
- apply_tx saves/restores the shared memo record (Fun.protect): a nested
  transaction via Call, or an exception raised mid-tx, no longer leaks
  this transaction's state into the outer one — the outer transaction
  also keeps its memo instead of losing it to the inner reset.
- attr_key_tbl is capped (65536 entries): keyword values flow through
  compare_attr too, so an unbounded vocabulary would grow the table
  without bound; past the cap keys are recomputed per call.
- Regression test: cardinality-one update on a past tx entity.

Author: tiensonqin

impl/datascript.ml

@@ -63,7 +63,9 @@ let tx_memo_apply tx_data =
   if transact_tx_memo.active then
     List.iter
       (fun d ->
-        if d.e > transact_tx_memo.base_max_eid then
+        if d.e > transact_tx_memo.base_max_eid
+           && d.e <= Db_impl.max_allocatable_entity_id
+        then
           let key = d.e, d.a in
           let bucket =
             Option.value (Hashtbl.find_opt transact_tx_memo.datoms key)
@@ -516,8 +518,13 @@ let add_active_datom_with_report_db ?(allow_tuple = false) ?(validate_value = tr
     (* tx-fresh entities (e allocated inside this transaction) cannot exist
        in the pre-tx index, so their same-fact and entity/attr lookups are
        answered from the tx memo instead of descending the index trees. *)
+    (* Transaction entities live above max_allocatable_entity_id and can
+       already carry index datoms from earlier transactions, so they are
+       never memo-fresh. *)
     let tx_fresh_bucket =
-      if transact_tx_memo.active && d.e > transact_tx_memo.base_max_eid then
+      if transact_tx_memo.active && d.e > transact_tx_memo.base_max_eid
+         && d.e <= Db_impl.max_allocatable_entity_id
+      then
         Some
           (Option.value
              (Hashtbl.find_opt transact_tx_memo.datoms (d.e, d.a))

impl/transact.ml

@@ -397,10 +397,13 @@ let remap_tempid_entity old_e new_e tempids =
 
 
 (* Per-transaction memo of datoms added to entities allocated inside this
-   transaction (e > base_max_eid): they cannot exist in the pre-tx index, so
-   same-fact and entity/attr lookups for them can be answered from this table
-   instead of descending the index trees.  Bulk seed transactions (imports)
-   consist almost entirely of such datoms. *)
+   transaction (base_max_eid < e <= max_allocatable_entity_id): they cannot
+   exist in the pre-tx index, so same-fact and entity/attr lookups for them
+   can be answered from this table instead of descending the index trees.
+   Bulk seed transactions (imports) consist almost entirely of such datoms.
+   Transaction entities (e >= tx0) are excluded: they are ids, not freshly
+   allocated entities, and may already carry index datoms from earlier
+   transactions. *)
 type tx_memo =
   { mutable active : bool
   ; mutable base_max_eid : entity_id
@@ -447,6 +450,24 @@ type apply_context =
 let apply_tx context tx_ops db =
   if context.is_filtered db then invalid_arg "filtered db is read-only";
   let tx_memo = context.tx_memo in
+  (* The memo record is shared by every transaction, so a nested transaction
+     (a Call fn that transacts) or an exception raised mid-tx must not leak
+     this transaction's state into the outer one: save any enclosing state
+     and restore it when this transaction ends or aborts. *)
+  let saved_memo =
+    if tx_memo.active then
+      Some (tx_memo.base_max_eid, Hashtbl.copy tx_memo.datoms)
+    else None
+  in
+  let restore_memo () =
+    match saved_memo with
+    | Some (base_max_eid, datoms) ->
+      Hashtbl.reset tx_memo.datoms;
+      Hashtbl.iter (Hashtbl.replace tx_memo.datoms) datoms;
+      tx_memo.base_max_eid <- base_max_eid
+    | None -> tx_memo.active <- false
+  in
+  Fun.protect ~finally:restore_memo @@ fun () ->
   tx_memo.base_max_eid <- db.max_eid;
   Hashtbl.reset tx_memo.datoms;
   tx_memo.active <- true;
@@ -1890,5 +1911,4 @@ let apply_tx context tx_ops db =
     , tx_data
     )
   in
-  tx_memo.active <- false;
   result

impl/transact.mli

@@ -49,9 +49,11 @@ val remap_resolved_tx_value : context -> entity_id -> entity_id -> tx_value -> t
 val remap_tempid_entity : entity_id -> entity_id -> tempid_map -> tempid_map
 
 (* Per-transaction memo of datoms added to entities allocated inside this
-   transaction (e > base_max_eid): they cannot exist in the pre-tx index, so
-   same-fact and entity/attr lookups for them can be answered from this table
-   instead of descending the index trees. *)
+   transaction (base_max_eid < e <= max_allocatable_entity_id): they cannot
+   exist in the pre-tx index, so same-fact and entity/attr lookups for them
+   can be answered from this table instead of descending the index trees.
+   Transaction entities (e >= tx0) are excluded: they may already carry
+   index datoms from earlier transactions. *)
 type tx_memo =
   { mutable active : bool
   ; mutable base_max_eid : entity_id

impl/util.ml

@@ -124,6 +124,10 @@ let split_keyword keyword =
    a missing ns sorts first — exactly the pairwise order.  Keys are
    memoized by attr content since index comparators see the same small
    set of attrs over and over. *)
+(* Bounded: keyword values also flow through compare_attr, so an unlimited
+   vocabulary would grow this table without bound; past the cap, keys are
+   recomputed per call. *)
+let attr_key_tbl_cap = 65536
 let attr_key_tbl : (attr, attr) Hashtbl.t = Hashtbl.create 256
 
 let attr_key attr =
@@ -137,7 +141,8 @@ let attr_key attr =
         String.init (String.length attr) (fun j ->
           if j = i then '\x00' else attr.[j])
     in
-    Hashtbl.replace attr_key_tbl attr key;
+    if Hashtbl.length attr_key_tbl < attr_key_tbl_cap then
+      Hashtbl.replace attr_key_tbl attr key;
     key
 
 let compare_attr left right =

test/test_datascript.ml

@@ -2314,6 +2314,24 @@ let test_transact__test_resolve_current_tx () =
     [ "db/current-tx", tx0 + 1; "entity", 1 ]
     report.tempids
 
+let test_transact__test_tx_entity_cardinality () =
+  (* Datoms on a transaction entity created by an earlier transaction
+     already live in the index: a cardinality-one update on it in a later
+     transaction must retract the previous value — tx ids are above
+     max_allocatable_entity_id, not transaction-fresh entities. *)
+  let schema = [ "note", indexed ] in
+  let report1 =
+    transact (empty_db ~schema ()) [ Add (CurrentTx, "note", String "v1") ]
+  in
+  let tx_eid = Option.get (resolve_tempid report1.tempids "db/current-tx") in
+  let report2 =
+    transact report1.db_after [ Add (Entity_id tx_eid, "note", String "v2") ]
+  in
+  assert_equal_triples
+    "cardinality-one update on a past tx entity retracts the old value"
+    [ tx_eid, "note", String "v2" ]
+    (datoms report2.db_after Eavt ~e:tx_eid ~a:"note" ())
+
 let test_current_tx_string_aliases_resolve_in_transactions () =
   let report =
     transact
@@ -17560,6 +17578,7 @@ let () =
   test_transact_report_exposes_tempids ();
   test_resolve_tempid_reads_tx_report_tempids ();
   test_transact__test_resolve_current_tx ();
+  test_transact__test_tx_entity_cardinality ();
   test_current_tx_string_aliases_resolve_in_transactions ();
   test_current_tx_string_aliases_can_be_value_only ();
   test_current_tx_colon_string_alias_resolves_in_transactions ();