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

Make the new Facet response Optional. #4907

Closed
MichelDiz opened this issue Mar 10, 2020 · 8 comments
Closed

Make the new Facet response Optional. #4907

MichelDiz opened this issue Mar 10, 2020 · 8 comments
Assignees
Labels
area/facets Issues related to face handling, querying, etc. kind/bug Something is broken. status/accepted We accept to investigate/work on it.

Comments

@MichelDiz
Copy link
Contributor

MichelDiz commented Mar 10, 2020

Experience Report

What you wanted to do

Query for facets and return it in an ideal format (which was the original behavior).

What you actually did

Query for my node and it returns an unwanted format that breaks my application.

Why that wasn't great, with examples

The PR #4267 has done a good and welcome change in Facet's behavior to solve an issue with List Type and Facets (adding support to facets on lists). Which is great. But, this breaks how things used to be done in Dgraph. And several users had to change their application or are blocked without fully understanding Facet's behavior. Confusing the process completely.

We explain in our docs how a JSON object is treated in Dgraph
e.g:

{
  "name": "Carol",
  "name|initial": "C",
  "dgraph.type": "Person",
  "friend": {
    "name": "Daryl",
    "friend|close": "yes",
    "dgraph.type": "Person"
  }
}

This object above explains in a logical way to the user how a facet should be treated (And indirectly we demonstrate how the structure would be in the response of a query with facets). However, when making a query we receive the same object in a completely different format. In a format that should be optional and perhaps mandatory for List Type only.

{
  q(func: has(friend)){
    name @facets
    dgraph.type
    friend @facets {
      name
      dgraph.type
    }
  }
}
{
  "data": {
    "q": [
      {
        "name": "Carol",
        "dgraph.type": [
          "Person"
        ],
        "friend": {
          "name": "Daryl",
          "dgraph.type": [
            "Person"
          ]
        },
        "friend|close": "yes"
      }
    ]
  }
}

This format above should be optional and bellow the default

{
  "data": {
    "q": [
      {
        "name": "Carol",
        "dgraph.type": [
          "Person"
        ],
        "friend": {
          "name": "Daryl",
          "friend|close": "yes",
          "dgraph.type": [
            "Person"
          ]
        }
      }
    ]
  }
}

Despite the change, this behavior does not affect the Value Facet at all.
If you query for:

{
  q(func: has(friend)){
    name @facets
  }
}

you gonna see (which is the previous behavior)

{
  "data": {
    "q": [
      {
        "name|initial": "C",
        "name": "Carol"
      }
    ]
  }
 }

Users are getting a bad experience from it.

Maybe we should make this behavior optional. Making possible to use the normal response (as always did) and a “map” response(as it is today).

e.g:

query {
  r(func: uid(0x01)) {
    seen @facets @map {
      uid
    }
  }
}

Any external references to support your case

https://docs.dgraph.io/mutations/#facets

https://discuss.dgraph.io/t/the-query-result-of-dgraph-v1-2-1-about-facets-looks-strange-did-i-miss-anything/5992
#4798
https://discuss.dgraph.io/t/facets-on-relations/5906

https://dgraphlabs.slack.com/archives/CSH96QK62/p1581011255272800
https://dgraph.slack.com/archives/C13LH03RR/p1583814358358000?thread_ts=1583813815.357600&cid=C13LH03RR

@MichelDiz MichelDiz added kind/bug Something is broken. status/accepted We accept to investigate/work on it. area/facets Issues related to face handling, querying, etc. labels Mar 10, 2020
@trungtin719tt
Copy link

Thank you for helping me raising the issue upon.
I don't really see the issue and reason why we change the facets response to the way it is now. But at a dgraph's user perspective, I think it would make more sense to have same data struct for creating or querying facets value.
And putting facets value inside the object would give a better understandable view to anyone even who new to dgraph just like me. Despite the fact that how it's physically stored behind.

@mileung
Copy link

mileung commented Mar 10, 2020

putting facets value inside the object would give a better understandable view

I second this. Please update the docs to reflect this change in behavior or re-add the "Facets on scalar predicates" feature which was a nice touch imo. Having to manually set the facet of each corresponding node after getting the query response will definitely lead to some hacky code.

@MichelDiz
Copy link
Contributor Author

@mileung Facets on scalar are okay. What was changed is facets in entities or List Type https://docs.dgraph.io/query-language/#list-type

@mileung
Copy link

mileung commented Mar 24, 2020

Will upcoming versions of Dgraph bring back the old behavior like?

{
  "data": {
    "q": [
      {
        "name|initial": "C",
        "name": "Carol"
      }
    ]
  }
 }

This new behavior is difficult to work with in examples like this

Screen Shot 2020-03-23 at 11 18 47 PM

where ideally, the facets should be like

  "data": {
    "q": [
      {
        "checkouts": [
          {
            "uid": "0x201d3",
            "lessons": [
              {
                "uid": "0x201ae",
                "title": "ROFL",
                "lessons|salePrice": 1
              }
            ],
            "total": 1,
            "date": 1585021708691
          }
        ]
      }
    ]
  }

@MichelDiz
Copy link
Contributor Author

We are still working on it.

@manishrjain
Copy link
Contributor

I think if the facet is attached to the object, then let's put the facet inside the object. If it is a list of scalar values, then we can use the map.

@ashish-goswami
Copy link
Contributor

Hi all, we have discussed facets format internally and prepared one document with different possible formats. Please checks below discuss post and provide your feedback.
https://discuss.dgraph.io/t/facets-format-in-mutation-requests-and-query-responses/6416

ashish-goswami added a commit that referenced this issue May 20, 2020
Fixes: #4798, #4581, #4907
DGRAPH-1109, DGRAPH-1062, DGRAPH-1143

This is PR changes facets format as discussed in the post: https://discuss.dgraph.io/t/facets-format-in-mutation-requests-and-query-responses/6416

After this PR is merged response/requests formats will look like as below:
Current UID predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "San Francisco",
        "state": {
          "name": "California"
        },
        "state|capital": false
      }
    ]
  }
}
New UID predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "San Francisco",
        "state": {
          "name": "California",
          "state|capital": false
        }
      }
    ]
  }
}
Current UID list predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "speaks": [
          {
            "name": "Spanish"
          },
          {
            "name": "Chinese"
          }
        ],
        "speaks|fluent": {
          "0": true,
          "1": false
        }
      }
    ]
  }
}
New UID list predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "speaks": [
          {
            "name": "Spanish",
            "speaks|fluent": true
          },
          {
            "name": "Chinese",
            "speaks|fluent": false
          }
        ]
      }
    ]
  }
}
Current scalar list predicate facets mutation request:

{
  "set": [
    {
      "uid": "_:1",
      "nickname": "Joshua",
      "nickname|kind": "official"
    },
    {
      "uid": "_:1",
      "nickname": "David"
    },
    {
      "uid": "_:1",
      "nickname": "Josh",
      "nickname|kind": "friends"
    }
  ]
}
New scalar list predicate facets mutation request:

{
  "set": {
      "uid": "_:1",
      "nickname": ["Joshua", "David", "Josh"],
      "nickname|kind": {
         "0": "official",
         "2": "friends"
      }
   }
}
NOTE: there is no change in the request/response facets format for scalar predicate type.
@lgalatin
Copy link
Contributor

Fixed in #5424

dna2github pushed a commit to dna2fork/dgraph that referenced this issue Jul 18, 2020
Fixes: dgraph-io#4798, dgraph-io#4581, dgraph-io#4907
DGRAPH-1109, DGRAPH-1062, DGRAPH-1143

This is PR changes facets format as discussed in the post: https://discuss.dgraph.io/t/facets-format-in-mutation-requests-and-query-responses/6416

After this PR is merged response/requests formats will look like as below:
Current UID predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "San Francisco",
        "state": {
          "name": "California"
        },
        "state|capital": false
      }
    ]
  }
}
New UID predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "San Francisco",
        "state": {
          "name": "California",
          "state|capital": false
        }
      }
    ]
  }
}
Current UID list predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "speaks": [
          {
            "name": "Spanish"
          },
          {
            "name": "Chinese"
          }
        ],
        "speaks|fluent": {
          "0": true,
          "1": false
        }
      }
    ]
  }
}
New UID list predicate facets query response:

{
  "data": {
    "q": [
      {
        "name": "Alice",
        "speaks": [
          {
            "name": "Spanish",
            "speaks|fluent": true
          },
          {
            "name": "Chinese",
            "speaks|fluent": false
          }
        ]
      }
    ]
  }
}
Current scalar list predicate facets mutation request:

{
  "set": [
    {
      "uid": "_:1",
      "nickname": "Joshua",
      "nickname|kind": "official"
    },
    {
      "uid": "_:1",
      "nickname": "David"
    },
    {
      "uid": "_:1",
      "nickname": "Josh",
      "nickname|kind": "friends"
    }
  ]
}
New scalar list predicate facets mutation request:

{
  "set": {
      "uid": "_:1",
      "nickname": ["Joshua", "David", "Josh"],
      "nickname|kind": {
         "0": "official",
         "2": "friends"
      }
   }
}
NOTE: there is no change in the request/response facets format for scalar predicate type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/facets Issues related to face handling, querying, etc. kind/bug Something is broken. status/accepted We accept to investigate/work on it.
Development

No branches or pull requests

6 participants