Skip to content

(fix/dgraph): Fix facets response with normalize #5690

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

Merged
merged 9 commits into from
Jul 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 36 additions & 17 deletions query/outputnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,14 @@ type encoder struct {
// Bytes 4-1 contains offset(uint32) for Arena.
// Bytes 7-6 contains attr.
// Bit MSB(first bit in Byte-8) contains list field value.
// Bit SecondMSB(second bit in Byte-8) contains facetsParent field value.
// Byte-5 is not getting used as of now.
// |-----------------------------------------------------------------------|
// | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 |
// | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 |
// |-----------------------------------------------------------------------|
// | MSB | | Unused | |
// | for | Attr ID | For | Offset inside Arena |
// | list | | Now | |
// | MSB - list | | Unused | |
// | SecondMSB - | Attr ID | For | Offset inside Arena |
// | facetsParent| | Now | |
// |-----------------------------------------------------------------------|
metaSlice []uint64
// childrenMap contains mapping of fastJsonNode to its children.
Expand Down Expand Up @@ -149,6 +150,8 @@ func (enc *encoder) makeScalarNode(attr uint16, val []byte, list bool) (fastJson
const (
// Value with most significant bit set to 1.
msbBit = 0x8000000000000000
// Value with second most significant bit set to 1.
secondMsbBit = 0x4000000000000000
// Value with all bits set to 1 for bytes 7 and 6.
setBytes76 = uint64(0x00FFFF0000000000)
// Compliment value of setBytes76.
Expand All @@ -162,7 +165,17 @@ const (
// 1. Attr => predicate associated with this node.
// 2. ScalarVal => Any value associated with node, if it is a leaf node.
// 3. List => Stores boolean value, true if this node is part of list.
// 4. Children(Attrs) => List of all children.
// 4. FacetsParent => Stores boolean value, true if this node is a facetsParent. facetsParent is
// node which is parent for facets values for a scalar list predicate. Eg: node "city|country"
// will have FacetsParent value as true.
// {
// "city": ["Bengaluru", "San Francisco"],
// "city|country": {
// "0": "india",
// "1": "US"
// }
// }
// 5. Children(Attrs) => List of all children.
//
// All of the data for fastJsonNode tree is stored in encoder to optimise memory usage. fastJsonNode
// type only stores one uint32(can be thought of id for this node). A fastJsonNode is created in
Expand Down Expand Up @@ -217,6 +230,10 @@ func (enc *encoder) setList(fj fastJsonNode, list bool) {
}
}

func (enc *encoder) setFacetsParent(fj fastJsonNode) {
enc.metaSlice[fj] |= secondMsbBit
}

// appendAttrs appends attrs to existing fj's attrs.
func (enc *encoder) appendAttrs(fj fastJsonNode, attrs ...fastJsonNode) {
cs, ok := enc.childrenMap[fj]
Expand All @@ -239,7 +256,11 @@ func (enc *encoder) getScalarVal(fj fastJsonNode) ([]byte, error) {
}

func (enc *encoder) getList(fj fastJsonNode) bool {
return ((enc.metaSlice[fj] & msbBit) > 0)
return (enc.metaSlice[fj] & msbBit) > 0
}

func (enc *encoder) getFacetsParent(fj fastJsonNode) bool {
return (enc.metaSlice[fj] & secondMsbBit) > 0
}

func (enc *encoder) getAttrs(fj fastJsonNode) []fastJsonNode {
Expand Down Expand Up @@ -522,6 +543,8 @@ func (enc *encoder) attachFacets(fj fastJsonNode, fieldName string, isList bool,
if err != nil {
return err
}
// Mark this node as facetsParent.
enc.setFacetsParent(facetNode)
enc.AddMapChild(fj, facetNode)
}
}
Expand Down Expand Up @@ -636,13 +659,9 @@ func (enc *encoder) normalize(fj fastJsonNode) ([][]fastJsonNode, error) {
for _, a := range fjAttrs {
// Here we are counting all non-scalar attributes of fj. If there are any such
// attributes, we will flatten it, otherwise we will return all attributes.

// When we call addMapChild it tries to find whether there is already an attribute
// with attr field same as attribute argument of addMapChild. If it doesn't find any
// such attribute, it creates an attribute with isChild = false. In those cases
// sometimes cnt remains zero and normalize returns attributes without flattening.
// So we are using len(a.attrs) > 0 instead of a.isChild
if len(enc.getAttrs(a)) > 0 {
// We should only consider those nodes for flattening which have children and are not
// facetsParent.
if len(enc.getAttrs(a)) > 0 && !enc.getFacetsParent(a) {
cnt++
}
}
Expand All @@ -658,17 +677,17 @@ func (enc *encoder) normalize(fj fastJsonNode) ([][]fastJsonNode, error) {
// merged with children later.
attrs := make([]fastJsonNode, 0, len(fjAttrs)-cnt)
for _, a := range fjAttrs {
// Check comment at previous occurrence of len(a.attrs) > 0
if len(enc.getAttrs(a)) == 0 {
// Here, add all nodes which have either no children or they are facetsParent.
if len(enc.getAttrs(a)) == 0 || enc.getFacetsParent(a) {
attrs = append(attrs, a)
}
}
parentSlice = append(parentSlice, attrs)

for ci := 0; ci < len(fjAttrs); {
childNode := fjAttrs[ci]
// Check comment at previous occurrence of len(a.attrs) > 0
if len(enc.getAttrs(childNode)) == 0 {
// Here, exclude all nodes which have either no children or they are facetsParent.
if len(enc.getAttrs(childNode)) == 0 || enc.getFacetsParent(childNode) {
ci++
continue
}
Expand Down
186 changes: 186 additions & 0 deletions query/query_facets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2048,6 +2048,192 @@ func TestFacetValueListPredicate(t *testing.T) {
`, js)
}

func TestFacetUIDPredicateWithNormalize(t *testing.T) {
populateClusterWithFacets()
query := `{
q(func: uid(0x1)) @normalize {
name: name
from: boss @facets {
boss: name
}
}
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `
{
"data": {
"q": [
{
"boss": "Roger",
"from|company": "company1",
"name": "Michonne"
}
]
}
}
`, js)
}

func TestFacetUIDListPredicateWithNormalize(t *testing.T) {
populateClusterWithFacets()
query := `{
q(func: uid(0x1)) @normalize {
name: name
friend @facets(since) {
friend_name: name
}
}
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `
{
"data": {
"q": [
{
"friend_name": "Rick Grimes",
"friend|since": "2006-01-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Glenn Rhee",
"friend|since": "2004-05-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Daryl Dixon",
"friend|since": "2007-05-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Andrea",
"friend|since": "2006-01-02T15:04:05Z",
"name": "Michonne"
}
]
}
}
`, js)
}

func TestNestedFacetUIDListPredicateWithNormalize(t *testing.T) {
populateClusterWithFacets()
query := `{
q(func: uid(0x1)) @normalize {
name: name
friend @facets(since) @normalize {
friend_name: name @facets
friend @facets(close) {
friend_name_level2: name
}
}
}
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `
{
"data": {
"q": [
{
"friend_name": "Rick Grimes",
"friend_name_level2": "Michonne",
"friend_name|dummy": true,
"friend_name|origin": "french",
"friend|since": "2006-01-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Glenn Rhee",
"friend_name|dummy": true,
"friend_name|origin": "french",
"friend|since": "2004-05-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Daryl Dixon",
"friend|since": "2007-05-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Andrea",
"friend_name_level2": "Michonne",
"friend|close": false,
"friend|since": "2006-01-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Andrea",
"friend_name_level2": "Glenn Rhee",
"friend|since": "2006-01-02T15:04:05Z",
"name": "Michonne"
},
{
"friend_name": "Andrea",
"friend_name_level2": "Daryl Dixon",
"friend|close": false,
"friend|since": "2006-01-02T15:04:05Z",
"name": "Michonne"
}
]
}
}
`, js)
}

func TestFacetValuePredicateWithNormalize(t *testing.T) {
populateClusterWithFacets()
query := `{
q(func: uid(1, 12000)) @normalize {
eng_name: name@en @facets
alt_name: alt_name @facets
}
}`
js := processQueryNoErr(t, query)
require.JSONEq(t, `
{
"data":{
"q":[
{
"eng_name|origin":"french",
"eng_name":"Michelle",
"alt_name|dummy":{
"0":true,
"1":false
},
"alt_name|origin":{
"0":"french",
"1":"spanish"
},
"alt_name|isNick":{
"1":true
},
"alt_name":[
"Michelle",
"Michelin"
]
},
{
"eng_name|dummy":true,
"eng_name|origin":"french",
"eng_name":"Harry",
"alt_name|dummy":{
"0":false
},
"alt_name|isNick":{
"0":true
},
"alt_name|origin":{
"0":"spanish"
},
"alt_name":[
"Potter"
]
}
]
}
}
`, js)
}

func TestFacetValueListPredicateSingleFacet(t *testing.T) {
populateClusterWithFacets()
query := `{
Expand Down