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.22k stars 6.31k forks source link

Object-based conditions behave incorrectly in relational queries #3385

Open Diluka opened 5 years ago

Diluka commented 5 years ago

Issue type:

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

Database system/driver:

[ ] cordova [ ] mongodb [ ] mssql [x] mysql / mariadb [ ] oracle [ ] postgres [ ] 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: Author

@Entity()
export class Author {
    @PrimaryGeneratedColumn()
    id: number;

    @Column({
        collation: 'utf8mb4_unicode_ci'
    })
    name: string;

    @OneToMany(() => Photo, photo => photo.author)
    photos: Photo[];

    @OneToMany(() => Album, album => album.author)
    albums: Album[];
}

Album

@Entity()
export class Album extends BaseEntity {
    @PrimaryGeneratedColumn()
    id: number;

    @Column()
    name: string;

    @Column()
    authorId: number;

    @ManyToOne(() => Author)
    author: Author;

    @ManyToMany(() => Photo)
    @JoinTable()
    photos: Photo[];
}

Photo

@Entity()
export class Photo extends BaseEntity {
    @PrimaryGeneratedColumn()
    id: number;

    @Column({length: 500})
    name: string;

    @Column('text', {nullable: true})
    description: string;

    @Column()
    filename: string;

    @Column('int', {default: 0})
    views: number;

    @Column({default: false})
    isPublished: boolean;

    @Column()
    authorId: number;

    @ManyToOne(() => Author)
    author: Author;

    @ManyToMany(() => Album)
    albums: Album[];
}

query album with photos and author

Album.createQueryBuilder('album')
            .leftJoin('album.photos', 'photo')
            .leftJoinAndSelect('album.author', 'author')
            .addSelect(['photo.name', 'photo.filename'])
            .where({
                id,
                photos: { // problem here
                    id: Not(In([3, 4, 5])),
                    isPublished: true
                },
                author: {id: 2}
            })
            .getOne();

sql

SELECT `album`.`id` AS `album_id`, `album`.`name` AS `album_name`, `album`.`authorId` AS `album_authorId`, `photo`.`name` AS `photo_name`, `photo`.`filename` AS `photo_filename`, `photo`.`id` AS `photo_id`, `author`.`id` AS `author_id`, `author`.`name` AS `author_name` FROM `album` `album` LEFT JOIN `album_photos_photo` `album_photo` ON `album_photo`.`albumId`=`album`.`id` LEFT JOIN `photo` `photo` ON `photo`.`id`=`album_photo`.`photoId`  LEFT JOIN `author` `author` ON `author`.`id`=`album`.`authorId` WHERE `album`.`id` = ? AND NOT(`album`.`albumId` IN (?, ?, ?)) AND `album`.`authorId` = ? -- PARAMETERS: [1,3,4,5,2]

error

Error: ER_BAD_FIELD_ERROR: Unknown column 'album.albumId' in 'where clause'
pleerock commented 5 years ago

this where signature is used for find options that is being called from repository methods. Don't mix approaches. Try to replace your where with string-based where condition and reproduce issue again.

Diluka commented 5 years ago

it works with string-based where condition. maybe there shall be a doc or an error or what else to warn developers that object-based where condition can only effect on the target table.

pleerock commented 5 years ago

@Diluka docs can't cover everything that shall not be used. Usually what should be used is covered by docs.

Diluka commented 5 years ago

if I code like this

Album.createQueryBuilder("album")
      .leftJoin("album.photos", "photo", "photo.id NOT IN (:pids) AND photo.isPublished = true", { pids: [3, 4, 5] })
      .leftJoinAndSelect("album.author", "author")
      .addSelect(["photo.name", "photo.filename"])
      .where({
        id,
        author: { id: 2 },
      })
      .getOne();

It will generate the right sql and pass the parameters in a wrong order

SELECT `album`.`id` AS `album_id`, `album`.`title` AS `album_title`, `album`.`authorId` AS `album_authorId`, `photo`.`name` AS `photo_name`, `photo`.`filename` AS `photo_filename`, `photo`.`id` AS `photo_id`, `author`.`id` AS `author_id`, `author`.`name` AS `author_name`, `author`.`age` AS `author_age` FROM `album` `album` LEFT JOIN `album_photos_photo` `album_photo` ON `album_photo`.`albumId`=`album`.`id` LEFT JOIN `photo` `photo` ON `photo`.`id`=`album_photo`.`photoId` AND (`photo`.`id` NOT IN (?) AND `photo`.`isPublished` = true)  LEFT JOIN `author` `author` ON `author`.`id`=`album`.`authorId` WHERE `album`.`id` = ? AND `album`.`authorId` = ? -- PARAMETERS: [1,2,[3,4,5]]

will object-based where condition support relation query in the future?

Diluka commented 5 years ago

The object-based where condition is a good design. It let us code less to do the same thing. It's more readable, maintainable and syntax checkable. I hope it will get fully supported.

pleerock commented 5 years ago

@Diluka QueryBuilder is to build dynamic SQL string. If you want object-based approach just use find method - it covers most use cases.

Diluka commented 5 years ago

I know that. but now, everyone shall use object-based where condition with caution.

Object-based conditions now work fine without other parameters, even for relational queries. However, if other string-based conditions have parameters, it is possible that the SQL statement is generated correctly, but the parameter delivery order is wrong.

this is OK

Album.createQueryBuilder("album")
      .leftJoin("album.photos", "photo", "photo.isPublished = true")
      .leftJoinAndSelect("album.author", "author")
      .addSelect(["photo.name", "photo.filename"])
      .where({
        id,
        author: { id: 2 },
      })
      .getOne();

but this is not

Album.createQueryBuilder("album")
      .leftJoin("album.photos", "photo", "photo.id NOT IN (:pids) AND photo.isPublished = true", { pids: [3, 4, 5] })
      .leftJoinAndSelect("album.author", "author")
      .addSelect(["photo.name", "photo.filename"])
      .where({
        id,
        author: { id: 2 },
      })
      .getOne();

both generate the right sql, but the latter's parameter has wrong order.

maybe generate sql with named parameter can solve this. But I don't know if all drivers support this feature. https://github.com/mysqljs/mysql/blob/master/Readme.md#custom-format