Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow mongodb projection #3756

Closed
wants to merge 6 commits into from
Closed

Conversation

rustamwin
Copy link
Contributor

Fixes #3266

@pleerock
Copy link
Member

We can't go with such changes. If we keep going with mongodb-specific changes entity-manager/repository is going to be a mess for rdbms and we'll always be blocked on some absolute incompatible features.

I suggest to remove dependency of MongoEntityManager from EntityManager (e.g. remove extends) and completely re-implement MongoEntityManager with mongodb-specific features.

@ElianCordoba
Copy link

Seams reasonable, could you provide some initial code/steps to get to that? I don't feel confident enough to take that task ¯_(ツ)_/¯

@pleerock
Copy link
Member

  1. remove extends EntityManager
  2. implement same functions we have before in EntityManager
  3. start improving mongodb support

@jyang0110
Copy link

why am I getting redirected to here when no solution is given...
Thanks for the work tho

@phil294
Copy link
Contributor

phil294 commented Apr 22, 2019

@rustamwin I am trying to understand your pull request: Why is it necessary to add a new projection option? There is already a
https://github.com/typeorm/typeorm/blob/master/src/entity-manager/MongoEntityManager.ts#L96
method which changes select into projection.
Wouldnt the linked issue (#3266) be fixed by simply adjusting convertFindOptionsSelectToProjectCriteria so that nested keys in select are put into the projection too?

since sth like XY.find({ where: { ...conditions }, select: [ ...fields ] }) already works perfectly.

Thanks!

@rustamwin
Copy link
Contributor Author

Why is it necessary to add a new projection option? There is already a
https://github.com/typeorm/typeorm/blob/master/src/entity-manager/MongoEntityManager.ts#L96
method which changes select into projection.

because select doesn't allow to use mongo projection, but just allow fields
mongodb has a magical projection 😄
https://docs.mongodb.com/manual/reference/operator/projection/

@phil294
Copy link
Contributor

phil294 commented Apr 23, 2019

@rustamwin I'm afraid I dont understand. I know that for "select" syntax in MongoDB, you have to use projection. But thats what https://github.com/typeorm/typeorm/blob/master/src/entity-manager/MongoEntityManager.ts#L96 already does, doesnt it? It transforms select into projection when using the mongo driver. convertFindOptionsSelectToProjectCriteria already transforms a ["name", "address.pincode"] into a { name: 1, 'address.pincode': 1 } projection. So it seems to me the nested mongo "select" (internally handled as a projection) should already work. What am I missing?

@gruckion
Copy link

Is this going to be merged? Would be nice to have this support.

@kateile
Copy link
Contributor

kateile commented Jul 27, 2019

waiting for this merge. projection is awesome feature in mongoDb

@robbyemmert
Copy link

Is this going to be merged? I think this will help my situation (and probably a lot of others, by the look of it).

@rustamwin
Copy link
Contributor Author

Won't be merged before this:

I suggest to remove dependency of MongoEntityManager from EntityManager (e.g. remove extends) and completely re-implement MongoEntityManager with mongodb-specific features.

@MowiliMi

This comment has been minimized.

@oussaki

This comment has been minimized.

@phil294
Copy link
Contributor

phil294 commented Aug 21, 2020

Wow since a year and this is still , open. i think i did a bad decision using this ORM. this feature is basic.

@oussaki TypeORM is an Open Source project. Feel free to contribute to this pull request yourself. Otherwise I doubt demanding features from volunteers as if they owe you something will be helpful to anyone.

I found this etiquette guide to make sense in most situations

@imnotjames
Copy link
Contributor

@phil294 I agree with you - one, on your most recent comment about courtesy and etiquette.

Thank you.

But secondly, on reusing select. Reusing select would bypass a lot of the work pleerock had suggested and would maintain a smaller, more cohesive API. We wouldn't have lots of custom mongodb options.

I'd personally prefer that we had a not as great mongodb experience for a more consistent experience across all of these data stores.

If you're after something with super deep mongodb feature support a generic ORM probably ain't gonna cut it. It'll never have feature parity with something that's built out with mongodb as it's primary goal. It can't.


I also see that more or less development on this PR has stalled for a few years. I'm gonna close it out because otherwise we end up with folks spamming it with.. well, various comments that aren't constructive towards the PR itself.

Please feel free to open a new PR and definitely continue discussion of this feature request in #3266

@imnotjames imnotjames closed this Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nested Select in mongodb