Repository navigation
fix: Isolate insert values and resolve options consistently across insert paths - #102
Merged
Merged
Conversation
InsertMany executed the planned graph directly, so every record of a batch received the same slice, map, or pointer passed through Set, and an Insert callback that mutated it changed the caller's value and the records inserted before it. The batch path now executes a clone of the graph, matching single inserts, so each node owns its values.
… values on insert Binding a pointer primary key to a pointer foreign key copied the pointer itself, so a child Insert that wrote through its FK also rewrote the parent's key in the returned result and in the value Cleanup passed to Delete. Pointer destinations now always receive freshly allocated storage. The executor also captures each node's cleanup value as soon as its Insert returns instead of after the whole graph ran, so later inserts cannot change which rows cleanup removes.
The insert entry points pulled WithContext out of the top-level options before traits were expanded, so a WithContext inside an InlineTrait or BlueprintTrait was ignored and a canceled context still inserted records. The context is now taken from the expanded root options, and Plan.InsertE honors a WithContext given at build time over its ctx argument, so single, batch, and plan inserts share one rule.
InsertMany resolved Seq, SeqRef, and SeqUse before traits were expanded, so a sequence defined in a blueprint trait reached validation unresolved and failed with "only supported by InsertMany", and a trait returned by SeqRef was left for a later pass. Trait expansion now resolves sequence options for the current record in the same pass, so they behave the same whether written directly, in an inline trait, in a blueprint trait, or under Ref.
Set options were collected into a map and applied in map iteration order, so setting an embedded struct and a field promoted from it in the same call produced either value at random. Sets are now kept as an ordered list and applied in the order written, so the later Set wins the same way later options override earlier ones elsewhere.
A child generated through has-many binds its belongs-to relation back
to the parent that created it, and the planner returned as soon as that
binding succeeded. A Use or Ref given for the same relation, such as
Ref("employees", Use("department", d)), passed validation and was then
silently dropped. The structural binding is kept and such an option is
now reported as ErrInvalidOption, since honoring it would mean
replacing or rebuilding the parent that already exists.
NodeAt and NodesForRoot followed only the dependencies of a root's own nodes, so when a batch shared a belongs-to parent, the has-many children generated under that parent belonged to no root and both lookups returned nothing for them. The lookup now also follows the dependents planned under each selected node, identified by their node ID, so a shared parent brings its generated subtree while nodes of other roots that merely reference it stay out.
…kups Explain that options resolve in one trait expansion pass on every insert path, that Set applies in written order, that each record owns the values it is built from, that Use and Ref on a has-many back-reference are rejected, and that root lookups include records generated under shared parents.
With the context now read from the expanded options, a nil WithContext looked the same as no WithContext and the caller's ctx was used instead. A nil WithContext again runs with context.Background(), so it overrides the caller's ctx like any other WithContext.
Code Metrics Report
Details | | main (9a2b1f6) | #102 (6d0f4d6) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 86.9% | 86.9% | +0.0% |
| Files | 59 | 59 | 0 |
| Lines | 4791 | 4784 | -7 |
- | Covered | 4165 | 4160 | -5 |
+ | Code to Test Ratio | 1:1.0 | 1:1.0 | +0.0 |
| Code | 17787 | 17800 | +13 |
+ | Test | 17916 | 18365 | +449 |
+ | Test Execution Time | 1m11s | 55s | -16s |Code coverage of files in pull request scope (82.5% → 82.6%, patch 86.6%)
Reported by octocov |
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
WithContextandSeq/SeqRef/SeqUsework inside traits, andSetapplies in the order writtenUse/Refon a has-many back-reference is rejected instead of silently dropped, andNodeAt/NodesForRootinclude has-many children generated under shared parentsChanges
InsertManyexecutes a clone of the planned graph, likePlan.InsertE, so an Insert that mutates aSetslice, map, or pointer no longer changes the caller's value or other records**T) copies the referenced storage, so a child Insert writing through its FK cannot rewrite the parent's keyDeletereceivesWithContextinsideInlineTrait/BlueprintTraitis honored, andPlan.InsertElets a build-timeWithContextoverride its ctx argumentSeq,SeqRef, andSeqUseare resolved per record during trait expansion, so they work in blueprint traits and in traits returned bySeqRefSetwinsUse/Refon that relation returnsErrInvalidOptionWithContextruns withcontext.Background()instead of falling back to the caller's ctx