Empty list returned from nested filters, mis-assigned filter object

[henrygeddes]

I've recently run into a bug which causes the first list of a sub node of a query return as empty.

We are using:
neo4j-graphql-js: ^2.19.2
apollo-server: ^2.20.0
neo4j:4.1.1 (Docker)
Tested through GraphQLPlayground

I've reduced my query into the simplest reproducible state, showing:

Example Query

{
  Department(filter: { id: 1 }) {
    name
    
    crew {
      assignments(filter: { id: 1}) {
       	id
      }
    }

    schedule {
      # actuals (
      #   first: 0
      # ) {
      # 	value
      # }

      averageHoursOverrides(
        filter:  {hours_gte: 1}
      ) {
        hours
      }
    }
  }
}

Result 1

{
  "data": {
    "Department": [
      {
        "name": "Animation",
        "crew":  [assignment[], ...assignment[]], # populated correctly
        "schedule": {
          "averageHoursOverrides": [] #empty list
        }
      }
    ]
  }
}

Enabling debug output shows the generated query:

MATCH (`department`:`Department`) WHERE (`department`.id = $filter.id) RETURN `department` { .name ,crew: [(`department`)<-[:`BELONGS_TO`]-(`department_crew`:`Crew`) | `department_crew` {assignments: [(`department_crew`)-[:`ASSIGNED_TO`]->(`department_crew_assignments`:`Assignment`) WHERE (`department_crew_assignments`.id = $1_filter.id) | `department_crew_assignments` { .id }] }] ,schedule: head([(`department`)-[:`HAS_SCHEDULE`]->(`department_schedule`:`Schedule`) | `department_schedule` {averageHoursOverrides: [(`department_schedule`)<-[:`CONNECTED_TO`]-(`department_schedule_averageHoursOverrides`:`AverageHoursOverride`) WHERE (`department_schedule_averageHoursOverrides`.hours >= $1_filter.hours_gte) | `department_schedule_averageHoursOverrides` { .hours }] }]) } AS `department`

With the filter object

{
  "offset": 0,
   "first": -1,
  "filter": {
     "id": 1
   },
   "1_filter": {
      "id": 1
   }
}

Note in the filter for AverageHoursOverride, WHERE ('department_schedule_averageHoursOverrides'.hours >= $1_filter.hours_gte
$1_filter.hours_gte does not exist (and instead this filter item takes the value of the first filter)

Result 2
By uncommenting actuals (another list), we get the response:

{
  "data": {
    "Department": [
      {
        "name": "Animation",
        "crew": [assignment[], ...assignment[]], # populated correctly
        "schedule": {
          "actuals": [], # empty list
          "averageHoursOverrides": [<AverageHourOverride>] # populated correctly
        }
      }
    ]
  }
}

Along with the generated query

MATCH (`department`:`Department`) WHERE (`department`.id = $filter.id) RETURN `department` { .name ,crew: [(`department`)<-[:`BELONGS_TO`]-(`department_crew`:`Crew`) | `department_crew` {assignments: [(`department_crew`)-[:`ASSIGNED_TO`]->(`department_crew_assignments`:`Assignment`) WHERE (`department_crew_assignments`.id = $1_filter.id) | `department_crew_assignments` { .id }] }] ,schedule: head([(`department`)-[:`HAS_SCHEDULE`]->(`department_schedule`:`Schedule`) | `department_schedule` {actuals: [(`department_schedule`)<-[:`CONNECTED_TO`]-(`department_schedule_actuals`:`Actuals`) | `department_schedule_actuals` { .crewWeeks }][..0] ,averageHoursOverrides: [(`department_schedule`)<-[:`CONNECTED_TO`]-(`department_schedule_averageHoursOverrides`:`AverageHoursOverride`) WHERE (`department_schedule_averageHoursOverrides`.hours >= $3_filter.hours_gte) | `department_schedule_averageHoursOverrides` { .hours }] }]) } AS `department`

And filter object

{
   "offset": 0,
   "first": -1,
   "filter": {
     "id": 1
   },
   "1_first": 0,
   "3_filter": {
     "hours_gte": 0
   },
   "1_filter": {
     "id": 1
   }
}

Again we have WHERE ('department_schedule_averageHoursOverrides'.hours >= $3_filter.hours_gte)
where $3_filter.hours_gte is correct

Summary
It looks like something is causing the second filter item to be mis-assigned, taking the value of the first filter. This only occurs for the second filter, all subsequent filters take the correct value.

Please let me know if there's anymore information I can provide.

[henrygeddes]

PR for temporary fix
I've spent a bit of time looking into this issue as it was affecting multiple queries on a work related project.
I have found a temporary fix which I've created a PR for:
#606

What I discovered:
buildCypherSelection fails to set the filter object for nested queries. This is due to paramIndex being reset for each sub query, which then overwrites an existing filter in the filter object.

Example query:

{
  Department(filter: { id: "department-id" }) {
    crew {
      leave(filter: {id: "leave"}){
        id
      }
      assignments(filter: { id: "crewAssignments"}) {
        id
      }
    }
    
    vendorInstances {
      assignments(filter: { id: "vendorAssignments"}) {
        id
      }
      
      department(filter: { id: "department"}) {
        id
      }
    }
 }
}

As this object is built recursively the filters for the second sub query are applied first:
1_filter: { id: "vendorAssignments"}
3_filter: { id: "department"}

paramIndex is then reset to 1 for the next subquery, which then overwrites the above object with:
1_filter: { id: "leave" }
3_filter: { id: "crewAssignments" }

This gives us a final filter object of:

{
  "offset": 0,
  "first": -1,
  "filter": {
    "id": "department-id"
  },
  "1_filter": {
    "id": "leave"
  },
  "3_filter": {
    "id": "crewAssignments"
  },
  "cypherParams": {
    "username": "<removed>"
  }
}

As a temporary fix I have added an additional parameter to buildCypherSelection which tracks paramIndex's that have already been used. Then instead of incrementing paramIndex we use getNextParamIndex which finds the highest used index and increments it by 1. This ensures paramIndex is always unique for any part of the selection.

This results in a filter object of:

neo4j-graphql-js {
  "offset": 0,
  "first": -1,
  "filter": {
    "id": "department-id"
  },
  "11_filter": {
    "id": "vendorAssignments"
  },
  "14_filter": {
    "id": "department"
  },
  "2_filter": {
    "id": "leave"
  },
  "5_filter": {
    "id": "crewAssignments"
  },
  "cypherParams": {
    "username": "<removed>"
  }
}

This is a pretty hacky fix that is intended as a temporary solution (and to help explain the core issue). Our project is now using a fork with this fix and it is working as intended, but I understand there's likely a much better way to handle this. I'm also aware that you are planning on refactoring recurse and paramIndex which will probably fix this issue.

@michaeldgraham
If you need any more information or have any questions please feel free to contact me (reply to this issue and I'll give you my personal email).

[michaeldgraham]

#608