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

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

Merged
merged 9 commits into from
Jul 9, 2020

Conversation

ashish-goswami
Copy link
Contributor

@ashish-goswami ashish-goswami commented Jun 19, 2020

Fixes: #5241
Fixes: DGRAPH-1670

This PR fixes issue with facets when it is retrieved in a query containing @normalize directive.
While forming @normalize response, we flatten a fastJsonNode and make its grand children,
direct children of it. This should be valid in all of cases except when fastJsonNode is parent of
facets nodes. For example consider below data:

<0x1> <name> "Alice" .
<0x1> <friend> "Bob" (from="college") .
<0x1> <friend> "Roman" (from="school") .

Also consider below query:

q(func: uid(0x1)) @normalize {
    name: name
    friend: friends @facets
}

Expected response is:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "friends|from": {
          "0": "college",
          "1": "school"
        },
        "friends": [
          "Bob",
          "Roman"
        ]
      }
    ]
  }
}

But actual response is:

{
  "data": {
    "q": [
      {
        "0": "college",
        "1": "school",
        "friends": [
          "Bob",
          "Roman"
        ],
        "name": "Alice"
      }
    ]
  }
}

Its happening because we are flattening facet parent node friends|from as well which have node
"0" and "1" as children.
We are solving it by having extra information in the node if it is a facets parent.


This change is Reviewable

Docs Preview: Dgraph Preview

@ashish-goswami ashish-goswami changed the title Fix facets response with normalize (fix/dgraph) Fix facets response with normalize Jun 23, 2020
@ashish-goswami ashish-goswami changed the title (fix/dgraph) Fix facets response with normalize (fix/dgraph): Fix facets response with normalize Jun 23, 2020
Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ashish-goswami)


query/outputnode.go, line 169 at r1 (raw file):

// 3. List => Stores boolean value, true if this node is part of list.
// 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 predicaite. Eg: node "country|city"

city|country

predicate


query/outputnode.go, line 662 at r1 (raw file):

		// 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.
		// We should only consider those nodes for falttening which have children and are not

flatenning


query/query_facets_test.go, line 2118 at r1 (raw file):

}

func TestFacetValueListPredicateWithNormalize(t *testing.T) {

TestFacetValuePredicateWithNormalize


query/query_facets_test.go, line 2121 at r1 (raw file):

	populateClusterWithFacets()
	query := `{
		q(func: uid(1, 12000)) @normalize {

Also add a test where value is nested like

func: uid(1, 12000) @normalize {
  name
  friends {
    name @facets 
  }
}

Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @ashish-goswami)


query/outputnode.go, line 263 at r1 (raw file):

(

I don't think you need the outer parenteses

Copy link

@vvbalaji-dgraph vvbalaji-dgraph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Thanks for adding the tests

ashish-goswami added a commit that referenced this pull request Jul 9, 2020
Fixes: #5241
Fixes: DGRAPH-1670

This PR is related to #5690. It issues with facets response when facets are retrieved with
@normalize directive. Below two cases are covered:

Fixing facets response with uid/uid list predicates.
Fixing facets response with scalard list predicates.
@ashish-goswami ashish-goswami merged commit f4c28b8 into master Jul 9, 2020
@ashish-goswami ashish-goswami deleted the ashish/normalize-facets-tests branch July 9, 2020 13:06
ashish-goswami added a commit that referenced this pull request Jul 9, 2020
Fixes: #5241
Fixes: DGRAPH-1670

This PR fixes issue with facets when it is retrieved in a query containing @normalize directive.
While forming @normalize response, we flatten a fastJsonNode and make its grand children,
direct children of it. This should be valid in all of cases except when fastJsonNode is parent of
facets nodes. For example consider below data:

<0x1> <name> "Alice" .
<0x1> <friend> "Bob" (from="college") .
<0x1> <friend> "Roman" (from="school") .
Also consider below query:

q(func: uid(0x1)) @normalize {
    name: name
    friend: friends @facets
}
Expected response is:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "friends|from": {
          "0": "college",
          "1": "school"
        },
        "friends": [
          "Bob",
          "Roman"
        ]
      }
    ]
  }
}
But actual response is:

{
  "data": {
    "q": [
      {
        "0": "college",
        "1": "school",
        "friends": [
          "Bob",
          "Roman"
        ],
        "name": "Alice"
      }
    ]
  }
}
Its happening because we are flattening facet parent node friends|from as well which have node
"0" and "1" as children.
We are solving it by having extra information in the node if it is a facets parent.

(cherry picked from commit f4c28b8)
ashish-goswami added a commit that referenced this pull request Jul 10, 2020
Fixes: #5241
Fixes: DGRAPH-1670

This PR fixes issue with facets when it is retrieved in a query containing @normalize directive.
While forming @normalize response, we flatten a fastJsonNode and make its grand children,
direct children of it. This should be valid in all of cases except when fastJsonNode is parent of
facets nodes. For example consider below data:

<0x1> <name> "Alice" .
<0x1> <friend> "Bob" (from="college") .
<0x1> <friend> "Roman" (from="school") .
Also consider below query:

q(func: uid(0x1)) @normalize {
    name: name
    friend: friends @facets
}
Expected response is:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "friends|from": {
          "0": "college",
          "1": "school"
        },
        "friends": [
          "Bob",
          "Roman"
        ]
      }
    ]
  }
}
But actual response is:

{
  "data": {
    "q": [
      {
        "0": "college",
        "1": "school",
        "friends": [
          "Bob",
          "Roman"
        ],
        "name": "Alice"
      }
    ]
  }
}
Its happening because we are flattening facet parent node friends|from as well which have node
"0" and "1" as children.
We are solving it by having extra information in the node if it is a facets parent.

(cherry picked from commit f4c28b8)
parasssh pushed a commit that referenced this pull request Jul 10, 2020
Fixes: #5241
Fixes: DGRAPH-1670

This PR fixes issue with facets when it is retrieved in a query containing @normalize directive.
While forming @normalize response, we flatten a fastJsonNode and make its grand children,
direct children of it. This should be valid in all of cases except when fastJsonNode is parent of
facets nodes. For example consider below data:

<0x1> <name> "Alice" .
<0x1> <friend> "Bob" (from="college") .
<0x1> <friend> "Roman" (from="school") .
Also consider below query:

q(func: uid(0x1)) @normalize {
    name: name
    friend: friends @facets
}
Expected response is:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "friends|from": {
          "0": "college",
          "1": "school"
        },
        "friends": [
          "Bob",
          "Roman"
        ]
      }
    ]
  }
}
But actual response is:

{
  "data": {
    "q": [
      {
        "0": "college",
        "1": "school",
        "friends": [
          "Bob",
          "Roman"
        ],
        "name": "Alice"
      }
    ]
  }
}
Its happening because we are flattening facet parent node friends|from as well which have node
"0" and "1" as children.
We are solving it by having extra information in the node if it is a facets parent.

(cherry picked from commit f4c28b8)
arijitAD pushed a commit that referenced this pull request Jul 14, 2020
Fixes: #5241
Fixes: DGRAPH-1670

This PR fixes issue with facets when it is retrieved in a query containing @normalize directive.
While forming @normalize response, we flatten a fastJsonNode and make its grand children,
direct children of it. This should be valid in all of cases except when fastJsonNode is parent of
facets nodes. For example consider below data:

<0x1> <name> "Alice" .
<0x1> <friend> "Bob" (from="college") .
<0x1> <friend> "Roman" (from="school") .
Also consider below query:

q(func: uid(0x1)) @normalize {
    name: name
    friend: friends @facets
}
Expected response is:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "friends|from": {
          "0": "college",
          "1": "school"
        },
        "friends": [
          "Bob",
          "Roman"
        ]
      }
    ]
  }
}
But actual response is:

{
  "data": {
    "q": [
      {
        "0": "college",
        "1": "school",
        "friends": [
          "Bob",
          "Roman"
        ],
        "name": "Alice"
      }
    ]
  }
}
Its happening because we are flattening facet parent node friends|from as well which have node
"0" and "1" as children.
We are solving it by having extra information in the node if it is a facets parent.
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 18, 2020
Fixes: dgraph-io#5241
Fixes: DGRAPH-1670

This PR fixes issue with facets when it is retrieved in a query containing @normalize directive.
While forming @normalize response, we flatten a fastJsonNode and make its grand children,
direct children of it. This should be valid in all of cases except when fastJsonNode is parent of
facets nodes. For example consider below data:

<0x1> <name> "Alice" .
<0x1> <friend> "Bob" (from="college") .
<0x1> <friend> "Roman" (from="school") .
Also consider below query:

q(func: uid(0x1)) @normalize {
    name: name
    friend: friends @facets
}
Expected response is:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "friends|from": {
          "0": "college",
          "1": "school"
        },
        "friends": [
          "Bob",
          "Roman"
        ]
      }
    ]
  }
}
But actual response is:

{
  "data": {
    "q": [
      {
        "0": "college",
        "1": "school",
        "friends": [
          "Bob",
          "Roman"
        ],
        "name": "Alice"
      }
    ]
  }
}
Its happening because we are flattening facet parent node friends|from as well which have node
"0" and "1" as children.
We are solving it by having extra information in the node if it is a facets parent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

@normalize broken for >=v1.2.0 when using @facets
4 participants