GraphQL: edge cases (lol)

Moved from GitHub dgraph/5711

Posted by dpeek:

What version of Dgraph are you using?

master

Have you tried reproducing the issue with the latest release?

yes

What is the hardware spec (RAM, OS)?

MacBook PRo 2017 16GB

Steps to reproduce the issue (command/config used to run Dgraph).

type Foo {
  key: String @id;
  bar: Foo;
}
mutation {
  addFoo(input: [{key: "key1", bar: {key: "key2}]) {
    foo {
      key
    }
  }
}
mutation {
  updateFoo(
    input: {
      filter: { key: { eq: "key1" } }
      set: { bar: null }
    }
  ) {
    foo {
      bar {
        key
      }
    }
  }
}

Expected behaviour and actual result.

I would expect this to work by removing an existing edge bar (and the reverse edge if you were doing that too).

It does not.

dpeek commented :

Some more useful examples, tested in slash:

Schema

type Foo {
  name: String! @id
  bar: Bar @ hasInverse(field: foo)
}

type Bar {
  name: String! @id
  foo: Foo @ hasInverse(field: bar)
}

Create initial node with edge

mutation {
  addFoo(input: [{name: "foo", bar: {name: "bar1"}}]) {
    foo {
      name
      bar {
        name
      }
    }
  }
}

Set edge to null doesn’t work

mutation {
  updateFoo(input: {filter: {name: {eq: "foo"}}, set: {bar: null}}) {
    foo {
      bar {
        name
      }
    }
  }
}
{
  "data": {
    "updateFoo": {
      "foo": [
        {
          "bar": {
            "name": "bar1"
          }
        }
      ]
    }
  },
  "extensions": {
    "touched_uids": 9,
    "queryCost": 1
  }
}

Set edge to new node works

mutation {
  updateFoo(input: {filter: {name: {eq: "foo"}}, set: {bar: {name: "bar2"}}}) {
    foo {
      bar {
        name
      }
    }
  }
}
{
  "data": {
    "updateFoo": {
      "foo": [
        {
          "bar": {
            "name": "bar2"
          }
        }
      ]
    }
  },
  "extensions": {
    "touched_uids": 23,
    "queryCost": 1
  }
}

But inverse edges not cleaned up

query {
  queryBar {
    name
    foo {
      name
    }
  }
}
{
  "data": {
    "queryBar": [
      {
        "name": "bar1",
        "foo": {
          "name": "foo"
        }
      },
      {
        "name": "bar2",
        "foo": {
          "name": "foo"
        }
      }
    ]
  },
  "extensions": {
    "touched_uids": 8,
    "queryCost": 1
  }
}

dpeek commented :

Updating edges adds instead of replacing

type Foo {
  name: String! @id
  bars: [Bar!]! @ hasInverse(field: foo)
}

type Bar {
  name: String! @id
  foo: Foo @ hasInverse(field: bars)
}
mutation {
  addFoo(input: [{name: "foo2", bars: [{name: "bar5"}, {name: "bar6"}]}]) {
    foo {
      name
      bars {
        name
      }
    }
  }
}
{
  "data": {
    "addFoo": {
      "foo": [
        {
          "name": "foo2",
          "bars": [
            {
              "name": "bar5"
            },
            {
              "name": "bar6"
            }
          ]
        }
      ]
    }
  },
  "extensions": {
    "touched_uids": 32,
    "queryCost": 1
  }
}
mutation {
  updateFoo(input: {filter: {name: {eq: "foo2"}}, set: {bars: [{name: "bar5"}]}}) {
    foo {
      name
      bars {
        name
      }
    }
  }
}
{
  "data": {
    "updateFoo": {
      "foo": [
        {
          "name": "foo2",
          "bars": [
            {
              "name": "bar5"
            },
            {
              "name": "bar6"
            }
          ]
        }
      ]
    }
  },
  "extensions": {
    "touched_uids": 23,
    "queryCost": 1
  }
}

vardhanapoorv commented :

@dpeek this is the expected behaviour since if we call set in Dgraph the same thing happens. If you replace set with remove it will function the way you expect it. Though I do understand that setting it to null you would expect it to run that way.

dpeek commented :

Understood that this is how dgraph works, but it will be surprising for GraphQL users that don’t know that. It also seems that right now there’s no way to remove an edge from the graphql endpoint?

One suggestion might be to generate separate add{TypeName}{CapitalisedFieldName} / delete{TypeName}{CapitalisedFieldName} mutations in the schema that allow append/delete, and then update should delete and replace?

MichaelJCompton commented :

There’s both set and remove options in the update mutations. So it is possible to both add and remove edges.

This models the way it’s always worked in Dgraph.

There’s a reason to have these two behaviours : say for example I’ve got 1,000,000 followers, and the app wants to add one more follower - does it really need to fetch all 1,000,000 from Dgraph, add the one, and then send back a mutation for 1,000,000+1. That’s why there’s set and remove mutations in Dgraph. Set really only does addition of edges, remove does deletion of edges. That might not be made clear enough in the docs.

Often that correctly models the way an app attaches updates to clickable components in a UI: e.g. you click follow, it just sends an update set mutation for the one new follower. The mutation/UI doesn’t have to know the full current state, it just needs to know about the change it wants to make.

I wonder because GraphQL is another level of abstraction up, if there needs to be a third option for ‘just make it exactly as I have serialized it from my client’. E.g: the client doesn’t want to write code to work out what’s different, what’s been added, what’s been removed. etc. it just wants to make a mutation.

An example could be something like ‘tags’ on a post - the user has clicked around to add and remove some tags as part of editing a post. In the handler for save, the frontend dev doesn’t want to write code to compute the tags added and the tags removed, they just want to say that the post has been edited, now looks like this, please save it in the update mutation. On the backend we can open a transaction, compute the diffs and save as per the Dgraph set and remove primitives.

dpeek commented :

GraphCool used to have addTypeField / removeTypeField and setTypeField / unsetTypeField mutations for this very reason – update would replace all while the the others would append / delete. I totally get what you are saying about aligning with how dgraph works, I’m just saying that’s it’s confusing from a user perspective, particularly for to-one edges.

Right now, to replace an edge my mutations looks like this:

mutation updateTypeField(
  $typeId: String!
  $newEdge: EdgeRef
  $previousEdgeId: String!
) {
  updateCluster(
    input: {
      filter: { typeId: { eq: $typeId } }
      set: { edge: $newEdge }
      remove: { edge: { edgeId: $previousEdgeId } }
    }
  ) {
    numUids
  }
}

MichaelJCompton commented :

Really? If you are just overwriting an existing singular edge/scalar, you should just set it to the new value.

dpeek commented :

Fairly sure (for edges to nodes, not scalars)… see “But inverse edges not cleaned up” above.

I think it is doing what dgraph does and adding an edge, and then whether you get the old the new back is luck of the draw as to which edge is first. Setting it to null certainly doesn’t work…

amaster507 commented :

GraphCool used to have addTypeField / removeTypeField and setTypeField / unsetTypeField mutations for this very reason

That would be handy! I have plenty of use cases for that.