Allow node reference replacements on 1-to-1 relationships

[jostillmanns]

Experience Report

We are building a large Saas solution powered by Dgraph. Naturally, we encountered several cases of 1-to-1 relationships that we needed to represent. Therefore we were quite excited when Dgraph announced native support for 1-to-1 relationships with the release of version 1.1 . As soon as the release was published we started migrating our software to the new version. During the migration we also tried out the new 1-to-1 feature and found that it was already a big help. However, we noticed, that when replacing a node reference, as in, for instance giving a User a different Team, an error was returned, warning us about either making the respective predicate a [uid] or removing the reference prior to the actual mutation. We discussed this issue in the discussion board and learned why this is the case. While we agree on the concerns expressed in the discussion, we still think simply replacing the node reference would be the expected behaviour and would make our code simpler.

Here is a test case that expresses the behaviour we would like to see:

package main

import (
	"context"
	"encoding/json"
	"fmt"
	"testing"

	"github.com/dgraph-io/dgo/v2"
	"github.com/dgraph-io/dgo/v2/protos/api"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
	"google.golang.org/grpc"
)

func Test_it_replaces_uid_predicates(t *testing.T) {
	conn, err := grpc.Dial("localhost:9080", grpc.WithInsecure())
	if err != nil {
		t.Fatalf("conntect: %v", err)
	}

	dg := dgo.NewDgraphClient(api.NewDgraphClient(conn))
	defer dg.Alter(context.Background(), &api.Operation{DropAll: true})

	err = dg.Alter(context.Background(), &api.Operation{
		Schema: `
type Node {
  name: string
  child: uid
}

name: string .
child: uid .
`,
	})
	require.NoError(t, err)

	type Node struct {
		UID  string `json:"uid"`
		Type string `json:"dgraph.type"`

		Name  string `json:"name"`
		Child *Node  `json:"child"`
	}

	in := Node{
		UID:  "_:parent",
		Type: "Node",
		Name: "parent",
		Child: &Node{
			UID:  "_:child",
			Type: "Node",
			Name: "child",
		},
	}

	js, err := json.Marshal(in)
	require.NoError(t, err)

	res, err := dg.NewTxn().Mutate(context.Background(), &api.Mutation{CommitNow: true, SetJson: js})
	require.NoError(t, err)

	parentUID := res.GetUids()["parent"]

	update := Node{
		UID:  res.GetUids()["parent"],
		Type: "Node",
		Child: &Node{
			UID:  "_:child",
			Type: "Node",
			Name: "child replacement",
		},
	}

	js, err = json.Marshal(update)
	require.NoError(t, err)

	res, err = dg.NewTxn().Mutate(context.Background(), &api.Mutation{CommitNow: true, SetJson: js})
	assert.NoError(t, err)

	q := `
query {
  n(func: uid(%s)) {
    uid
    name
    child {
      uid
      name
    }
  }
}
`

	queryRes, err := dg.NewReadOnlyTxn().Query(context.Background(), fmt.Sprintf(q, parentUID))
	require.NoError(t, err)

	var actual map[string][]Node
	err = json.Unmarshal(queryRes.GetJson(), &actual)
	require.NoError(t, err)

	require.Len(t, actual["n"], 1)
	assert.Equal(t, "parent", actual["n"][0].Name)
	assert.Equal(t, "child replacement", actual["n"][0].Child.Name)
}

Any external references to support your case

Discussion board thread: https://discuss.dgraph.io/t/unexpected-bahaviour-when-mutating-1-to-1-relations/5180

[campoy]

Thanks for the issue, @jostillmanns

It does make a lot of sense to make predicates of type uid behave more like predicates of type string, so I would expect this not to be an issue.

In order to avoid surprises, I would propose the following schedule:

• 1.1.x: behavior off by default but enabled via flags or env vars
• 1.2.0: behavior on by default but disabled via flags or env vars
• 1.3.0: behavior deprecated and code removed from our codebase

This would allow you to simply your code as soon as possible while making sure we have time to communicate the change in behavior widely.

Would that work for you?

[campoy]

Hey @martinmr, is this already in a sprint? If not, please do not work on it yet.

[martinmr]

not working on it. Just assigned it to myself since I wrote the original warning.

[jostillmanns]

@campoy the schedule seems reasonable. Thank you.

[martinmr]

Fixed by #4411

@manishrjain said there was no need for the flag. The version of dgraph after 1.1.1 will allow overwriting uid values by default.