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

Why ALL is used instead of ANY when converting where query filters to cypher? #190

Closed
morningcloud opened this issue May 5, 2021 · 5 comments · Fixed by #196
Closed
Labels
bug Something isn't working confirmed Confirmed bug

Comments

@morningcloud
Copy link

Describe the bug
I have a use case where one node (e.g. User) is linked to multiple nodes of the same label (e.g. User Demographics), each User can be linked to one or more different types of demographics depending on the client the users are under. I wanted to write a query to fetch users that have specific demographic data. But the results I am getting for the related queries is not expected. Below explains the use case and the result I am getting and what I actually expected.

Type definitions

type User {
    client_id: String
    uid: String
    demographics: [UserDemographics] @relationship(type: "HAS_DEMOGRAPHIC", direction: OUT)
}

type UserDemographics {
    client_id: String,
    type: String,
    value: String,
    users: [User] @relationship(type: "HAS_DEMOGRAPHIC", direction: IN)
    }

image

To Reproduce
Steps to reproduce the behavior:

  1. Start neo4j & graphql server
  2. Execute the following Cypher queries to populate neo4j with sample data:
Create (:User {uid: "user1"}),(:User {uid: "user2"}),(:UserDemographics{type:"Gender",value:"Female"}),(:UserDemographics{type:"Gender",value:"Male"}),(:UserDemographics{type:"Age",value:"50+"}),(:UserDemographics{type:"State",value:"VIC"})
MATCH (u:User {uid: 'user1'}) MATCH (d:UserDemographics {type: 'Gender', value:'Female'}) create (u)-[:HAS_DEMOGRAPHIC]->(d)
MATCH (u:User {uid: 'user2'}) MATCH (d:UserDemographics {type: 'Gender', value:'Male'}) create (u)-[:HAS_DEMOGRAPHIC]->(d)
MATCH (u:User {uid: 'user1'}) MATCH (d:UserDemographics {type: 'Age', value:'50+'}) create (u)-[:HAS_DEMOGRAPHIC]->(d)
MATCH (u:User {uid: 'user2'}) MATCH (d:UserDemographics {type: 'Age', value:'50+'}) create (u)-[:HAS_DEMOGRAPHIC]->(d)
MATCH (u:User {uid: 'user1'}) MATCH (d:UserDemographics {type: 'State', value:'VIC'}) create (u)-[:HAS_DEMOGRAPHIC]->(d)
MATCH (u:User {uid: 'user2'}) MATCH (d:UserDemographics {type: 'State', value:'VIC'}) create (u)-[:HAS_DEMOGRAPHIC]->(d)
  1. Then run the following GraphQL Query:
query{
  users(where: {demographics:{
      type:"Gender", value:"Female"}
  }){
    uid
    demographics{
      type
      value
    }
  }
}
  1. Result returned is:
{
  "data": {
    "users": []
  }
}
  1. Run the following GraphQL Query:
query{
  users(where: {demographics:{
    OR:[
      {type:"Gender", value:"Female"},
      {type:"State"}
      {type:"Age"},
    ]}
  }){
    uid
    demographics{
      type
      value
    }
  }
}
  1. Result returned is:
{
  "data": {
    "users": [
      {
        "uid": "user1",
        "demographics": [
          {
            "type": "State",
            "value": "VIC"
          },
          {
            "type": "Age",
            "value": "50+"
          },
          {
            "type": "Gender",
            "value": "Female"
          }
        ]
      }
    ]
  }
}

Expected behavior
I expected to see the result shown in step 6 to be returned when running the query in step 3. I also expected to see all users listed as a result of the query in step 5 since there is an OR operation

System Details:

  • OS: macOS
  • Version: @neo4j/graphql@1.0.0
  • Node.js version: 12.10.0

Additional context
When running server with Debug Logging DEBUG=@neo4j/graphql:* node src/index.js I can see the generated cypher for the query is using ALL when I replace ALL with ANY and run the same cypher query in neo4j browser, I get the expected result for both queries. Is it done intentionally? From my perspective it doesn't seem intuitive and I don't feel it would be right to list all the possible demographic types when doing the query as this list can change per client and can get updated over time.

Cypher:
MATCH (this:User)
WHERE EXISTS((this)-[:HAS_DEMOGRAPHIC]->(:UserDemographics)) AND ALL(this_demographics IN [(this)-[:HAS_DEMOGRAPHIC]->(this_demographics:UserDemographics) | this_demographics] WHERE this_demographics.type = $this_demographics_type AND this_demographics.value = $this_demographics_value)
RETURN this { .uid, demographics: [ (this)-[:HAS_DEMOGRAPHIC]->(this_demographics:UserDemographics)   | this_demographics { .type, .value } ] } as this
Params:
{
  "this_demographics_type": "Gender",
  "this_demographics_value": "Female"
}
@morningcloud morningcloud added bug Something isn't working inbox labels May 5, 2021
@darrellwarde
Copy link
Contributor

Hey @morningcloud, thanks for raising this, and with so much detail to reproduce.

I'm still taking in all of the information, but there's definitely something to investigate here. Going to create a branch and make some test cases around the information you've provided, and I'll take it from there.

@darrellwarde
Copy link
Contributor

So I've created a PR containing the very simple fix for this problem, validated by test cases using the data provided in this issue.

Need to get some eyes on it to ensure that this is indeed the appropriate solution for the problem, please feel free to take a look.

If we agree to merge, I'm going to recommend we do a patch release containing this fix, as we obviously want to be returning expected data from our library.

@darrellwarde darrellwarde added the confirmed Confirmed bug label May 7, 2021
darrellwarde added a commit that referenced this issue May 10, 2021
Replace ALL with ANY in whilst querying relationship equality
@darrellwarde
Copy link
Contributor

Now released in 1.0.1: https://github.com/neo4j/graphql/releases/tag/%40neo4j%2Fgraphql%401.0.1 🙂

@morningcloud
Copy link
Author

Thank you @darrellwarde, I can confirm that the behaviour is as expected with the latest release 👍🏻

@MarianAlexandruAlecu
Copy link

@darrellwarde Hey, a bit late to the party, but I think the same behaviour should apply to @auth where & bind in case we are referencing a relationship that is a list

With the following schema:

type User {
  id: ID! @id
  name: String!
}
type Chat {
  id: ID! @id
  title: String @readonly
  description: String @readonly
  users: [User] @relationship(type: "HAS_USER", direction: OUT)
}

extend type Chat
  @auth(rules: [
    { operations: [CREATE, READ, UPDATE, DELETE], where: { users: { id: "$jwt.sub" } } }
  ])

How can I get all Chats for the logged user? Because where & bind apply ALL(), I will be able to get only chats where $jwt.sub is the only user.

On the other hand, I think ALL() makes sense if it was a singular relationship:

type Chat {
  user: User @relationship(type: "HAS_USER", direction: OUT)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants