typeorm: QueryBuilder produces wrong SQL with order by + nested fields, breaks pagination-related queries with skip() and take()

I just encountered this issue using findAndCount using typeorm@0.3.17. I wanted to sort joined entities by their primary key as they appear to be unsorted by default.

// example object graph
[
  {
    "id": 1,
    // nested is unsorted
    "nested": [{ "id": 5 }, { "id": 1 }, { "id": 3 }]
  }
]

When adding a nested sort order I no longer got the expected entries returned. In my test data set I got 7 entries whereas I was expecting 10.

// example call
findAndCount({
  order: { nested: { id: "ASC" } },
  skip: 0,
  take: 10,
  relations: {
    nested: true,
  },
})

If there is another way to sort nested relations when using findAndCount please let me know 😸

I inspected the generated SQL and believe I’ve spotted the issue.

-- 1st of 3 queries executed by typeorm
SELECT
    DISTINCT "distinctAlias"."Foo_id" AS "ids_Foo_id",
    "distinctAlias"."Foo**Foo_nested_id"
FROM
    (
        -- omitted for brevity
    ) "distinctAlias"
ORDER BY
    "distinctAlias"."Foo**Foo_nested_id" ASC,
    "Foo_id" ASC
LIMIT
    10

This is doing DISTINCT and LIMIT with the nested ids joined on. This is the problem. Here’s an example of the output. Notice that there are duplicates in Foo_id

This output is used in the 3rd and final query.

-- 3rd of 3 queries executed by typeorm
-- rest omitted for brevity
WHERE
    "Foo"."id" IN (10, 16, 4, 4, 4, 15, 6, 21, 21, 14) -- notice the duplicate ids

I don’t know how to fix this in typeorm though. But hope it can help fix the issue.

Adding my take to the discussion:

1. The simplest reproduction case:

await this.mainEntityRepository
      .createQueryBuilder('mainEntity')
      .leftJoin('mainEntity.joinedEntity', 'joinedEntity')
      .orderBy('joinedEntity.order', 'ASC')
      .take(<N>)
      .getMany();

(with multiple joinedEntitys per mainEntity)

1. Analysing current query plan:

SELECT DISTINCT
	"distinctAlias"."mainEntity_id" as "ids_mainEntity_id",
    "distinctAlias"."joinedEntity_order"
FROM (
	SELECT "mainEntity"."id" AS "mainEntity_id", "joinedEntity"."order" AS "joinedEntity_order"
    FROM "mainEntity"
    LEFT JOIN "joinedEntity" ON "joinedEntity"."mainEntityId"="mainEntity"."id"
) "distinctAlias"
ORDER BY "distinctAlias"."joinedEntity_order" ASC, "mainEntity_id" ASC
LIMIT <N>;

SELECT "mainEntity".*, "joinedEntity".*
FROM "mainEntity" LEFT JOIN "joinedEntity" ON "joinedEntity"."recipeId"="mainEntity"."id"
WHERE "mainEntity"."id" IN (<N ids from previous query>)
ORDER BY "joinedEntity"."order" ASC;

1. Suggestion of query plan:

SELECT "distinctAlias"."mainEntity_id" as "ids_mainEntity_id"
FROM (
	SELECT "mainEntity"."id" AS "mainEntity_id", "joinedEntity"."order" AS "joinedEntity_order"
    FROM "mainEntity"
    LEFT JOIN "joinedEntity" ON "joinedEntity"."mainEntityId"="mainEntity"."id"
) "distinctAlias"
GROUP BY "distinctAlias"."mainEntity_id"
ORDER BY min("distinctAlias"."joinedEntity_order") ASC, "mainEntity_id" ASC
LIMIT <N>;

SELECT "mainEntity".*, "joinedEntity".*
FROM "mainEntity" LEFT JOIN "joinedEntity" ON "joinedEntity"."recipeId"="mainEntity"."id"
WHERE "mainEntity"."id" IN (<N ids from previous query>)
ORDER BY "joinedEntity"."order" ASC;

Basically, replacing SELECT DISTINCT id, <other orderBy fields> FROM everything ORDER BY <all orderBy fields> by SELECT id FROM everything GROUP BY id ORDER BY <min of all orderBy fields>

1. Discussion

I would be happy to submit a PR, but it would be awesome having feedbacks from typeorm core contributor(s) beforehand!

Thanks for the reading 😉

@Nilegfx now that we have a better handle on the issue (thanks everyone), I will rename it from original Unexpected behaviour with skip() and take() re queries with a particular order-by clause to QueryBuilder produces wrong SQL with order by + nested fields, breaks pagination-related queries with skip() and take() in hopes it helps get the issue more attention.

I’ve just encountered this bug too. Thank you firxworx for pointing out that the issue is with the orderby on nested field! In my case, I have a parent entity (left side of one to many), and a child entity with composite primary key (right side of one to many). One column of 2-column composite primary key references parent entity. I make the query builder as such:

const sessions = await sessionsRepository
  .createQueryBuilder('sessions')
  .leftJoinAndSelect('sessions.packets', 'packets')
  .select([
    'sessions.id',
    'packets.sequence',
    //... something else
  ])
  .where({
    userId: user.id,
  })
  .take(limit)
  .skip(offset)
  .orderBy('sessions.id', 'DESC')
  .addOrderBy('packets.sequence', 'ASC') // !!! THE ISSUE IS ON THIS LINE
  .getManyAndCount();

If I have the noted line in place, I see TypeORM produces the query:

SELECT DISTINCT "distinctAlias"."sessions_id" as "ids_sessions_id", "distinctAlias"."sessions_id", "distinctAlias"."packets_sequence" FROM ...

Which is incorrect: it selects primary column of parent entity AND BOTH primary columns of child entity, thus the result will have more than one row for one parent entity.

For comparison, without the noted line, here’s what TypeORM generates:

SELECT DISTINCT "distinctAlias"."sessions_id" as "ids_sessions_id", "distinctAlias"."sessions_id" FROM ...

Which is correct: it will give only one row per one parent entity (session).

Still facing the issue as well with 0.3.11

The bug still persists

We are at 0.3.7 right now. I will check later and inform you.

@firxworx , thanks for your reply.

Unfortunately, I’m trying to add a sorting feature to my library https://github.com/benjamin658/typeorm-cursor-pagination, and it relies on the query builder, so it’s not possible to use the raw query.

@benjamin658 if you need an escape hatch you can always hand-bomb a raw SQL query yourself. Refer to EntityManager.query(): https://typeorm.io/#/entity-manager-api. Of course, be careful re potential for SQL Injection vulnerabilities, etc. as the onus is now on you to be safe.

A part of me is surprised that this issue from last year is still kicking about… how are a ton of devs not facing this exact issue given the overall popularity of TypeORM especially w/ postgres!?!?