Support for alternative loading strategies

[willsoto]

Apologies if this is documented somewhere, but I have not been able to find anything about it.

Is your feature request related to a problem? Please describe.

Not really a problem, but more of an inconvenience. It would be nice to be able to specify the Loading technique without having to drop to using the query builder.

Describe the solution you'd like

I would like to be able to specify the loading technique at the time I choose to load relationships.

Looking at the docs on Collections and seeing how relationships are currently loaded, one potential API for this could be something like:

type Strategy = 'lazy' | 'joined' | 'subquery' | 'selectin';

const author = orm.em.findOne(Author, '...', ['books', 'publishers']); // current

const author = orm.em.findOne(Author, '...', {
  books: {
    strategy: 'joined',
  },
  publishers: {
    strategy: 'subquery',
  },
}); // suggested

await author.books.init({
  strategy: 'joined',
});

Similar to SQLAlchemy, you could even specify the load type at the relationship level:

@Entity()
export class Book {
  @ManyToOne({
    entity: () => Author,
    strategy: 'joined',
  })
  author!: Author;
}

Describe alternatives you've considered

As mentioned above, the alternative right now seems to be to use the query builder.

Additional context

This feature seems fairly standard to ORMs. Some examples include SQLAlchemy and Objection.js

---

Thanks for the great project btw!

[B4nan]

Currently not supported, as you said, you need to use QB if you do not want to load entities as the ORM does it by default (1 query per entity).

The problem is not really in the query construction, but rather in the mapping. I am quite happy with how it works now as the mapping part is deadly simple, but I was planning to support mapping joined results too at some point of time.

So not a priority for now I would say, having quite a lot of things I want to focus in following months.

[willsoto]

Thanks for the quick response.

I would be happy to help implement this feature if it is only a matter of bandwidth.

If you are willing, you can point me in the right direction with any designs you might already have in mind and I can take a stab at it.

[B4nan]

Great to hear that!

I already started working on v4 and soon I want to split the ORM into several packages, so the time for a PR will come once that is done. Also I am not sure if we need all those options, the subquery one sounds a bit unnecessary (and also there is no support for subqueries in the QB currently, so that would be probably prerequisite). I would start with fetch joining as additional variant to the current way.

I definitely want to have this option on the property level too. Not sure about the API you proposed, but that is not really important for now. We can always rename things later, and I would start with version where one can set this only at the property level.

About the direction, I guess EntityLoader is one place to look, then ObjectHydrator, also AbstractSqlDriver/DatabaseDriver I guess, rest I am not sure right now :]

[willsoto]

Great!

I watch releases on this repo, but let me know once you feel you are ready to accept a PR. In the meantime, I'll poke around and get familiar with the codebase.

Also I am not sure if we need all those options, the subquery one sounds a bit unnecessary (and also there is no support for subqueries in the QB currently, so that would be probably prerequisite). I would start with fetch joining as additional variant to the current way.

I definitely want to have this option on the property level too. Not sure about the API you proposed, but that is not really important for now. We can always rename things later, and I would start with version where one can set this only at the property level.

I definitely think we would wanna spec out what would be supported. My examples were mostly for demonstrative purposes using prior art from a great ORM 😄

Once we get to that point, we can discuss that here.

[tahmidsadik]

Join with condition would be 🔥

[B4nan]

It's already supported (https://mikro-orm.io/docs/entity-manager/#searching-by-referenced-entity-fields), this issue is about being able to allow using single query and mapping the results to entities. Currently it will always fire 1 query per entity, with this change we could fire only 1 query for multiple entities.

[B4nan]

@willsoto So the split into packages is done (#475), feel free to PR this into dev branch where v4 development is happening.

As discussed, I would add the fetch joining as an alternative to current where in way. Not sure about the naming but we can improve that later, so for now let's use strategy and joined/selectin as you suggested (let's keep the selectin as default). I would add that only to the property options first, but the suggested change for populate is also fine - although I want to maintain the array way too, so only optionally.

Not sure about the suggestion about collections, as Collection.init() does fire only one query, there is no need for this unless you provide also the populate param.

Also I don't understand the lazy option, this is about the strategy (how), not about being lazy/eager (when). Relations are lazy by default, one can make them eager via eager: true.

[willsoto]

@B4nan That is great! Thanks for following up with me.

I'll spend some time hopefully tonight / this week and start browsing the dev branch to get familiar with it. Before I start writing any code, I'll comment here with the proposed naming and scope and make sure you are in agreement at least in principle. Like you said, naming and stuff can be improved later. 😄

[willsoto]

Ok. So I've spent some time browsing the code and I think I have a good sense of where stuff needs to happen, but I wanna make sure I got it right.

I will add a property named strategy to this interface:

mikro-orm/packages/core/src/decorators/Property.ts

Lines 58 to 62 in dfd731e

	export interface ReferenceOptions<T extends AnyEntity<T>> extends PropertyOptions {
	entity?: string | (() => EntityName<T>);
	cascade?: Cascade[];
	eager?: boolean;
	}

strategy will be defined for now as:

type Strategy = 'joined' | 'selectin';

It seems like I will have to make some changes to the QueryBuilder and the ObjectHydrator. You also mention the EntityLoader and the AbstractSqlDriver.

I haven't gotten a sense of the order in which things should happen, so any guidance there would be good. If you have any additional thoughts or preferences, please let me know. Also let me know if I am missing anything.

[B4nan]

Not sure if you will need to change QB itself, this should be more about how AbstractSqlDriver instructs/uses it. And I mentioned EntityLoader as that is where current populate implementation lives.

Agreed with the rest, most of the work will be probably in the hydrator part. Also to be able to map multiple entities from one query, you will probably need to explicitly select all the fields aliased so you can be sure they won't collide (e.g. Author.name and Publisher.name).

[B4nan]

So I finally had some time to work on this. Although I was planing to do something else, until I realised that what we merged was really just a proof of concept and it was blocking me from implementing the lazy scalar props.

So with my recent changes:

• composite keys should be supported (you always used fieldNames[0])
• we load relation props (you only used scalar props in select, no m:1 or 1:1 owners)
• we handle this on em.find() too, but the mapping part is still missing
• lazy scalar props should be supported too with the joined strategy

I also needed to redesign the populate param so we can actually bare the information for nested populate, as your version simply skipped unknown keys - and for nested population all of them were unknown as they were concatenations (e.g. books.author). So we now have a PopulateMap<T> where one can also set the strategy for given relation, overriding what is in the property metadata:

await orm.em.findOneOrFail(Author2, { id: author2.id }, { populate: { books: LoadStrategy.JOINED } });

I also added few more test.todos. I hope you will have some more time for this, as there is still a lot to do. I would focus on supporting the em.find() mapping first, followed by mapping of nested relations (it looked like we support only 1 level currently). If you won't have time for this in the near future, please let me know so I can focus on it myself, as currently this is somewhat blocking the release (we still have few weeks for sure, will be releasing the first alpha now, but a WIP like this shouldn't be part of future RC versions or late betas).

My changes are not yet merged, but you can see it here: #585. Need to add more tests first to verify it actually works with all the use cases, also lazy props are not yet working in mongo, but should be merging this later today.

[willsoto]

If you won't have time for this in the near future, please let me know so I can focus on it myself, as currently this is somewhat blocking the release (we still have few weeks for sure, will be releasing the first alpha now, but a WIP like this shouldn't be part of future RC versions or late betas).

I honestly can't commit since my schedule has become increasingly busy in the past couple of weeks. As much as I would like to see the feature through to completion, I don't want to be the reason you are holding up a release.

If there are any smaller pieces of work (<5-6 hours) that can be done in isolation or with minimal interaction (thinking about rebases, merge conflicts, etc) feel free to point me to them.

[B4nan]

No worries, I should be able to finalize this myself.

If there are any smaller pieces of work (<5-6 hours) that can be done in isolation or with minimal interaction (thinking about rebases, merge conflicts, etc) feel free to point me to them.

Would be great if you could prepare some of the missing tests, and write a bit of docs, that should fit in few hours for sure :]

[willsoto]

Would be great if you could prepare some of the missing tests, and write a bit of docs, that should fit in few hours for sure :]

You got it. I'll wait till #585 is merged for tests but I can start writing docs tomorrow most likely.

[B4nan]

Closing as finalized via #600 (and 3b0384e, 90dc71f and c25782e), feel free to try out and add more tests if you can think of something missing. Same applies to the docs, it could be much more verbose, but now I want to focus on actual development, time for improving docs will come later.

[wSedlacek]

@B4nan It would be great to have some additional documentation on when you would want to use what strategy.
The existing documentation does a good job telling you that you can use either strategy, but the trade offs between the different strategies are still unclear.

I would expect it would have to do with the latency to the database, the complexity of the sql statement, etc.
However I am just speculating with that.