GraphQL should allow updates of multiple relational levels

Issues GraphQL
,
1

Moved from GitHub dgraph/5815

Posted by martaver:

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

Using slash graphql, given the schema:

type Player {
  id: ID!
  title: String! @search(by: [fulltext])  
  parent: Player
  children: [Player] @hasInverse(field: parent)
}

Create a tree of Players, with parent/children edges. Let’s add a root Player with two child Players:

mutation {
  addPlayer(input: [
    {
    title: "root",
    children: [
      {
        title: "A"
      },
      {
        title: "B"      
      }
    ]
  }
  ]) {
    player {
    	id
      title
      children {
      	id
        title
    	}
  }
  }
}

Now, I have a UI component in the front-end that can manipulate the tree, and I want to move the Player B to be a child of Player A. It restructured the tree, and gives me a new hierarchy, which I want to push to dgraph.

mutation SetPlayerTree($id: ID!, $children: [PlayerRef]) {
    updatePlayer(input: {
        filter: {
            id: [$id]
        }
        set: {
            children: $children
        }
    }) {
        player {
        id
        children {
            id
            children {
                id
            }
        }
    }
    }
}

With vars, where B has now been moved to be a child of A (replace the ids with whatever you got from your last query):

{
  "id": "0x4e22",
  "children": [{"id": "0x4e25", "children": [{"id": "0x4e26", "children": []}]}]
}

Expected behaviour and actual result.

I would expect the response to return the hierarchy root -> A -> B, however it returns root -> [A, B]:

{
  "data": {
    "updatePlayer": {
      "player": [
        {
          "id": "0x4e22",
          "children": [
            {
              "id": "0x4e25"
            },
            {
              "id": "0x4e26"
            }
          ]
        }
      ]
    }
  },
  "extensions": {
    "touched_uids": 17,
    "queryCost": 1
  }
}
1 Like
2

arijitAD commented :

In the case of mutation, we update the Uids up to one level. But If Uid is not provided then we perform add mutation at all levels.
Hence, in this update, the nested part will be ignored.

{
  "id": "0x4e22",
  "children": [{"id": "0x4e25", "children": [{"id": "0x4e26", "children": []}]}]
}

Actual update.

{
  "id": "0x4e22",
  "children": [{"id": "0x4e25"}]
}

But this will work fine.

{
  "id": "0x4e22",
  "children": [{"id": "0x4e25", "children": [{"title": "C", "children": []}]}]
}

The documentation isn’t clear on this. We will update it.
In order to achieve what you are doing, you need to perform the update on Uid of A(0x4e25) and since you are already using @hasinverse it will delete the link between root and B, and B will be added as children of A.

{
  "id": "0x4e25",
  "children": [{"id": "0x4e26"}]
}
3

martaver commented :

Okay, I understand, that makes sense so far. I’ve actually implemented this the other way around, by setting ‘parent’ on the child node, and dgraph handles that nicely.

I would like to advocate that dgraph should accept changing relationships for more than one level, then. This would be more intuitive, because the add operation also works at multiple levels.

A use case for this is (in our case) where we have a tree editor, where the user can (locally) make a series of structural changes to a taxonomy. The content of each node stays the same but they can drag existing nodes around. Once the user selects save, the component submits the resulting tree structure in JSON format. Keep in mind that in this use case we are talking purely about updating existing nodes.

With dgraph’s current updateXXX resolver, there are two APIs for mutations set and remove. In my case, I need to compare the old tree structure to the new structure, walking each node to create a sequence of diffs - additionals and removals. Then I have to match them by their UIDs to validate that I have detected all the moves in the tree. Each of these, then I send to dgraph as an update to the node’s parent. After each query is done, the tree is updated.

I mention a similar scenario in my other post too: GraphQL doesn't preserve order of edges in a collection. · Issue #5816 · dgraph-io/dgraph · GitHub as an argument for why the updateXXX resolvers should actually have three APIs set, add and remove.

In an ideal world, I would be able to provide the desired structure of any depth and have dgraph update all relations, e.g. for original mutation input:

{
  "id": "0x4e22",
  "children": [{"id": "0x4e25", "children": [{"id": "0x4e26", "children": []}]}]
}

Having to break up such updates into a sequence of operations has a few problems:

  1. It breaks transactionality and concurrency.
  2. Using a front-end state management solution like Apollo Client means that each query forces a UI update, unless I write custom logic to handle mutations to the local cache optimistically.
  3. It forces the front-end to incorporate logic about HOW a request is to be achieved - which is something that should be kept the responsibility of a back-end.

From this perspective, Slash GraphQL is more of a hinderance than a help. For example, why wouldn’t I just implement my own Apollo Server and handle these special cases there, probably leveraging the more powerful syntax of GraphQL+/-?

Except, these aren’t really special cases, we’re talking about everyday problems in front-end development.

To this end, I’m going to rename this issue as a proposal to allow for multi-level updates.

4

arijitAD commented :

Multiple level updates will solve the above use cases you mentioned. You can still use Dgraph’s transaction to achieve the same, and coming to Slash GraphQL, there is a proposal to expose GraphQL+/- as well so that you will be able to use the more powerful syntax. We would consider including multiple level updates in the upcoming release based on user interests.

5

martaver commented :

How can I use dgraph’s transaction via the pure graphql endpoint?

6

@martaver

We don’t have plans to allow multi-level updates for now. The reason that we only keep updates to a single level is to not have unintended consequences while using the API. For example if you had a state which was linked to a country, updating the state shouldn’t allow you to also update the name of the country linked to a state. That should be done through the country’s own update API.

So the rationale here is that allowing you to do updates at multi-levels would have a lot of unintended side-effects and APIs might end up updating data that they are not responsible for. Doing so makes it hard to figure out what data is being mutated by which API.

1 Like
7

I disagree…

The shape of the update is declarative. You declare exactly what you want updated. In the example you provide, it’s perfectly reasonable to request updating the name on a state’s parent country. Why not? That’s exactly the power that a graph API provides… Unlikely, maybe… but reasonable.

The difficulties come in the semantics around whether values are set as is or merged, handling deletes and in particular in handling modifications to collections.

All of these could be determined with decorators.

Without having this kind of flexibility, we can’t make complicated updates in a single request, and being able to navigate relational complexity is supposed to be the primary selling point of using a graph database like dgraph… we don’t even have transaction capabilities in the graphql api!

That leaves one option: to roll our own graphql server in between dgraph and the client. In which case - what’s the point of the graphql API? I might as well use the graphql +/- API then…

I really don’t understand the product positioning around slash graphql - I don’t see how it could be valuable for anything more than basic prototypes unless the existing apis are extended significantly.

8

At some point we are looking into providing support for transactions through the GraphQL API. That along with custom revolvers should help you achieve what you are trying to do here. I’ll leave this issue open to see if other users are also interested it. If there is enough interest, we can look into supporting this.

1 Like
9

That’s true - custom resolvers at the server side would cover most of these scenarios!

Transactionality is the only missing piece, then…