fix(search-typesense): replace what a local lookup stated with the resolved document - #819
Merged
Conversation
…solved document - A resolved local lookup used to spread the target’s record over the stored copy, per physical field, so a name stated here in one language survived beside the target’s name in another, and a field the target’s record lacks survived after the target dropped it - Return the resolved document instead, so a resolved entry says exactly what the projection asked for, as a plain lookup does; the stored copy still serves the identified-but-not-indexed and unidentified cases - Reword the ‘local’ contract and the docs from ‘overlays’ to ‘replaces’ - Widen the test fixture so the stored copy and the target’s record disagree per physical field
… stored copy - Reword the comments in the local lookup tests and ADR 24 that still described the overlay - Use typographer’s apostrophes in the added prose
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.
Fix #816
A
lookupreference withlocal: truestores what the referring document states about the referent, andlocalLookupValuein@lde/search-typesensecombined the resolved document with that copy at query time. The combination was a spread over the target type’s physical fields, and a language-keyed text field fans out into several of those. So aname_undthe referring document stated survived beside the target’sname_nl, and a resolved reference answered two names where the document it points to answers one. The same gap let any field the target’s record does not carry survive on the reference after the target dropped it.Change
Once the lookup found the target’s document, return that document instead of merging it over the stored copy. The three cases the contract on
ReferenceStrategy.localalready describes are now literal:id;id.The per-logical-field merge from the issue was rejected: it fixes the name but still leaks fields the target lacks.
What stays the same
Nothing changes at index time. The copy is still stored unconditionally, because at extraction the question is is this referent identified, while only at query time is it is that document indexed. The stored shape is unchanged, so no reindex is needed, and the copy remains available should a surface want to show what the referring dataset says about the referent.
The wording of the
localcontract in@lde/search, the reference docs, ADR 24 and the test comments changes from “overlays” to “replaces”. The test fixture now stores the copy in another language than the target’s record, plus a field that record lacks, so the old spread fails the test.Follow-up
#818 records the redesign discussed while fixing this: dropping the
localflag so every lookup stores the copy, with the_idcompanion serving facet and join as well as filter.