fix(Mutation): Deeply-nested uid facets

[nelsonpecora]

Between v20.03 and v20.11 (with #5424), dgraph lost the ability to handle deeply-nested uid facets. A JSON mutation with non-nested uid facets looks like this:

{
  "set": [{
  	"title": "parent",
  	"children": [{
  		"title": "child",
  		"children|position": 1
		}]
	}]
}

A mutation with deeply-nested uid facets looks like this:

{
  "set": [{
  	"title": "parent",
  	"children": [{
  		"title": "child",
  		"children|position": 1,
  		"children": [{
  			"title": "grandchild",
  			"children|position": 1
			}]
		}]
	}]
}

There's something about trying to set both children and children|position on the same JSON object that's causing it to error. The equivalent mutation in RDF succeeds:

{
  set {
    <_:parent> <title> "parent" .
    <_:parent> <children> <_:child> (position=1) .
    <_:child> <title> "child" .
    <_:child> <children> <_:grandchild> (position=1) .
    <_:grandchild> <title> "grandchild" .
  }
}

---

Fix(karl): The short explanation is there was an old TODO mentioning that we should only run parseMapFacets if we are within a JSON scalar array, but we were running it from within JSON object arrays as well. Only running it from within JSON scalar arrays fixed the issue.

Longer explanation: By calling parseMapFacets from within a JSON object array, the "friend|close": true facet was correctly assumed to have an object (map) value rather than its scalar value, and this error was returned.

The reason "another_friend|close": true worked despite this is because parseScalarMap looks for facets on the same level as the predicate. For example, here's how map facets should look:

{
    "nickname": ["alice", "bob", "josh"],
    "nickname|kind": {
        "0": "friends",
        "2": "official"
    }
}

So in the working another_friend example, parseMapFacets is called two times (once for each named JSON array declaration):

1. parseMapFacets(map[string]interface{}{}, "friend|")
   • mapToNquads encounters "friend": [ on the current level and calls parseMapFacets with all facets found on the current level (none) and "friend" + "|" as the prefix parameter.
   • This works because m is empty and parseMapFacets returns nil.
2. parseMapFacets(map[string]interface{}{"friend|close": true}, "another_friend|")
   • mapToNquads encounters "another_friend": [ on the current level and calls parseMapFacets with all facets found on the current level and "another_friend" + "|" as the prefix parameter.
   • This works because we check if prefix (in this case, another_friend|) is in the facet name (in this case, friend|close) and parseMapFacets returns nil.

So parseMapFacets is only called two times, both times returning nil, and we pick up the facets with parseScalarFacets later.

With this PR fix, we only call parseMapFacets for JSON scalar arrays and avoid everything mentioned above.

---

This change is 

[CLAassistant]

All committers have signed the CLA.

[karlmcguire]

Hello, I'm looking into this. Just leaving some notes here.

1. nested scalar facet - different name (works)

[
    {
        "name": "Alice",
        "friend": [
            {
                "name": "Dave",
                "friend|close": true,
                "another_friend": [
                    {
                        "name": "Emily",
                        "another_friend|close": true
                    }
                ]
            }
        ]
    }
]


(*api.NQuad)(0xc0005a6150)(subject:"_:dg.2351440435.1" predicate:"name" object_value:<str_val:"Alice" > )
(*api.NQuad)(0xc0005a62a0)(subject:"_:dg.2351440435.2" predicate:"name" object_value:<str_val:"Dave" > )
(*api.NQuad)(0xc0005a63f0)(subject:"_:dg.2351440435.3" predicate:"name" object_value:<str_val:"Emily" > )
(*api.NQuad)(0xc0005a6380)(subject:"_:dg.2351440435.2" predicate:"another_friend" object_id:"_:dg.2351440435.3" facets:<key:"close" value:"\001" val_type:BOOL > )
(*api.NQuad)(0xc0005a6230)(subject:"_:dg.2351440435.1" predicate:"friend" object_id:"_:dg.2351440435.2" facets:<key:"close" value:"\001" val_type:BOOL > )

2. nested scalar facet (doesn't work, but should)

[
    {
        "name": "Alice",
        "friend": [
            {
                "name": "Dave",
                "friend|close": true,
                "friend": [
                    {
                        "name": "Emily",
                        "friend|close": true
                    }
                ]
            }
        ]
    }
]

(*api.NQuad)(0xc000448000)(subject:"_:dg.3392663716.1" predicate:"name" object_value:<str_val:"Alice" > )
(*api.NQuad)(0xc000448150)(subject:"_:dg.3392663716.2" predicate:"name" object_value:<str_val:"Dave" > )

(*errors.fundamental)(0xc00050e100)(facets format should be of type map for scalarlist predicates, found: true for facet: friend|close)

[karlmcguire]

2. nested scalar facet (working now)

[
    {
        "name": "Alice",
        "friend": [
            {
                "name": "Dave",
                "friend|close": true,
                "friend": [
                    {
                        "name": "Emily",
                        "friend|close": true
                    }
                ]
            }
        ]
    }
]


(*api.NQuad)(0xc000501030)(subject:"_:dg.3180879551.1" predicate:"name" object_value:<str_val:"Alice" > )
(*api.NQuad)(0xc000501260)(subject:"_:dg.3180879551.3" predicate:"name" object_value:<str_val:"Emily" > )
(*api.NQuad)(0xc0005011f0)(subject:"_:dg.3180879551.2" predicate:"friend" object_id:"_:dg.3180879551.3" facets:<key:"close" value:"\001" val_type:BOOL > )
(*api.NQuad)(0xc0005012d0)(subject:"_:dg.3180879551.2" predicate:"name" object_value:<str_val:"Dave" > )
(*api.NQuad)(0xc000501110)(subject:"_:dg.3180879551.1" predicate:"friend" object_id:"_:dg.3180879551.2" facets:<key:"close" value:"\001" val_type:BOOL > )

[danielmai]

Reviewed 1 of 2 files at r3, 2 of 2 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @karlmcguire and @nelsonpecora)

---

go.mod, line 17 at r4 (raw file):

	github.com/blevesearch/bleve v1.0.13
	github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd
	github.com/davecgh/go-spew v1.1.1

What's go-spew? I don't see it used in this PR, so why is this dependency added?

[danielmai]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @karlmcguire and @nelsonpecora)

---

go.mod, line 17 at r4 (raw file):

Previously, danielmai (Daniel Mai) wrote…

What's go-spew? I don't see it used in this PR, so why is this dependency added?

Nevermind, I see you're using it for debugging.

[danielmai]

Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @danielmai and @karlmcguire)