fix: transact forward references resolve graph endpoint ints at execute time
transact() promises atomic forward references — add an entity and relate to
it in one batch — but the planner resolved relationship endpoint ints at
PLAN time, before the batch's add operations had applied. Asking the id
mapper about an entity that does not exist yet made the (correctly strict)
native mapper throw in EntityIdMapper.getOrAssign, so
transact([{op:'add', id:X}, {op:'relate', to:X}]) failed on native
deployments; the permissive JS mapper masked the bug and silently leaked an
id assignment whenever a batch was later rejected by a commit precondition
(normal control flow since the conditional-commit CAS landed).
Make endpoint resolution lazy: AddToGraphIndexOperation and
RemoveFromGraphIndexOperation take VerbEndpointInts — an eager
{sourceInt, targetInt} or a thunk evaluated when the operation EXECUTES,
mirroring the constructor's already-lazy generationFn. The four
transact-planner sites (relate, its bidirectional reverse edge, remove's
relationship cascade, unrelate) pass thunks, so resolution happens inside
the commit after same-batch adds have applied; the seven single-operation
sites keep eager objects (their endpoints provably pre-exist — relate
validates both, unrelate/updateRelation/remove read the existing verb).
The remove operation's rollback captures the ints resolved at execute so
its re-add uses the same mappings.
Byproduct fix: a rejected batch no longer pollutes the id mapper — the
regression suite pins this (mapper has no assignment for the phantom
entity after a precondition-rejected forward-ref batch), alongside the
exact reported shape, both-endpoints-in-batch, bidirectional,
add+relate+remove in one batch, and the split-transact control
(tests/integration/transact-forward-ref-graph.test.ts).
This commit is contained in:
parent
3688d5ce88
commit
a175406497
3 changed files with 202 additions and 29 deletions
|
|
@ -3024,7 +3024,7 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
// rollback can re-add through the BigInt addVerb contract)
|
||||
const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb)
|
||||
tx.addOperation(
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration)
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, { sourceInt, targetInt }, this.graphWriteGeneration)
|
||||
)
|
||||
// Delete verb metadata
|
||||
tx.addOperation(
|
||||
|
|
@ -3734,7 +3734,7 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
// Operation 3: Add to graph index for O(1) lookups
|
||||
tx.addOperation(
|
||||
new AddToGraphIndexOperation(
|
||||
this.graphIndex, verb, sourceInt, targetInt,
|
||||
this.graphIndex, verb, { sourceInt, targetInt },
|
||||
this.graphWriteGeneration,
|
||||
(verbInt) => this.cacheVerbInt(verbInt, id)
|
||||
)
|
||||
|
|
@ -3772,7 +3772,7 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
// Operation 6: Add reverse relationship to graph index
|
||||
tx.addOperation(
|
||||
new AddToGraphIndexOperation(
|
||||
this.graphIndex, reverseVerb, targetInt, sourceInt,
|
||||
this.graphIndex, reverseVerb, { sourceInt: targetInt, targetInt: sourceInt },
|
||||
this.graphWriteGeneration,
|
||||
(verbInt) => this.cacheVerbInt(verbInt, reverseId)
|
||||
)
|
||||
|
|
@ -3854,7 +3854,7 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
if (verb && endpointInts) {
|
||||
tx.addOperation(
|
||||
new RemoveFromGraphIndexOperation(
|
||||
this.graphIndex, verb, endpointInts.sourceInt, endpointInts.targetInt, this.graphWriteGeneration
|
||||
this.graphIndex, verb, endpointInts, this.graphWriteGeneration
|
||||
)
|
||||
)
|
||||
}
|
||||
|
|
@ -4006,12 +4006,12 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
if (typeChanged && reindexInts) {
|
||||
tx.addOperation(
|
||||
new RemoveFromGraphIndexOperation(
|
||||
this.graphIndex, existing, reindexInts.sourceInt, reindexInts.targetInt, this.graphWriteGeneration
|
||||
this.graphIndex, existing, reindexInts, this.graphWriteGeneration
|
||||
)
|
||||
)
|
||||
tx.addOperation(
|
||||
new AddToGraphIndexOperation(
|
||||
this.graphIndex, verbForIndex, reindexInts.sourceInt, reindexInts.targetInt,
|
||||
this.graphIndex, verbForIndex, reindexInts,
|
||||
this.graphWriteGeneration,
|
||||
(verbInt) => this.cacheVerbInt(verbInt, params.id)
|
||||
)
|
||||
|
|
@ -6634,7 +6634,7 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
for (const verb of allVerbs) {
|
||||
const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb)
|
||||
tx.addOperation(
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration)
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, { sourceInt, targetInt }, this.graphWriteGeneration)
|
||||
)
|
||||
tx.addOperation(
|
||||
new DeleteVerbMetadataOperation(this.storage, verb.id)
|
||||
|
|
@ -9014,9 +9014,12 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
}
|
||||
plan.operations.push(new DeleteNounMetadataOperation(this.storage, id))
|
||||
for (const verb of cascade.values()) {
|
||||
const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb)
|
||||
plan.operations.push(
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration),
|
||||
// Endpoint ints resolve at EXECUTE time — a cascade verb (or its
|
||||
// endpoints) may have been created earlier in this same batch, so a
|
||||
// plan-time resolution would ask the id mapper about entities that do
|
||||
// not exist yet (the native mapper rightly refuses).
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, () => this.resolveVerbEndpointInts(verb), this.graphWriteGeneration),
|
||||
new DeleteVerbMetadataOperation(this.storage, verb.id)
|
||||
)
|
||||
plan.touchedVerbs.push(verb.id)
|
||||
|
|
@ -9163,8 +9166,6 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
data: params.data,
|
||||
createdAt: now
|
||||
}
|
||||
const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb)
|
||||
|
||||
plan.operations.push(
|
||||
new SaveVerbOperation(this.storage, {
|
||||
id,
|
||||
|
|
@ -9175,7 +9176,11 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
targetId: params.to
|
||||
}),
|
||||
new SaveVerbMetadataOperation(this.storage, id, verbMetadata),
|
||||
new AddToGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration, (verbInt) =>
|
||||
// Endpoint ints resolve at EXECUTE time — after any same-batch add of
|
||||
// an endpoint has applied. Plan-time resolution was a consumer-reported
|
||||
// native/JS parity bug: transact([add X, relate →X]) asked the native
|
||||
// id mapper to assign an int for an entity that did not exist yet.
|
||||
new AddToGraphIndexOperation(this.graphIndex, verb, () => this.resolveVerbEndpointInts(verb), this.graphWriteGeneration, (verbInt) =>
|
||||
this.cacheVerbInt(verbInt, id)
|
||||
)
|
||||
)
|
||||
|
|
@ -9203,9 +9208,9 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
...verb,
|
||||
id: reverseId,
|
||||
sourceId: params.to,
|
||||
targetId: params.from,
|
||||
sourceInt: targetInt,
|
||||
targetInt: sourceInt
|
||||
targetId: params.from
|
||||
// sourceInt/targetInt are mirrored onto this object when the graph
|
||||
// operation resolves endpoints at execute time.
|
||||
}
|
||||
plan.operations.push(
|
||||
new SaveVerbOperation(this.storage, {
|
||||
|
|
@ -9217,7 +9222,7 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
targetId: params.from
|
||||
}),
|
||||
new SaveVerbMetadataOperation(this.storage, reverseId, verbMetadata),
|
||||
new AddToGraphIndexOperation(this.graphIndex, reverseVerb, targetInt, sourceInt, this.graphWriteGeneration, (verbInt) =>
|
||||
new AddToGraphIndexOperation(this.graphIndex, reverseVerb, () => this.resolveVerbEndpointInts(reverseVerb), this.graphWriteGeneration, (verbInt) =>
|
||||
this.cacheVerbInt(verbInt, reverseId)
|
||||
)
|
||||
)
|
||||
|
|
@ -9259,9 +9264,10 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
: (state.verbs.get(id) ?? (await this.storage.getVerb(id)))
|
||||
|
||||
if (verb) {
|
||||
const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb)
|
||||
plan.operations.push(
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration)
|
||||
// Endpoint ints resolve at EXECUTE time — the verb (or its endpoints)
|
||||
// may have been created earlier in this same batch (forward refs).
|
||||
new RemoveFromGraphIndexOperation(this.graphIndex, verb, () => this.resolveVerbEndpointInts(verb), this.graphWriteGeneration)
|
||||
)
|
||||
}
|
||||
plan.operations.push(new DeleteVerbMetadataOperation(this.storage, id))
|
||||
|
|
|
|||
|
|
@ -176,14 +176,36 @@ export class RemoveFromMetadataIndexOperation implements Operation {
|
|||
* generation is reused for the rollback removal, so an add and its undo
|
||||
* reference one watermark in a provider's per-generation edge chain.
|
||||
*/
|
||||
/**
|
||||
* A verb's interned endpoint ints — eager (already resolved), or a thunk
|
||||
* evaluated when the operation EXECUTES. The lazy form exists for
|
||||
* `transact()` forward references: a relate whose endpoint is added in the
|
||||
* SAME batch cannot resolve ints at plan time (the entity does not exist yet
|
||||
* — a strict native id mapper rightly refuses to assign, and even a permissive
|
||||
* one would leak the assignment if the batch is rejected at precommit).
|
||||
* Deferring to execute time resolves after the batch's add operations have
|
||||
* applied, mirroring how `generationFn` is already evaluated lazily.
|
||||
*/
|
||||
export type VerbEndpointInts =
|
||||
| { sourceInt: bigint; targetInt: bigint }
|
||||
| (() => { sourceInt: bigint; targetInt: bigint })
|
||||
|
||||
/** Resolve a {@link VerbEndpointInts} at execution time. */
|
||||
function resolveEndpointInts(
|
||||
endpoints: VerbEndpointInts
|
||||
): { sourceInt: bigint; targetInt: bigint } {
|
||||
return typeof endpoints === 'function' ? endpoints() : endpoints
|
||||
}
|
||||
|
||||
export class AddToGraphIndexOperation implements Operation {
|
||||
readonly name = 'AddToGraphIndex'
|
||||
|
||||
/**
|
||||
* @param index - The graph-index provider (JS baseline or native).
|
||||
* @param verb - The verb to index (`sourceInt`/`targetInt` mirrored on it).
|
||||
* @param sourceInt - The source entity's interned int.
|
||||
* @param targetInt - The target entity's interned int.
|
||||
* @param endpointInts - The endpoints' interned ints — eager, or a thunk
|
||||
* evaluated at execute time (REQUIRED for transact forward references;
|
||||
* see {@link VerbEndpointInts}).
|
||||
* @param generationFn - Resolves the commit generation to stamp this edge at,
|
||||
* evaluated when the operation executes (see class note).
|
||||
* @param onVerbInt - Optional hook invoked with the interned verb int
|
||||
|
|
@ -192,17 +214,18 @@ export class AddToGraphIndexOperation implements Operation {
|
|||
constructor(
|
||||
private readonly index: GraphIndexProvider,
|
||||
private readonly verb: GraphVerb,
|
||||
private readonly sourceInt: bigint,
|
||||
private readonly targetInt: bigint,
|
||||
private readonly endpointInts: VerbEndpointInts,
|
||||
private readonly generationFn: () => bigint,
|
||||
private readonly onVerbInt?: (verbInt: bigint) => void
|
||||
) {}
|
||||
|
||||
async execute(): Promise<RollbackAction> {
|
||||
// Stamp this edge at the in-flight commit generation; reuse it for the
|
||||
// rollback so add + undo reference the same watermark.
|
||||
// rollback so add + undo reference the same watermark. Endpoint ints
|
||||
// resolve HERE — after any same-batch adds have applied.
|
||||
const generation = this.generationFn()
|
||||
const verbInt = await this.index.addVerb(this.verb, this.sourceInt, this.targetInt, generation)
|
||||
const { sourceInt, targetInt } = resolveEndpointInts(this.endpointInts)
|
||||
const verbInt = await this.index.addVerb(this.verb, sourceInt, targetInt, generation)
|
||||
this.onVerbInt?.(verbInt)
|
||||
|
||||
// Return rollback action
|
||||
|
|
@ -231,28 +254,32 @@ export class RemoveFromGraphIndexOperation implements Operation {
|
|||
/**
|
||||
* @param index - The graph-index provider (JS baseline or native).
|
||||
* @param verb - The verb being removed (required for rollback re-add).
|
||||
* @param sourceInt - The source entity's interned int (rollback re-add).
|
||||
* @param targetInt - The target entity's interned int (rollback re-add).
|
||||
* @param endpointInts - The endpoints' interned ints for the rollback
|
||||
* re-add — eager, or a thunk evaluated at execute time (required when the
|
||||
* verb or its endpoints were created in the SAME transact batch; see
|
||||
* {@link VerbEndpointInts}).
|
||||
* @param generationFn - Resolves the commit generation for this removal,
|
||||
* evaluated when the operation executes.
|
||||
*/
|
||||
constructor(
|
||||
private readonly index: GraphIndexProvider,
|
||||
private readonly verb: GraphVerb, // Required for rollback
|
||||
private readonly sourceInt: bigint,
|
||||
private readonly targetInt: bigint,
|
||||
private readonly endpointInts: VerbEndpointInts,
|
||||
private readonly generationFn: () => bigint
|
||||
) {}
|
||||
|
||||
async execute(): Promise<RollbackAction> {
|
||||
// Resolve the removal generation once; reuse it for the rollback re-add.
|
||||
// Endpoint ints resolve HERE (after any same-batch adds applied) and are
|
||||
// captured for the rollback, whose re-add must use the same mappings.
|
||||
const generation = this.generationFn()
|
||||
const { sourceInt, targetInt } = resolveEndpointInts(this.endpointInts)
|
||||
await this.index.removeVerb(this.verb.id, generation)
|
||||
|
||||
// Return rollback action
|
||||
return async () => {
|
||||
// Re-add verb with original data
|
||||
await this.index.addVerb(this.verb, this.sourceInt, this.targetInt, generation)
|
||||
await this.index.addVerb(this.verb, sourceInt, targetInt, generation)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue