nestjs-query icon indicating copy to clipboard operation
nestjs-query copied to clipboard

Nested items wrongly returned as null

Open rostaingc opened this issue 1 year ago • 4 comments

Describe the bug

Nested items are returned as null, when they are not.

Example query

todoItems {
  id
  name
  toTags {
    tag {
      id
      name
    }
  }
}

You have two tables todoItems and tags as well as a many2many join table todo-to-tag. If there are soft deleted records inside the join table and you try to query the todoItems with their linked tags, all todoItems will be returned with the right number of toTags (join column) records but some of the tags inside the toTags will be returned as null.

Have you read the Contributing Guidelines?

Yes

To Reproduce Steps to reproduce the behavior:

I have created a fork of this repo and added an example and failling test that should pass once the issue is fixed. https://github.com/rostaingc/nestjs-query

Expected behavior All the items in the nested objects should be returned and not be null.

Additional context

When analyzing the SQL queries made by the nestjs query, what happens is:

  • Query A is made on the todoItems table.

  • Query B made on the join table: SELECT "todos-to-tags"."todoID", "todos-to-tags"."tagID" FROM "todos-to-tags" WHERE "todos-to-tags"."deleted" IS NULL AND "todos-to-tags"."todoID" IN (x) With x being the IDs returned by query A

  • Query C is made on the tags table: SELECT * FROM "tags" INNER JOIN "todos-to-tags" ON "tags".id = "todos-to-tags"."tagID" WHERE "tags"."deleted" IS NULL AND "todos-to-tags"."todoID" IN (x) AND "todos-to-tags"."tagID" IN (y) LIMIT z With x and y being the IDs returned by query B and z the total count of items returned by query B.

The issue here is that in query C, there is no check on deleted for the join table ("todos-to-tags"."deleted" IS NULL), thus leading to deleted join records being returned, and non deleted join items being left out because of the LIMIT.

When the data is then aggregated and put inside each nest item, some of the tags are null as they were cut off by the LIMIT.

The solution would be to make sure that there is a check for deleted IS NULL on all tables of the query in query C.

rostaingc avatar Oct 17 '24 10:10 rostaingc

Thanks for the detailed ticket, if I check the repo I do not see any additional commits? Could you create a PR to this repo with the test? That way I can more easily checkout that code locally.

TriPSs avatar Oct 17 '24 10:10 TriPSs

Sorry I didnt realize i had not pushed the last commit

Here is the PR for it.

https://github.com/TriPSs/nestjs-query/pull/308

rostaingc avatar Oct 18 '24 00:10 rostaingc

@TriPSs Let me know if you have enough info with the example in the PR or if you want any additional details Thanks

rostaingc avatar Oct 21 '24 01:10 rostaingc

So after investigating your PR (thanks for that!) I found what caused it, changes made in #99 are the cause (this thread more specifically).

Sadly I do not yet see a solution to the issue that will make sure both cases work, we would need to investigate if there is any, if not what behavior we want/expect to be correct.

Maybe @hedwiggggg can also weigh in here?

TriPSs avatar Oct 24 '24 14:10 TriPSs