Query in getModelsPerType() only by ids

[netzknecht]

There is no need to use other query conditions (e.g. local and global scopes) here again, because they are already present in the query in the "getIdAndOrderAttributes()"-method. So, it's sufficient enough to load the models by id.

I've recently posted that pull request at protonemedia/laravel-cross-eloquent-search, but that one is closed because I had to delete the forked head repository, to switch to a fork of your fork artkonekt/search.

[fulopattila122]

Hello, and sorry for the slow reaction.

Please apply the StyleCI patch https://github.styleci.io/analyses/o7kl0P and push the changes to the PR.

Also, could you tell me what the goal of the change is? What will be different after merging this PR?

[fulopattila122]

Unfortunately, the changes (as of now) break the eager load functionalities, which can result in a significant performance penalty:

see the test run

[netzknecht]

Commit f7c8caf changes the behavior the relevant models gets loaded from the database. Because all query conditions for searching and filtering are already applied on the queries per model before and the models gets loaded explicitly by there ids, other conditions applied by global scopes are not necessary again and reduce performance. The getFreshBuilder() method returns a query builder with global scopes, newQueryWithoutScopes() not.

Commit dc8977e forwards methods to the query builder of the models to search for. It allows to add constraints to all models in a handy way, instead of repeating this step for all models.

[netzknecht]

Unfortunately, the changes (as of now) break the eager load functionalities, which can result in a significant performance penalty:

see the test run

I just saw that too, unfortunately I don't have much time to researching the cause.

[fulopattila122]

I just saw that too, unfortunately I don't have much time to researching the cause.

Me neither. Let's keep this open and once we have some time, let's come back and see the possible performance improvements.