Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List value returned for a non-list uid predicate #4879

Closed
abhimanyusinghgaur opened this issue Mar 3, 2020 · 3 comments · Fixed by #4883
Closed

List value returned for a non-list uid predicate #4879

abhimanyusinghgaur opened this issue Mar 3, 2020 · 3 comments · Fixed by #4883
Labels
kind/bug Something is broken. status/accepted We accept to investigate/work on it.

Comments

@abhimanyusinghgaur
Copy link
Contributor

What version of Dgraph are you using?

latest:master

Have you tried reproducing the issue with the latest release?

Yes

What is the hardware spec (RAM, OS)?

doesn't matter

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

  • Run dgraph zero
  • Run dgraph alpha
  • Set following dgraph schema:
uuid: string @index(hash) .
author: uid .
post: [uid] .

type Post {
   uuid
   author
}

type Author {
	uuid
	post
}
  • That means, a Post can have only one Author, while an Author can have multiple Posts.
  • Perform following mutation:
{
	"set": {
			"uid": "_:Post1",
			"uuid": "Post1",
			"dgraph.type": "Post",
			"author": {
				"uid": "_:Author1",
				"uuid": "Author1",
				"dgraph.type": "Author"
			}
		}
}
  • Now, perform following query, to see what mutation did:
query {
    q1(func: eq(dgraph.type, "Post")) {
    	uid
    	uuid
    	author {
    		uid
    		uuid
    	}
    }
  }
  • Result will be something like this (and that is fine):
{
    "data": {
        "q1": [
            {
                "uid": "0x2",
                "uuid": "Post1",
                "author": {
                    "uid": "0x1",
                    "uuid": "Author1"
                }
            }
        ]
    },
    "extensions": {
        "server_latency": {
            "parsing_ns": 68739,
            "processing_ns": 2970060,
            "encoding_ns": 19707,
            "assign_timestamp_ns": 600806,
            "total_ns": 3744032
        },
        "txn": {
            "start_ts": 12
        },
        "metrics": {
            "num_uids": {
                "author": 1,
                "dgraph.type": 0,
                "post": 1,
                "uid": 2,
                "uuid": 2
            }
        }
    }
}
  • Let's do one more mutation, to set the author for Post1 to a new author:
{
	"set": {
			"uid": "0x2",
			"author": {
				"uid": "_:Author2",
				"uuid": "Author2",
				"dgraph.type": "Author"
			}
		}
}
  • Again, perform the query to see results:
query {
    q1(func: eq(dgraph.type, "Post")) {
    	uid
    	uuid
    	author {
    		uid
    		uuid
    	}
    }
  }

Expected behaviour and actual result.

  • Expected behaviour:
{
    "data": {
        "q1": [
            {
                "uid": "0x2",
                "uuid": "Post1",
                "author": {
                    "uid": "0x3",
                    "uuid": "Author2"
                }
            }
        ]
    },
    ...
}
  • Actual Results:
{
    "data": {
        "q1": [
            {
                "uid": "0x2",
                "uuid": "Post1",
                "author": {
                    "uid": "0x1",
                    "uuid": "Author1",
                    "uid": "0x3",
                    "uuid": "Author2"
                }
            }
        ]
    },
    ...
}

Post1 should have Author2 as its only author, but it gives both Author1 and Author2 .

@abhimanyusinghgaur abhimanyusinghgaur added the kind/bug Something is broken. label Mar 3, 2020
@abhimanyusinghgaur
Copy link
Contributor Author

abhimanyusinghgaur commented Mar 3, 2020

@lgalatin lgalatin added the status/accepted We accept to investigate/work on it. label Mar 3, 2020
@martinmr
Copy link
Contributor

martinmr commented Mar 4, 2020

ugh. So it looks like there used to be a check that would prevent you from adding multiple uids. This check was removed in 45a6d5b but the values are not being correctly overwritten. I think the code worked when I introduced this feature but it depended on this check and there was not a regression test so we didn't notice when the feature broke.

Two action items.

  • Fix the bug
  • Add a regression test.

@martinmr
Copy link
Contributor

martinmr commented Mar 4, 2020

It looks like 1.2 and 1.2.1 are the only releases affected.

martinmr added a commit that referenced this issue Mar 12, 2020
When the warning to delete and re-add the value was removed, non-list uid
predicates were not being overwritten. This fixes this by iterating over the list
and replacing theexisting postings with a copy with the operation set to a delete.

It also adds a test to prevent this from happening again.

Fixes #4879
martinmr added a commit that referenced this issue Mar 17, 2020
When the warning to delete and re-add the value was removed, non-list uid
predicates were not being overwritten. This fixes this by iterating over the list
and replacing theexisting postings with a copy with the operation set to a delete.

It also adds a test to prevent this from happening again.

Fixes #4879
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something is broken. status/accepted We accept to investigate/work on it.
Development

Successfully merging a pull request may close this issue.

3 participants