neo4j / graphql

A GraphQL to Cypher query execution layer for Neo4j and JavaScript GraphQL implementations.
https://neo4j.com/docs/graphql-manual/current/
Apache License 2.0
507 stars 149 forks source link

Using alias for an autogenerated <connection> relation in a GraphQL query leads to an invalid cypher query generation when used nested in the GraphQL query #1817

Closed norbertBergmann closed 2 years ago

norbertBergmann commented 2 years ago

Describe the bug In a graphql query aliases can be used for querying nodes and node's properties which is working fine. For relation between nodes a specific "[relationName]connection" element is generated automatically in graphql. Using an alias for this connection element leads to an invalid cypher query generation and a neo4j error message if the a connection element is used nested within the graphql query .

Type definitions Three type definitions which have relations defined between them like Type_A - [RelationA]-Type_B-[RelationB]-Type_C

To Reproduce 1.Run queries of the following types: This one works fine:

{
  Type_A {
    AliasNestedLevel1: RelationAConnection {
       edges {
        node {
          id
        }
      }
    }
}

Result:

{
  "data": {
    "Type_A": [
      {
        "AliasLevel1": {
          "edges": [
            {
              "node": {
                "id": "1d298720-f32d-4277-a03e-3989129d2fb5"
              }
            }]
       }
   }
}
  1. This one returns empty element for "edges":

    {
    Type_A {
    AliasNestedLevel1: RelationAConnection {
       AliasNestedLevel1_1: edges {
        node {
          id
        }
      }
    }
    }

    Result:

    {
    "data": {
    "Type_A": [
      {
        "AliasLevel1": {
          "AliasLevel1_1": []
        }
      }
    }
    }
  2. This one creates neo4j error due to an invalid cypher query:

    {
    Type_A {
    RelationA {
      AliasNestedLevel2: RelationBConnection {
        edges {
          node {
            id
          }
        }
      }
    }
    }
    }

    Result:

    {
    "errors": [
    {
      "message": "Failed to invoke function `apoc.cypher.runFirstColumn`: Caused by: org.neo4j.exceptions.SyntaxException: Variable `RelationBConnection` not defined 
  3. The auto generated and invalid cypher looks like the following. The invalid parts of the last 2 lines are highlighted bold:

_MATCH (this:Type_A) RETURN this { RelationA: [ (this)<-[:RelationA]-(this_RelationA:Type_B) | this_RelationA { RelationBConnection: apoc.cypher.runFirstColumn("CALL { WITH this_RelationA MATCH (this_RelationA)<-[this_RelationA_RelationB_relationship:RelationB]-(this_RelationA_Type_C:Type_C) WITH collect({ node: { id: this_RelationA_Type_C.Id } }) AS edges RETURN { edges: edges, totalCount: size(edges) } AS AliasNestedLevel2 } RETURN RelationBConnection", { this_RelationA: thisRelationA, auth: $auth }, false) } ] } as this

Expected behavior Aliases should be lead to valid results and valid cypher queries on any nested level of a graphql query

System (please complete the following information):

neo4j-team-graphql commented 2 years ago

Many thanks for raising this bug report @norbertBergmann. :bug: We will now attempt to reproduce the bug based on the steps you have provided.

Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:

If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket.

Thanks again! :pray:

darrellwarde commented 2 years ago

Hey @norbertBergmann, can you please provide us with some more solid information to reproduce? We really need:

We can't really extrapolate steps to reproduce from issues without this information. Thanks!

ericmartineau commented 2 years ago

I'm observing this behavior when dealing with Union types. Everything works correctly when you select the concrete types, but when you select the union type, then the *Connection properties within the union type generate this error.

Definitions:


type PetOwner {
  phoneNumber: String!
}

@relationshipProperties
interface PetOwnerProperties {
   purchaseDate: Timestamp!
}

type Cat {
  name: String!
  owner: PetOwner! @relationship(type: "OWNER", direction: OUT, properties: "PetOwnerProperties")
}

type Dog {
  name: String!
  owner: PetOwner! @relationship(type: "OWNER", direction: OUT, properties: "PetOwnerProperties")
}

type Pet {
  breed: Breed!
}

union Breed = Cat | Dog;

type PetList {
  pets: [Pet!]! @relationship(type: "PET", direction: OUT)
}

This query works:

fragment DogFragment on Dog {
  dogs {
    ownerConnection {
       edges {
         node {
            phoneNumber
         }
         purchaseDate
      } 
    }
  }
}
query allDogs {
   ...DogFragment
}

but this one fails:

query allPets {
  petLists {
    ...on Dog {
      ...DogFragment
    }
  }
}

Sorry, I didn't test this fully, but hopefully it communicates the IDEA

darrellwarde commented 2 years ago

I'm a little confused by this one - I just plugged the above information in but had to make a bunch of changes so it's now:

fragment DogFragment on Dog {
  ownerConnection {
    edges {
      node {
        phoneNumber
      }
      purchaseDate
    }
  }
}

query allPets {
  petLists {
    pets {
      breed {
        ... on Dog {
          ...DogFragment
        }
      }
    }
  }
}

This executes with no issues, unless I'm misunderstanding the issue you're having?

norbertBergmann commented 2 years ago

Hey @norbertBergmann, can you please provide us with some more solid information to reproduce? We really need:

  • Some specific type definitions
  • If necessary, Cypher to add some test data to the database

We can't really extrapolate steps to reproduce from issues without this information. Thanks!

Hi @darrellwarde ,

thank you very much for investigating this problem.

In contrast to the side discussion about Union-types - my initial problem was related to the use of "aliases" in GQL-queries.

I try to give a concrete example.

type ContainerType {
  id: ID! @id
  name: String!
  specifiesContainers: [Container!]! @relationship(type: "hasContainer", properties: "CoT_Co_hasContainer", direction: OUT)
}

type Container {
  id: ID! @id
  name: String
  containsMaterial: [Material!]! @relationship(type: "hasMaterial", properties: "Co_Ma_hasMaterial", direction: OUT)
}

type Material {
  id: ID! @id
  name: String
}

interface CoT_Co_hasContainer @relationshipProperties {
  id: ID! @id
}

interface Co_Ma_hasMaterial@relationshipProperties {
  id: ID! @id
}

This types represent a concrete example of my given abstract example in my initial post. In the initial post the example was given like: Type_A- [RelationA]-Type_B-[RelationB]-Type_C Comparing with the concrete example like following: ContainerType- [specifiesContainers]-Container-[containsMaterial]-Material

So now we have 3 different kinde of queries to investigate:

1. This one works fine:

{
  ContainerType{
    AliasNestedLevel1: specifiesContainersConnection {
       edges {
        node {
          id
        }
      }
    }
}

Result:

{
  "data": {
    "ContainerType": [
      {
        "AliasLevel1": {
          "edges": [
            {
              "node": {
                "id": "1d298720-f32d-4277-a03e-3989129d2fb5"
              }
            }]
       }
   }
}
  1. This one returns empty element for "edges":
    {
    ContainerType{
    AliasNestedLevel1: specifiesContainersConnection {
       AliasNestedLevel1_1: edges {
        node {
          id
        }
      }
    }
    }

    Result:

    {
    "data": {
    "ContainerType": [
      {
        "AliasLevel1": {
          "AliasLevel1_1": []
        }
      }
    }
    }
  2. This one creates neo4j error due to an invalid cypher query:
    {
    ContainerType{
    specifiesContainers {
      AliasNestedLevel2: containsMaterialConnection {
        edges {
          node {
            id
          }
        }
      }
    }
    }
    }

    Result:

    {
    "errors": [
    {
      "message": "Failed to invoke function `apoc.cypher.runFirstColumn`: Caused by: org.neo4j.exceptions.SyntaxException: Variable `containsMaterialConnection` not defined 
darrellwarde commented 2 years ago

I see, thanks for the detailed explanation!

We will discuss in the team, but we might put this on hold for a few weeks, because we looking at completely overhauling how we generate the Cypher for this functionality, and we may just want to make sure that this case works once we have made those changes.

angrykoala commented 2 years ago

As mentioned by @darrellwarde, connections were completely overhauled in #2053 in the last release (3.8.0),

Currently, the error is reproducible but with the following message:

Neo4jError: Variable `containsMaterialConnection` not defined (line 13, column 65 (offset: 641))
"    WITH this_specifiesContainers { containsMaterialConnection: containsMaterialConnection } AS this_specifiesContainers"

The generated Cypher:

MATCH (this:`ContainerType`)
CALL {
    WITH this
    MATCH (this)-[thisthis0:hasContainer]->(this_specifiesContainers:`Container`)
    CALL {
    WITH this_specifiesContainers
    MATCH (this_specifiesContainers)-[this_specifiesContainers_hasmaterial_relationship:hasMaterial]->(this_specifiesContainers_material:Material)
    WITH collect({ node: { id: this_specifiesContainers_material.id } }) AS edges
    UNWIND edges as edge
    WITH collect(edge) AS edges, size(collect(edge)) AS totalCount
    RETURN { edges: edges, totalCount: totalCount } AS AliasNestedLevel2
    }
    WITH this_specifiesContainers { containsMaterialConnection: containsMaterialConnection } AS this_specifiesContainers
    RETURN collect(this_specifiesContainers) AS this_specifiesContainers
}
RETURN this { specifiesContainers: this_specifiesContainers } as this
neo4j-team-graphql commented 2 years ago

We've been able to confirm this bug using the steps to reproduce that you provided - many thanks @norbertBergmann! :pray: We will now prioritise the bug and address it appropriately.

angrykoala commented 2 years ago

This has been fixed in #2098