Repository navigation
Resolve lazy values when bulk updating - #35
Open
kac-axelor wants to merge 1 commit into
Open
kac-axelor wants to merge 1 commit into
kac-axelor wants to merge 1 commit into
Conversation
- Bulk updates now write text, json and binary fields correctly instead
of storing an empty object for them.
- A lazy value read from one record can be passed straight into
updateAll, which previously stored "{}".
- Binary fields can be bulk updated; before they failed outright.
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.
updateAllnever resolved lazy values.createandupdateroute everynon-relation value through
resolveLazy;updateAllusedvalueOrIDalone,so a promise reached the query builder and pg serialized it as
{}:Nothing warns you:
TextisPromise<string | null>, so the promise is thedeclared type for a bulk set and the round trip above typechecks with no cast.
Text and json were silently overwritten with
{}; binary failed outright withinvalid input syntax for type oid: "{}".createAllis unaffected — it builds rows with#handleCreateand inherits theresolution.
Non-relations now go through
resolveLazy, which also creates the large objectfor
oidcolumns — so binary bulk updates work instead of erroring. ConfiningvalueOrIDto relations additionally stops it collapsing a json value thatcarries an
idkey down to just that id.Tests
Five e2e tests in the bulk suite; three fail on
dev:dev({})updateAll— fails ondev({})null— passes ondev, pins the working pathdev(oid syntax error)Notes
Bulk-updating a binary column points every matched row at one large object,
where
update()gives each row its own. Safe under the ORM, since every writeallocates a fresh oid — rewriting one row repoints only that row, which the
last test pins — and under
vacuumlo, which unlinks by reference scan.A no-match bulk update still creates its large object. One instance of a wider
gap: nothing here ever reclaims a large object, on any path, and
lo_unlinkisitself broken (
src/client/fields/lob.ts:37). Its own change.Left out:
updateAllnever callsrejectProtectedFields, sointernalandreadonlyfields can be written in bulk. Pre-existing, orthogonal, andtightening it is a behaviour change that deserves its own PR.