Filtering on relations breaks pagination

[mwoelk]

Describe the bug
When performing a query that includes or-filters with overlapping results on relations, it generates LEFT JOIN clauses (tested in postgres) in the query which leads to multiple rows in the database result containing the base entity. If pagination is used the query also contains a LIMIT. If the limit is e.g. set to 10 and we get more than 10 overlapping results the query returns only this one base entity instead of 10 different ones.

To Reproduce
Steps to reproduce the behavior:

1. Create an entity (E) containing a field (E.F) and two filterable relations (R1, R2) containing at least one filterable field each (R1.F, R2.F)
2. Create multiple dummy entities containing a larger amount of related entitites
3. Query E using pagination and or-filter on values of E.F, R1.F and R2.F in a way that some entities match for multiple of those fields

Reproduction in the "basic"-example

1. In the TodoItemDTO change the two CursorConnection fields to FilterableCursorConnection to make them filterable.
2. Run the following query in the modified "basic"-example:

{
  todoItems(
   paging: {first: 2}
   filter: { 
     or: [
       { id: { eq: 2 } }, 
       { subTasks: { title: { like: "Create Nest App%" } } } 
     ]
   }
) {
    edges {
      node {
        id
        title
      }
    }
  }
}

3. It returns only the ToDo with id 1 instead both 1 and 2. Changing the first parameter of paging to 3 leads to both ToDos 1 and 2 to be returned.

I started working on fixing things here (already modified the basic example and added a failing test case): https://github.com/mwoelk/nestjs-query/tree/issue954

Analysis based on the modified "basic" example
Ignoring the paging argument, the underlying database query would return something similar to the following table (due to the JOINs involved):

todoId	subTaskTitle	...
1	Create Nest App - Sub Task 1	...
1	Create Nest App - Sub Task 2	...
1	Create Nest App - Sub Task 3	...
2	Create Entity - Sub Task 1	...

Due to the first argument set in paging, the database query is added an LIMIT (first + 1) which leads to the behaviour described above. E.g. if first < 3, only the rows with an id of 1 are returned.

Expected behavior
The number of results should match the expected pagination size if there are enough results available.

Desktop (please complete the following information):

• Node Verson v14.13.1
• Nestjs-query Version v0.21.2
• Using Typeorm with postgres adapter

[mwoelk]

Relating to this issue in the typeorm repo: typeorm/typeorm#4742 (comment) skip and take should be used instead of offset and limit when working with JOINS.

This would require a change here:

nestjs-query/packages/query-typeorm/src/query/filter-query.builder.ts

Line 116 in beb96c3

return qb.limit(paging.limit).offset(paging.offset);

The problem with skip/take is that it "gets all the results, merge them together into entities and then apply skip / take, which performance-wise a bit slower". For simple queries that don't work on relations limit/offset would be the better way to go. So a change should check wheater there's a join required and if so apply skip/take and otherwise use the faster offset/limit.

Maybe a small hint in the docs on the performance impact when using such queries on large relations might be nice to add as well.

[doug-martin]

Fixed in v0.24.1.