Skip to content

Commit ba4e972

Browse files
committed
fix: clean duplicate_* index tables on retract; test transit read-cache wrap
- refresh_indexes_with_tx_data removed a retracted fact from the three PSet indexes but left its copy in duplicate_datoms and the duplicate_*_by_attr/entity tables (populated by full rebuilds via set_indexes_from_datoms/restore). find_avet_exact consults duplicate_avet_by_attr, so a restored+retracted unique-attr fact still resolved lookup refs instead of raising 'Nothing found for entity id'. Now the fact is filtered from all duplicate tables on retract, matching upstream where a retracted datom is gone entirely. - test_melange_transit_backend: regression test for the transit read cache wrapping at 1936 entries (the melange-transit 0.1.0 misdecode fixed in 0.1.2) — a ^XX cache code past the wrap must resolve to the overwritten slot, not the stale one. - test_query_engine_fixes: retract_cleans_duplicate_avet_tables covers the stale-lookup-ref repro.
1 parent 0c97606 commit ba4e972

3 files changed

Lines changed: 107 additions & 1 deletion

File tree

‎impl/db.ml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,30 @@ let add_datom_to_indexes db datom =
222222
; max_datom_e = max db.max_datom_e datom.e
223223
}
224224

225+
let remove_fact_from_duplicate_tables db active =
226+
(* Retraction removes the fact entirely, so every stored copy — including
227+
the ones kept out of the PSet indexes in the duplicate tables — must go.
228+
The tables are shared with prior db values, so rebuild them rather than
229+
mutate in place. *)
230+
let duplicate_datoms = List.filter (fun d -> not (same_fact d active)) db.duplicate_datoms in
231+
if duplicate_datoms == db.duplicate_datoms then
232+
db
233+
else
234+
let duplicate_aevt_datoms = List.sort (Util.compare_datom Aevt) duplicate_datoms in
235+
let duplicate_avet_datoms =
236+
duplicate_datoms
237+
|> List.filter (fun datom -> Schema.schema_attr_is_avet_accessible db.schema datom.a)
238+
|> List.sort (Util.compare_datom Avet)
239+
in
240+
{ db with
241+
duplicate_datoms
242+
; duplicate_aevt_datoms
243+
; duplicate_avet_datoms
244+
; duplicate_eavt_by_entity = duplicate_eavt_by_entity duplicate_datoms
245+
; duplicate_aevt_by_attr = duplicate_datoms_by_attr duplicate_aevt_datoms
246+
; duplicate_avet_by_attr = duplicate_datoms_by_attr duplicate_avet_datoms
247+
}
248+
225249
let refresh_indexes_with_tx_data db tx_data =
226250
let db =
227251
List.fold_left
@@ -232,6 +256,7 @@ let refresh_indexes_with_tx_data db tx_data =
232256
match find_active_datom_by_fact db datom with
233257
| None -> db
234258
| Some active ->
259+
let db = remove_fact_from_duplicate_tables db active in
235260
{ db with
236261
eavt_index = PSet.remove active db.eavt_index
237262
; aevt_index = PSet.remove active db.aevt_index

‎test/test_melange_transit_backend.ml‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,41 @@ let test_native_backend_converts_edn_values () =
7070
] -> ()
7171
| _ -> failwith "native melange-transit backend should convert EDN maps to Transit maps"
7272

73+
let test_native_backend_read_cache_wraps_at_max_entries () =
74+
(* transit-js caches both writes and reads; both caches wrap at
75+
44*44 = 1936 entries, so ^XX cache codes reuse slots. Encoding
76+
1936+64 distinct cacheable keywords then repeating the last one
77+
emits ^1C — index 1999 wrapped to slot 63, which the read cache
78+
must resolve to the overwritten entry, not the original.
79+
melange-transit 0.1.0 indexed the read cache by Hashtbl.length
80+
without wrapping and decoded it as the stale number-63. *)
81+
let max_cache_entries = 44 * 44 in
82+
let count = max_cache_entries + 64 in
83+
let last = count - 1 in
84+
let kw i = Json.Keyword (Printf.sprintf "long.namespace.attr/number-%d" i) in
85+
let payload = Json.Array (List.init count kw @ [ kw last ]) in
86+
match Json.of_string (Json.to_string payload) with
87+
| Json.Array values ->
88+
let decode i =
89+
match List.nth values i with
90+
| Json.Keyword s -> s
91+
| _ -> failf "entry %d should decode to a keyword" i
92+
in
93+
List.iteri
94+
(fun i _ ->
95+
let expected = Printf.sprintf "long.namespace.attr/number-%d" i in
96+
let actual = decode i in
97+
if not (String.equal expected actual) then
98+
failf "entry %d: expected %S, got %S" i expected actual)
99+
(List.init count Fun.id);
100+
expect_equal "wrapped cache reference"
101+
(Printf.sprintf "long.namespace.attr/number-%d" last)
102+
(decode count)
103+
| _ -> failwith "expected a transit array"
104+
73105
let () =
74106
test_native_backend_decodes_logseq_storage_shape ();
75107
test_native_backend_writes_transit_json ();
76108
test_native_backend_roundtrips_verbose_storage_keys ();
77-
test_native_backend_converts_edn_values ()
109+
test_native_backend_converts_edn_values ();
110+
test_native_backend_read_cache_wraps_at_max_entries ()

‎test/test_query_engine_fixes.ml‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,53 @@ let test_entity_map_lookup_ref_never_resolves_raises () =
543543
then
544544
failf "unexpected error message: %s" msg)
545545

546+
let test_retract_cleans_duplicate_avet_tables () =
547+
(* Bug 5: datoms that compare equal under eavt (same fact stored twice, as
548+
produced by a full index rebuild over restore) are kept out of the PSet
549+
indexes in the duplicate_* tables — and find_avet_exact consults
550+
duplicate_avet_by_attr. Retraction removed the fact from the three PSets
551+
but left the duplicate copy behind, so [:block/uuid u] still resolved
552+
after the entity was retracted. Upstream retracts the fact entirely and
553+
the lookup ref raises "Nothing found for entity id". *)
554+
let u1 = "11111111-1111-1111-1111-111111111111" in
555+
let uuid_datom =
556+
{ e = 1; a = "block/uuid"; v = Uuid u1; tx = 1; added = true }
557+
in
558+
let db =
559+
init_db ~schema:(block_schema ())
560+
[ uuid_datom
561+
; { e = 1; a = "block/title"; v = String "Page"; tx = 1; added = true }
562+
; uuid_datom ]
563+
in
564+
(if List.length db.duplicate_datoms = 0 then
565+
failf "precondition: init_db should route the repeated fact to duplicate_datoms");
566+
let db = db |> db_with [ RetractEntity (Entity_id 1) ] in
567+
(match datoms db Eavt ~a:"block/uuid" () |> List.of_seq with
568+
| [] -> ()
569+
| datoms -> failf "retracted uuid datom still visible in eavt: %d" (List.length datoms));
570+
(match datoms db Avet ~a:"block/uuid" () |> List.of_seq with
571+
| [] -> ()
572+
| datoms -> failf "retracted uuid datom still visible in avet: %d" (List.length datoms));
573+
(try
574+
ignore
575+
(db
576+
|> db_with
577+
[ Entity
578+
{ db_id = None
579+
; attrs =
580+
[ "block/title", One_value (String "Child")
581+
; "block/parent", One_value (lookup_ref "block/uuid" (Uuid u1))
582+
]
583+
}
584+
]);
585+
failf "lookup ref to a retracted entity should raise"
586+
with
587+
| Invalid_argument msg ->
588+
if not
589+
(String.starts_with ~prefix:"Nothing found for entity id" msg)
590+
then
591+
failf "unexpected error message: %s" msg)
592+
546593
(* Bug D: upstream maybe-wrap-multival — inside a multival attr a 2-element
547594
collection is a lookup ref only when its head names a unique-identity attr;
548595
otherwise it expands into individual values *)
@@ -892,4 +939,5 @@ let () =
892939
; "rule_attr_var_property", test_rule_attr_var_property
893940
; "rule_tags_query_with_eid_input", test_rule_tags_query_with_eid_input
894941
; "edn_symbol_special_chars", test_edn_symbol_special_chars
942+
; "retract_cleans_duplicate_avet_tables", test_retract_cleans_duplicate_avet_tables
895943
]

0 commit comments

Comments
 (0)