Repository navigation
Resolve lookup-refs via bounded AVET seeks, not full-index scans - #36
Merged
Merged
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Lookup_refs.entity_id materialized visible_datoms and scanned the whole list for the unique attr/value pair. Route it through the context's entid function (entid_db -> find_avet_exact), which performs a bounded AVET slice on the (attr, value) prefix plus the duplicate table. The context now carries entid instead of visible_datoms; entity_id_in_datoms keeps its list semantics for staged transaction datoms. find_avet_exact now applies db.filter_pred so the bounded path preserves the old visible_datoms filter behavior on filtered dbs (and restores upstream parity for entid on filtered dbs). Removes dead Transact_datoms.entid (no callers) and the visible_datoms context requirement.
devin-ai-integration
Bot
force-pushed
the
devin/lookup-ref-avet
branch
from
October 5, 2026 07:09
8482a3d to
19c95d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lookup_refs.entity_idresolved lookup-refs by materializingvisible_datoms(a whole-EAVT materialization +filter_predpass) and then linearly scanning that list for the unique attr/value pair — a full-index scan per resolution. This routes direct resolution through boundedAVETseeks instead, matching upstream DataScript's indexed lookup.Changes:
Lookup_refs.contextreplaces thevisible_datoms : db -> datom listfield withentid : db -> attr -> value -> entity_id option;entity_idresolves through it (context.entidis wired toentid_db→find_avet_exact, a boundedPSet.sliceon the(a, v)prefix plusduplicate_avet_by_attr). A sharedentity_id_of_resolutionhelper factors theis_uniquecheck +strict_missingerror path, soentity_idandentity_id_in_datomsproduce identical results/error messages.entity_id_in_datomsstill scans the supplied datom list (entid_in_datomsoverdatoms), which is what staged-transaction paths call. This change is resolution-only.find_avet_exactnow appliesdb.filter_predto its bounded result set. This preserves the oldvisible_datomsfilter behavior on filtered dbs (upstreamFilteredDBalso filters-datoms), and restores pre-5e3238aparity forentidon filtered dbs.Transact_datoms.Make'svisible_datomscontext requirement and itsentidhelper (zero callers after5e3238a), plus the now-unusedvisible_datomsalias indatascript.ml.Before/after measurement (20k-block outliner db, 100k datoms, native exe on this VM — scratch bench, not committed):
entity_idviavisible_datomsscan~1400× faster; scan range drops from the full 100k-datom index to a bounded attribute slice (~1 datom, unique attr). The new
entity_idnow costs the same asDatascript.entid(~2.3 µs), which already usedfind_avet_exact.Tests: all 34 native test suites pass (
test_lookup_refscovers both the unresolved-lookup-ref and non-unique-attr error paths).dune build impl/green.docs/upstream_differences.mdupdated: the "Direct Lookup Refs Scan Visible Datoms" divergence is now resolved.Link to Devin session: https://app.devin.ai/sessions/7705343425064c3a9b38d8b0e4ceb045
Open in Devin Desktop: https://app.devin.ai/desktop/session/7705343425064c3a9b38d8b0e4ceb045?variant=devin
Requested by: @tiensonqin