typeorm icon indicating copy to clipboard operation
typeorm copied to clipboard

feat: add join operator

Open iJhefe opened this issue 3 years ago • 3 comments

this feature enable to explicit what join should be used in find options

Description of change

with this we will be able to explicit define how relations should be joined using

// only users with posts
const users = await manager.find(User, {
   relations: {
    posts: InnerJoin()
  }
})

// users with posts or not
const users = await manager.find(User, {
  relations: {
    posts: LeftJoin()
  }
})

// using LeftJoin operator is the same as (at this moment)
const users = await manager.find(User, {
  relations: {
    posts: true
  }
})

using join operator we can be sure about how relations are being joined

Pull-Request Checklist

  • [x] Code is up-to-date with the master branch
  • [x] npm run format to apply prettier formatting
  • [x] npm run test passes with this change
  • [x] This pull request links relevant issues as Fixes #0000
  • [x] There are new or updated unit tests validating the change
  • [ ] Documentation has been updated to reflect this change
  • [x] The new commits follow conventions explained in CONTRIBUTING.md

iJhefe avatar Jan 14 '23 19:01 iJhefe

related #9708 #9395

iJhefe avatar Jan 14 '23 20:01 iJhefe

Not sure if I want to add additional complication to the relations.

What if I suggest an alternative implementation for the inner join:

// only users with posts
const users = await manager.find(User, {
   relations: {
    posts: true,
  },
  where: {
    posts: Not(IsNull())
  }
})

or maybe other alternatives with where applied?

pleerock avatar Feb 06 '23 17:02 pleerock

// only users with posts
const users = await manager.find(User, {
   relations: {
    posts: true,
  },
  where: {
    posts: Not(IsNull())
  }
})

The result data in the inner join query is the same.

But the query you suggested is

  1. creating the join result data
  2. filter the data (filter post null data)

so slower than inner join

kyungyeon-byun-imweb-me avatar Feb 13 '23 09:02 kyungyeon-byun-imweb-me

From the performance perspective it's the same.

From the code perspective I find current solution acceptable: it's both flexible and built on exist concepts which make logically sense. I don't see a point to add more complications with different new scenarios like this one.

I'll have to close it until this feature gets more popularity.

pleerock avatar Apr 15 '23 08:04 pleerock

Hi @pleerock ,

Why did I get the error "This relation isn't supported by given find operator" while testing your solution?

cjnoname avatar Apr 20 '23 05:04 cjnoname