Skip to content

fix(graphql): allow update mutations to set an @id field to its existing value#9702

Open
shiva-istari wants to merge 2 commits into
mainfrom
shiva/graphql-id
Open

fix(graphql): allow update mutations to set an @id field to its existing value#9702
shiva-istari wants to merge 2 commits into
mainfrom
shiva/graphql-id

Conversation

@shiva-istari
Copy link
Copy Markdown
Contributor

@shiva-istari shiva-istari commented May 13, 2026

Description

When a GraphQL update mutation included an id directive field in its set patch with the same value already stored on the node, the resolver incorrectly returned null instead of applying the update.
The rewriter ran an existence query for the id value before execution. When it found a matching UID it assumed a conflict and aborted rewriting it had no way to know at that point whether the existing UID was the same node being updated (no-op, should succeed) or a different node (genuine conflict, should error).

The same early-abort also caused a false-positive error in a second case: when a filter matched no nodes at all, the existence of a matching id directive value elsewhere in the database was being flagged as a conflict even though no mutation ran.
A third related bug: the remove patch was issuing unnecessary existence queries for top-level id fields. Removing a scalar value from a node can never steal uniqueness from another node, so those queries were wasted work and could generate spurious conflict errors.

Fix
Deferred conflict detection a two-phase approach:

  1. Rewrite time: when an id directive value is found to exist during UpdateWithSet, instead of aborting, record a xidConflict{existenceUID, xidField, xidValue, typeName} entry and let the mutation proceed.
  2. Post-execution (VerifyXIDConflicts): after the upsert runs, compare each recorded existenceUID against the UIDs that were actually mutated. If the existence UID is among the mutated UIDs → same node → not a
    conflict, succeed. If it is not → different node → return the conflict error. If mutatedUIDs is empty (filter matched nothing) → no verification needed, return nil.

The new XIDConflictVerifier interface on MutationRewriter keeps the post-execution check opt-in only UpdateRewriter implements it.

Additionally, the remove patch path strips top-level id fields before generating existence queries, and rewriteObject skips id processing entirely for UpdateWithRemove at top level.

@shiva-istari shiva-istari requested a review from a team as a code owner May 13, 2026 11:25
@github-actions github-actions Bot added area/testing Testing related issues area/graphql Issues related to GraphQL support on Dgraph. area/core internal mechanisms go Pull requests that update Go code labels May 13, 2026
@blacksmith-sh

This comment has been minimized.

@matthewmcneely matthewmcneely self-requested a review May 14, 2026 16:28
@matthewmcneely
Copy link
Copy Markdown
Contributor

@shiva-istari Can you add a proper PR description covering the changes in detail? It's not clear looking at the code what the exact problem this is fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core internal mechanisms area/graphql Issues related to GraphQL support on Dgraph. area/testing Testing related issues go Pull requests that update Go code

Development

Successfully merging this pull request may close these issues.

2 participants