Repository navigation
fix(model): honor document.id in upsert()'s no-match insert - #149
Merged
Merged
Conversation
upsert() with a condition serialized the model with force_dict=True, so a set document.id was dropped on the no-match insert: the document landed at a fresh id and document.id was then overwritten with it, destroying even the evidence that the caller asked for a specific id (#120). The same instance produced two different outcomes depending on whether a condition was passed. The no-match insert now honors document.id exactly as insert() does: a set id says "put it here", so the document is inserted at that id, and a taken id raises DocumentAlreadyExistsError instead of silently duplicating the document elsewhere. Only id=None still gets a fresh id, and the condition's own id value is still never adopted. A matching condition still wins over a preset id (pinned by a test). Delegating to Table.upsert() cannot express this: handed a Document and a cond, TinyDB extracts the doc_id and passes it to Table.update(document, cond, doc_ids=[doc_id]), which checks doc_ids before cond — the condition is never evaluated. The non-id-condition branch therefore inlines upstream's update-then-insert through the public Table.update()/Table.insert(); the id-condition branch keeps its atomic write cycle and now passes the id-bearing Document through to the fallback insert. Both branches translate the taken-id ValueError via the new _upsert_insert() helper. Friction recorded on the Upstream Limitations page and tracked as #148. Assisted-by: Claude:claude-fable-5
✅ Deploy Preview for tinydantic-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Closes #120
What
upsert()with a condition silently discarded a setdocument.id: the no-match insert landed at a fresh id and then overwrotedocument.idwith it, destroying even the evidence that the caller asked for a specific id. The same instance produced two different outcomes depending on whether a condition was passed.The no-match insert now honors
document.idexactly asinsert()does — a set id says "put it here" — and a taken id raisesDocumentAlreadyExistsErrorinstead of silently duplicating the document elsewhere. Onlyid=Nonestill gets a fresh id, the condition's own id value is still never adopted, and a matching condition still wins over a preset id (now pinned by a test).How
Delegating to
Table.upsert()cannot express this: handed aDocumentand a cond, TinyDB extracts thedoc_idand callsTable.update(document, cond, doc_ids=[doc_id]), which checksdoc_idsbeforecond— the condition is never evaluated. The non-id-condition branch therefore inlines upstream's update-then-insert through the publicTable.update()/Table.insert(); the id-condition branch keeps its atomic write cycle and passes the id-bearingDocumentthrough to the fallback insert. Both branches translate the taken-idValueErrorvia the new_upsert_insert()helper.Per the upstream-friction policy, the
Table.upsert()behavior is recorded on the Upstream Limitations page and tracked as #148 (public-API workaround; no private-API registry entry needed).Docs
upsert()docstring: documents the id-honoring insert and the newRaises:entry.DocumentAlreadyExistsErrorexample.Verification
[1] != [7],DID NOT RAISE).poe test: 3990 passed.poe test-matrix: green on Python 3.10–3.14, exit 0.poe check(ruff, sbom, cspell, mypy, pyright) and strictpoe docs-build: clean.🤖 Generated with Claude Code