fix(search)!: give a Reference Type's entries no id - #832
Open
ddeboer wants to merge 1 commit into
Open
Conversation
A Reference Type is nested in its referrer and kept in no collection of its own, so nothing can resolve an entry by key – yet the projection gave every entry an id whenever the graph happened to name the node. Nothing read it. The collection definition declares an id for a locally-nested Root Type alone, so this was a value the projection wrote and the collection never mentioned; a weld names the flat identity companion beside the entry, not the entry; and an entry's own key answers no query a consumer would ask. What it did do was make an entry's shape depend on whether a publisher minted an IRI for a relationship – present on some entries, absent on others, for a reason that means nothing downstream. A Root Type keeps its id whether nested or not: as a root it is the document key, and under a local lookup it is what the lookup resolves against. The rule is about the KIND of type, not about where it is projected. BREAKING CHANGE: a nested entry of a Reference Type no longer carries `id`. In a GraphQL surface the emitted edge type loses its `id` field. Two entries that differed only by the node IRI behind them now deduplicate into one, which is what two statements of the same fact should be.
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.
A Reference Type is nested in its referrer and kept in no collection of its own – ADR 24 says a referent needs no identity, and
schema.tssays it has no document key to declare. The projection said otherwise: every entry got anidwhenever the graph happened to name the node.Nothing read it
nestedFieldsemits anidwhennestedType.class !== undefined– a locally-nested Root Type, whose id is the key its lookup resolves against. For a Reference Type it emits none, so the projection was writing a value the collection does not mention. It survived only because Typesense tolerates an undeclared nested key.creator_id), not the entry's own key.…/work#production-role-1. Two works by the same photographer have different ones, and no two datasets could share one, so it can neither match nor join.What it did do was make an entry's shape depend on whether a publisher minted an IRI for a relationship. Downstream that is arbitrary: in Linked Open Limburg the Role node is a blank node for 159 of Limburgs Museum's 291 creator roles and for all 17 of Discovery Museum's, so more than half of one collection's entries had the field and the rest did not, for a reason no consumer can act on.
The rule is about the kind of type, not the nesting
A Root Type keeps its
idwhether nested or not – as a root it is the document key, and under alocallookup it is what the lookup resolves against. So the guard issearchType.class === undefinedindocumentIdOf, which is exactly the line the collection definition already draws.Worth flagging, because I got this discriminator wrong earlier in #801: there it was used to answer is this being projected as a nested entry, where a locally-nested Root Type made it false and the arity broke. Here the question genuinely is what kind of type is this, and the same test is the right one.
Consequences
idfield.iddropped.Sequencing
Independent of #828, which is a
fixreleasing as 0.25.1. This one is breaking, so it lands as 0.26.0 – worth keeping apart so a consumer can take the phantom-entry fix in #828 without absorbing an API break at the same time.