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

@cascade disables value edge facet fetching in PR #4267 #4310

Closed
robsws opened this issue Nov 22, 2019 · 5 comments · Fixed by #4530
Closed

@cascade disables value edge facet fetching in PR #4267 #4310

robsws opened this issue Nov 22, 2019 · 5 comments · Fixed by #4530
Assignees
Labels
area/facets Issues related to face handling, querying, etc. area/querylang/cascade Related to the cascade directive. kind/bug Something is broken. status/accepted We accept to investigate/work on it.
Milestone

Comments

@robsws
Copy link

robsws commented Nov 22, 2019

I am building Dgraph from source using the animesh2049/issue_4081 branch currently up for review in PR #4267.

Using the following data:

{
   set {
     _:alice <Name> "Alice Tester" (since=2019-01-01) .
   }
}

I am able to fetch the "since" facet back without the @cascade directive, but not with it. To demonstrate, running this query:

{
  query(func:eq(Name, "Alice Tester")) {
    Name @facets(since)
  }
}

gives this result:

    "query": [
      {
        "Name|since": {
          "0": "2019-01-01T00:00:00Z"
        },
        "Name": [
          "Alice Tester"
        ]
      }
    ]

And this query, with @cascade:

{
  query(func:eq(Name, "Alice Tester")) @cascade {
    Name @facets(since)
  }
}

gives this result:

    "query": [
      {
        "Name": [
          "Alice Tester"
        ]
      }
    ]

As far as I know, @cascade should not be affecting whether or not facets are returned for a value edge. As a further note, this problem does not seem to occur for uid edges, only value edges.

@shekarm
Copy link

shekarm commented Nov 22, 2019

Hello,

Thank you for your question. Let me find someone within our team to address it.

@MichelDiz
Copy link
Contributor

MichelDiz commented Nov 25, 2019

Hi guys,

I'm not 100% about this behavior. For me cascade is related to nodes only. Facets aren't first-class-citizens do the majority of the functions aren't designed to work with them.

I'll let someone with the deepest knowledge in the cascade design work out what to do better or not.

Cheers.

@MichelDiz
Copy link
Contributor

MichelDiz commented Nov 25, 2019

BTW, I believe this is the best workaround you can have

{
  T as var(func: type(User)) @cascade {
    Name
    age
  }
  
  query(func: uid(T)){
    Name @facets(since)
    age
  }

}

This way you can isolate the facet from the cascade function.

Actually I think this is the solution. If so, case closed.

@sleto-it sleto-it added the status/needs-attention This issue needs more eyes on it, more investigation might be required before accepting/rejecting it label Dec 9, 2019
@MichelDiz MichelDiz added area/facets Issues related to face handling, querying, etc. area/querylang/cascade Related to the cascade directive. labels Dec 11, 2019
@sleto-it sleto-it added kind/bug Something is broken. status/accepted We accept to investigate/work on it. and removed status/needs-attention This issue needs more eyes on it, more investigation might be required before accepting/rejecting it labels Jan 9, 2020
ashish-goswami added a commit that referenced this issue Jan 13, 2020
Fixes #4310
Currently we are not able to fetch fecets associated with scalar predicate when using
cascade directive.
While processing cascade directive, we update uidMatrix. Every uid in uidMatrix is checked if
it is present in destUIDs, if not we remove it from uidMatrix. We also remove entry from
facetsMatrix for same uid. This works fine when predicate is of uid/[uid] type.
When predicate is of scalar type, destUIDs and uidMatrix will be empty, hence we should not
update facetsMatrix as facets here are corresponding to valueMatrix.
This PR avoid updating facetsMatrix when a uid list in uidMatrix is empty in updateUidMatrix().
@sleto-it
Copy link
Contributor

Hi all,
This ticket was fixed by PR #4530, which has now been merged

The fix will be delivered in v1.2

Please comment if more help is needed, and we will reopen

Many thanks,

@sleto-it sleto-it added this to the Dgraph v1.2 milestone Jan 13, 2020
@robsws
Copy link
Author

robsws commented Jan 20, 2020

Thanks for the fix here. I can confirm it looks good in 1.2.

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. area/querylang/cascade Related to the cascade directive. kind/bug Something is broken. status/accepted We accept to investigate/work on it.
Development

Successfully merging a pull request may close this issue.

5 participants