typeorm / typeorm

ORM for TypeScript and JavaScript. Supports MySQL, PostgreSQL, MariaDB, SQLite, MS SQL Server, Oracle, SAP Hana, WebSQL databases. Works in NodeJS, Browser, Ionic, Cordova and Electron platforms.
http://typeorm.io
MIT License
34.07k stars 6.28k forks source link

findOne with relations does two queries. #5694

Open vikas-kyatannawar opened 4 years ago

vikas-kyatannawar commented 4 years ago

Issue type:

[ ] question [ X] bug report [ ] feature request [ ] documentation issue

Database system/driver:

[ ] cordova [ ] mongodb [ ] mssql [X ] mysql / mariadb [ ] oracle [ ] postgres [ ] cockroachdb [ ] sqlite [ ] sqljs [ ] react-native [ ] expo

TypeORM version:

[X ] latest [ ] @next [ ] 0.x.x (or put your version here)

Steps to reproduce or a small repository showing the problem: Details in the repository. https://github.com/vikas-kyatannawar/typeorm-find

Note: Issue doesn't exist for find() and findByIds() or in one to one relation. Only reproducible in one to many, many to one and in findOne() and findOneOrFail().
EricRabil commented 4 years ago

I too am experiencing this – I am able to circumvent this by calling find() instead, however it feels dirty to do so.

Moreover, it appears to be selecting all columns in both queries, not just the latter query. Here's the logged queries when I call findOne({ userID: "eric"})

query: SELECT DISTINCT "distinctAlias"."User_uuid" as "ids_User_uuid" FROM (SELECT "User"."uuid" AS "User_uuid", "User"."userID" AS "User_userID", "User"."passwordHash" AS "User_passwordHash", "User"."excludes" AS "User_excludes", "User_oAuthLinks"."uuid" AS "User_oAuthLinks_uuid", "User_oAuthLinks"."platform" AS "User_oAuthLinks_platform", "User_oAuthLinks"."platformID" AS "User_oAuthLinks_platformID", "User_oAuthLinks"."pipeDirection" AS "User_oAuthLinks_pipeDirection", "User_oAuthLinks"."userUuid" AS "User_oAuthLinks_userUuid" FROM "user" "User" LEFT JOIN "o_auth_link" "User_oAuthLinks" ON "User_oAuthLinks"."userUuid"="User"."uuid" WHERE "User"."userID" = $1) "distinctAlias" ORDER BY "User_uuid" ASC LIMIT 1 -- PARAMETERS: ["eric"]
query: SELECT "User"."uuid" AS "User_uuid", "User"."userID" AS "User_userID", "User"."passwordHash" AS "User_passwordHash", "User"."excludes" AS "User_excludes", "User_oAuthLinks"."uuid" AS "User_oAuthLinks_uuid", "User_oAuthLinks"."platform" AS "User_oAuthLinks_platform", "User_oAuthLinks"."platformID" AS "User_oAuthLinks_platformID", "User_oAuthLinks"."pipeDirection" AS "User_oAuthLinks_pipeDirection", "User_oAuthLinks"."userUuid" AS "User_oAuthLinks_userUuid" FROM "user" "User" LEFT JOIN "o_auth_link" "User_oAuthLinks" ON "User_oAuthLinks"."userUuid"="User"."uuid" WHERE ("User"."userID" = $1) AND "User"."uuid" IN ($2) -- PARAMETERS: ["eric","3da54b3d-df51-4712-bf7e-9d62e3ab0f02"]

And here's my schema

@Entity()
export class User extends BaseEntity {
  @PrimaryGeneratedColumn("uuid")
  uuid: string;

  @Column({ unique: true })
  userID: string;

  @Column()
  passwordHash: string;

  @OneToMany(type => OAuthLink, link => link.user, { eager: true })
  oAuthLinks: OAuthLink[];

  @Column("simple-array", { default: '' })
  excludes: string[];
}

From what I can infer from the logged queries, it is first selecting the primary column, while also selecting the entirety of the schema, then querying the table again with the primary column and the search criteria.

dimon82 commented 4 years ago

@pleerock any updates/plans on this one?

zWaR commented 4 years ago

@pleerock We're experiencing the same exact behavior. Are there perhaps any news regarding this?

Thanks so much for the awesome work on TypeORM, it's a great library.

zonorion commented 4 years ago

any updates? I replaced by query builder :(

DiMatteoL commented 3 years ago

Hey, Having the same problem on Postgres with find, and findAndCount. It only happens when I try to paginate (adding limit and/or skip to the filter options).

stelescuraul commented 3 years ago

same for Postgres when doing findOne or findOneOrFail.

nebkat commented 3 years ago

I believe this is related to:

https://github.com/typeorm/typeorm/blob/f27622daf9784d06496975bbbcacdb0938fd2c82/src/entity-manager/EntityManager.ts#L781-L786

A SELECT with take: 1 should really only be used if a One-to-Many or Many-to-Many relation is present, since for One-to-One or Many-to-One a simple LIMIT 1 would be sufficient. Even then take seems to be doing some relatively complex stuff and I have had some queries fail because of it, so I'm not sure it should ever be a default unless it is improved.

quyennt21 commented 3 years ago

any updates? I replaced by query builder :(

findOne with a primary key Ex: const trip = await this.tripRepository.findOne(trip.id, { select: ['id', 'corp_id', 'status'], relations: ['trips', 'trips.tripsItems'], })

robinjhuang commented 3 years ago

Also having this issue with find on Postgres.

alexisvisco commented 3 years ago

any updates? I replaced by query builder :(

findOne with a primary key Ex: const trip = await this.tripRepository.findOne(trip.id, { select: ['id', 'corp_id', 'status'], relations: ['trips', 'trips.tripsItems'], })

So weird it doesn't do the two queries with that workaround... So if you have a query with a condition and you do not know the primary key the orm is broken ? Interesting ...

nguyen190887 commented 3 years ago

Got the same issue with PostgreSQL.

If I passed options like below, it generates 2 queries

this.userRepository.findOne({
  where: {
    id: userId,
  },
  relations: ['relation1', 'relation2', 'relation3'],
});

After I removed the where and used the findOne(id?: string | number | Date | ObjectID, options?: FindOneOptions<Entity>) overload like below, it only generated 1 query

this.userRepository.findOne(userId, {
  relations: ['relation1', 'relation2', 'relation3'],
});

I think this issue should be addressed to avoid performance impact.

captainjapeng commented 3 years ago

I've experienced the same issue on PostgreSQL. But passing the primary key was not possible since I was using a composite key, as a workaround, I've used query builder and join the relations using leftJoinAndSelect and fetch the entity using getOne()

brbarnett commented 3 years ago

I see this behavior with find as well with PostgreSQL, specifically when take is included in FindManyOptions<Entity>. skip does not create this duplicate phenomenon.

tomi-mercado commented 3 years ago

Same problem with Postgres. Any update of this?

vkmita commented 2 years ago

I've ran into this issue at well (Postgres) +1

bradak1 commented 2 years ago

We are seeing this also with pg

morganfuchs commented 2 years ago

I also have the problem (MYSQL) for find() with several relations, a simple condition and a pagination

findAll({
    relations: ["relation1", "relation2"],
    where: { id = '' },
    take: 100,
    skip: 0,
});
cduff commented 2 years ago

any updates? I replaced by query builder :(

findOne with a primary key Ex: const trip = await this.tripRepository.findOne(trip.id, { select: ['id', 'corp_id', 'status'], relations: ['trips', 'trips.tripsItems'], })

Looks like this workaround isn't possible from version 0.3.0 as the findOne(primaryKey, ...) overload has been removed?!

cduff commented 2 years ago

FYI I'm working around this issue using the following extension method defined in my dataSource.ts.

import { EntityTarget, FindOneOptions } from "typeorm";
import { EntityManager } from "typeorm/entity-manager/EntityManager";

declare module "typeorm" {
  interface EntityManager {
    /**
     * As per findOne but works around https://github.com/typeorm/typeorm/issues/5694.
     * Calls find then returns first entity or null.
     */
    findFirst<Entity>(
      entityClass: EntityTarget<Entity>,
      options: FindOneOptions<Entity>
    ): Promise<Entity | null>;
  }
}

EntityManager.prototype.findFirst = async function (entityClass, options) {
  return (await this.find(entityClass, options))[0] ?? null;
};

UPDATE

Actually, simplest workaround is to just use like below as mentioned in the 0.3.0 release notes:

const [user] = await userRepository.find()
// user will be entity or undefined
schnitzel25 commented 2 years ago

Actually, on large data sets, having n queries without joins and hydrating the results after, is far more performant. I'm having such an issue now with TypeORM and I was hoping for lazy loading on getOne. No such luck.

Laravel's Eloquent ORM does this by default.

select from parent; select from child where id in ();

Depending on the number of lazy loaded relations, it can provide query speeds 3-5 times better.

cduff commented 2 years ago

Actually, on large data sets, having n queries without joins and hydrating the results after, is far more performant. I'm having such an issue now with TypeORM and I was hoping for lazy loading on getOne. No such luck.

Laravel's Eloquent ORM does this by default.

select from parent; select from child where id in ();

Depending on the number of lazy loaded relations, it can provide query speeds 3-5 times better.

@schnitzel25 Do you mean like the new relationLoadStrategy that came in https://github.com/typeorm/typeorm/releases/tag/0.3.0?

schnitzel25 commented 2 years ago

@cduff Yes, thank you, I guess that was too recent for me to find. Unfortunately, nestjs is not supporting 0.3.x yet, hope to get support soon.

rbrewer-echelon commented 2 years ago

I am seeing the same behavior in getMany and getManyAndCount via querybuilder -- swapping to getRawMany will eliminate the first distinct call (which in my case is taking upwards of 20 seconds??) but still not ideal.

Also not on 0.3.x yet, due to NestJS

jbgomond commented 1 year ago

Same problem here with version 0.3.10, the generated query when using findOne is doing a DISTINCT / sub query in FROM :(

jeanC-git commented 1 year ago

Same problem here with typeorm 0.3.10 const ticket = await this.ticketRepository.findOneBy({ id }); makes 2 querys SELECT DISTINCT "distinctAlias". .... SELECT ....

Any updates ?

daveslutzkin commented 1 year ago

Yes, we're seeing this issue too with typeorm 0.3.11. Exactly the same as @jeanC-git describes above. The ORM decides to do a superfluous second query when that shouldn't be needed.

daveslutzkin commented 1 year ago

The workaround for us is to change:

const object = await this.objectRepository.findOneBy({ id })

to

const [object] = await this.objectRepository.find({ where: { id } })
conoremclaughlin commented 1 year ago

Superfluous queries also apply in the case of combining getMany and limit on Postgres. As far as we can tell, it does the exact same query twice and doesn't use distinctAlias. We moved away from take and skip due to its inconsistent behavior and lack of limit on the initial query but we're still taking this performance hit.

ertl commented 1 year ago

Can confirm the same behavour on oracle

Superfluous queries also apply in the case of combining getMany and limit on Postgres. As far as we can tell, it does the exact same query twice and doesn't use distinctAlias. We moved away from take and skip due to its inconsistent behavior and lack of limit on the initial query but we're still taking this performance hit.

I can confirm the same behaviour with Oracle

ertl commented 1 year ago

It appears that the use of two queries in combination with a join and pagination works as designed. See the comments in the SelectQueryBuilder.ts class.

// for pagination enabled (e.g. skip and take) its much more complicated - its a special process // where we make two queries to find the data we need // first query find ids in skip and take range // and second query loads the actual data in given ids range

I was able to reproduce this problem with a OneToMany relationship. With a OneToOne relationship, only one query was created. Unfortunately, I do not understand why this is necessary. It doesn't seem to be necessary for my "simple" queries, but there may be more complex cases where it is necessary.

Howewer I found a Workaround, which works in my case:

 await repository.find({where: {relationId: relation.id}, take: 10})
@Entity()
export class ExampleEntity {
    @PrimaryGeneratedColumn()
    id: number

    @Column()
    name:string

    @ManyToOne('ExampleEntity', 'example')
    @JoinColumn()
    relation:RelationEntity

    @Column()
    relationId:number

    @CreateDateColumn()
    createdAt?: Date;
}
@Entity()
export class RelationEntity {
    @PrimaryGeneratedColumn()
    id: number

    @Column()
    name:string

    @OneToMany('ExampleEntity', 'relation')
    example2:ExampleEntity
}
jhfgloria commented 1 year ago

Are there any news on this topic? Right now I fall back to find, but it feels clumsy to request a list instead of an item.

sam-gronblom-rj commented 1 year ago

I just noticed recently that this was happening for us. I guess what's happening is that when you just use EntityManager.findOne with where typeorm cannot know whether what you are querying for is actually unique. So it has to first perform one query to find a single base entity to base the following lookup from.

However, if you already know that you are querying by the primary key of an entity, we shouldn't need to do this, because the uniqueness constraint on the PK should ensure that we only get one entity.

It seems one way to fix this would be to add a way to signify to Typeorm that you are querying by PK. Maybe a new method like EntityManager.findOneByPrimaryKey(findOptions). I guess Typeorm should also have the metadata to confirm that the passed values include all the required PK columns, but no more or less.

Until this gets a fix, I will create my own wrapper function using find and then check the result length is not 0 and not > 1.

conoremclaughlin commented 1 year ago

For any debugging depending on the version, I'll note that TypeORM will also print queries twice if you having logging set to all, and have printSql() in the call chain, so you need to be careful of removing that as a factor.

Redouane64 commented 1 year ago

Same problem here with typeorm 0.3.10 const ticket = await this.ticketRepository.findOneBy({ id }); makes 2 querys SELECT DISTINCT "distinctAlias". .... SELECT ....

Any updates ?

I have same issue with findAndCount and running version: ^0.3.11.

it is strange because it started to happen after database schema changes.

ortonomy commented 1 year ago

Came here after experiencing this today. this was reported over 3 years ago. TypeORM not interested in fixing it?

muhammetozendev commented 1 year ago

This solved the issue for my case:

await this.myRepo
      .createQueryBuilder()
      .setFindOptions(options)
      .getOne();

I just had a look at the source code and it seems that findOne is using entity manager and utilizing setFindOptions method in order to send query. When I create my own queryBuilder and set the find options immediately there, it sends a single query. Have a look at the source code:

https://github.com/typeorm/typeorm/blob/9471bfcebc6d6d6d193106a6076757338e15fa53/src/entity-manager/EntityManager.ts

I usually create an abstract repository class in my projects and implement all the common CRUD methods there. Then I create custom repositories which extend that abstract repository class and inject only child repository classes. That way, the implementation details will be abstracted away from the controllers and services. Here's my findOne method from abstract repository:

async findOne<T>(options: FindOneOptions<T>) {
    return await this.getRepository()
      .createQueryBuilder()
      .setFindOptions(options)
      .getOne();
  }

I hope this helps

mgohin commented 11 months ago

Still a problem with typeorm 0.3.17 and PostgreSQL

osmanyz commented 10 months ago

Are there any good news? I'm having the same issue, but couldn't find a good solution yet. Just'll be watching the issue.

Woodz commented 8 months ago

From looking at the Git history, it looks like this is intentional behaviour for handling pagination (https://github.com/typeorm/typeorm/commit/3951852bbfb332bf52734a79137d0a215eee6f24#diff-5725f0aa91cd5f33f502f605bcbc2ef2549c9f92a9ca1c984c03c2b8a80d73a5R303-R317). I believe that the bug is that TypeOrm assumes that by having a limit set on the query, the caller first needs to have the IDs of matching rows identified before the actual rows are retrieved.

rbrewer-echelon commented 6 months ago

confirmed const [entity] = await this.repository.find({ ... }) does not create the initial "bad" query.

Adding a take to it brings the superfluous query back.

rbrewer-echelon commented 6 months ago

await this.myRepo .createQueryBuilder() .setFindOptions(options) .getOne();

important to note this will create one query, however none of your relations will be automagically enforced for you.

flamewow commented 6 months ago

experiencing it on 0.3.20

Blaze34 commented 5 months ago

any updates?

Redouane64 commented 5 months ago

I managed to fix this issue by adding explicitly foreign key column that reference the parent table in one-to-many relationship.

here is column: https://github.com/Redouane64/birdhouse-api/blob/master/src/birdhouse/entities/birdhouse-occupancy.entity.ts#L27

In this demo entity I have one to many relationship between BirdhouseEntity and birdhouseOccupancyEntity. In BirdhouseOccupancyEntity I explicitly declated ubid column in the child entity.

Hopefully someone find this helpful!

clintonb commented 5 months ago

@Redouane64 this is how we've setup our relationships for a while, and it does not resolve the problem for us.

Also, I recall having a similar issue, to which this seems related (or perhaps a duplicate): #4998.

C-Collamar commented 3 months ago

Not really complaining, I'm just surprised that this has been an ongoing issue since 2020.

As I understand, there used to be a workaround in https://github.com/typeorm/typeorm/issues/5694#issuecomment-1077524726, until version typeorm@0.3.0 unsupported it. Also, another workaround mentioned https://github.com/typeorm/typeorm/issues/5694#issuecomment-2071078690 doesn't work for me, as it only throws a TypeORMError2.

Moving forward, I'm going with the same workaround mentioned in https://github.com/typeorm/typeorm/issues/5694#issuecomment-1368668150. Use the .find() in place of .findOne() (or its variants), then destructure the resulting array.

- const object = await this.objectRepository.findOneBy({ id }, relations: { ... });
+ const [object] = await this.objectRepository.find({ where: { id }, relations: { ... }});

The only downside I see in this workaround is that I can't use the take option in .find() because, for some reason, the unnecessary query reappears as mentioned in https://github.com/typeorm/typeorm/issues/5694#issuecomment-1999871532. The upside is that, this won't likely be an issue if you know that your query will only ever result in a single row of object fetched from the database.

CodingWatchCollector commented 2 months ago

Not really complaining, I'm just surprised that this has been an ongoing issue since 2020.

As I understand, there used to be a workaround in #5694 (comment), until version typeorm@0.3.0 unsupported it. Also, another workaround mentioned #5694 (comment) doesn't work for me, as it only throws a TypeORMError2.

Moving forward, I'm going with the same workaround mentioned in #5694 (comment). Use the .find() in place of .findOne() (or its variants), then destructure the resulting array.

- const object = await this.objectRepository.findOneBy({ id }, relations: { ... });
+ const [object] = await this.objectRepository.find({ where: { id }, relations: { ... }});

The only downside I see in this workaround is that I can't use the take option in .find() because, for some reason, the unnecessary query reappears as mentioned in #5694 (comment). The upside is that, this won't likely be an issue if you know that your query will only ever result in a single row of object fetched from the database.

We are using offset() and limit() suggested in this answer. The downside: Typeorm suggests to not use it for pagination.

vinicius-buildops commented 2 days ago

await this.myRepo .createQueryBuilder() .setFindOptions(options) .getOne();

important to note this will create one query, however none of your relations will be automagically enforced for you.

I'm currently using this workaround. Relations work just fine. Using MySql and TypeOrm 0.3.20. Replaced my findById with this.