framework: Constraints on eager-loaded hasMany doesn't retrieve data correctly

@giovannipds Your issue is not related to this thread.

It’s not possible to use a relationship with an orWhere() like this.

@azcoppen @themsaid I’ve updated the modified query builder to handle the belongToMany relationship. Not sure if this takes care of the remaining relationships or not… just using it for my own use case (GraphQL). I’m also not a fan of using the ReflectionMethod, but since the methods I need access to are protected, it seemed like the lesser of two evils (rather than refactoring the BelongToMany relationship).

If I can find some time this weekend, I’ll create some tests and check out the other relationships to see if everything still works and possibly submit a PR. Not sure what to do about SQLite in this case though since it won’t accept the supplied query.

Usage

$users = User::all();

$users->fetch(['posts' => function ($q, $user) {
    $q->take(2);
}]);

Collection Macro

Collection::macro('fetch', function ($relations) {
    if (count($this->items) > 0) {
        if (is_string($relations)) {
            $relations = [$relations];
        }

        $query = $this->first()->newQuery()->with($relations);
        $this->items = app(QueryBuilder::class)->eagerLoadRelations($query, $this->items);
    }
    return $this;
});

Updated Query Builder

use Closure;
use ReflectionMethod;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Database\Eloquent\Model;

class QueryBuilder
{
    /**
     * Eager load relationships on collection.
     *
     * @param  Builder $builder
     * @param  array   $models
     * @return array
     */
    public function eagerLoadRelations(Builder $builder, array $models)
    {
        foreach ($builder->getEagerLoads() as $name => $constraints) {
            if (strpos($name, '.') === false) {
                $models = $this->loadRelation($builder, $models, $name, $constraints);
            }
        }

        return $models;
    }

    /**
     * Eagerly load the relationship on a set of models.
     *
     * @param  Builder  $builder
     * @param  array    $models
     * @param  string   $name
     * @param  Closure  $constraints
     * @return array
     */
    protected function loadRelation(Builder $builder, array $models, $name, Closure $constraints)
    {
        $relation = $builder->getRelation($name);
        $queries = $this->getQueries($builder, $models, $name, $constraints);
        $related = $queries->first()->getModel();

        $bindings = $queries->map(function ($query) {
            return $query->getBindings();
        })->collapse()->toArray();

        $sql = $queries->map(function ($query) {
            return '(' . $query->toSql() . ')';
        })->implode(' UNION ALL ');

        $table = $related->getTable();
        $results = \DB::select("SELECT `{$table}`.* FROM ({$sql}) AS `{$table}`", $bindings);
        $hydrated = $this->hydrate($related, $relation, $results);

        return $relation->match($models, $related->newCollection($hydrated), $name);
    }

    /**
     * Get queries to fetch relationships.
     *
     * @param  Builder  $builder
     * @param  array    $models
     * @param  string   $name
     * @param  Closure  $constraints
     * @return array
     */
    protected function getQueries(Builder $builder, array $models, $name, Closure $constraints)
    {
        return collect($models)->map(function ($model) use ($builder, $name, $constraints) {
            $relation = $builder->getRelation($name);

            $relation->addEagerConstraints([$model]);

            call_user_func_array($constraints, [$relation, $model]);

            if (method_exists($relation, 'getSelectColumns')) {
                $r = new ReflectionMethod(get_class($relation), 'getSelectColumns');
                $r->setAccessible(true);
                // Not sure if this is absolutely correct...
                $select = $r->invoke($relation, ['*']);
                $relation->addSelect($select);
            }

            $relation->initRelation([$model], $name);

            return $relation;
        });
    }

    /**
     * Hydrate related models.
     *
     * @param  Model    $related
     * @param  Relation $relation
     * @param  array    $results
     * @return array
     */
    protected function hydrate(Model $related, Relation $relation, array $results)
    {
        $models = $related->hydrate($results, $related->getConnectionName())->all();

        if (count($models) > 0 && method_exists($relation, 'hydratePivotRelation')) {
            $r = new ReflectionMethod(get_class($relation), 'hydratePivotRelation');
            $r->setAccessible(true);
            $r->invoke($relation, $models);
        }

        return $models;
    }
}

@derekmd Naturally, I think that’s a good idea. Would you write it?

We could also throw an exception (that links to the documentation) if someone tries to apply a limit to their relationship when eager loading.

Duplicate issues:

• https://github.com/laravel/framework/issues/18014
• https://github.com/laravel/framework/issues/4835
• https://github.com/laravel/framework/issues/15585

As noted by @staudenmeir, his pull request was declined by Taylor due to the maintenance burden for a solution not supported by all databases that Laravel projects run.

Since this problem has come up so much and it can now be fixed by a third-party package, maybe the best solution is to update https://laravel.com/docs/5.7/eloquent-relationships#eager-loading under heading “Constraining Eager Loads” to clearly communicate that limit() and take() aren’t supported unless that package is installed and MySQL, etc. are the most recent version?

I released the rejected PR as a package: https://github.com/staudenmeir/eloquent-eager-limit

OMG… Why this issue has not been fixed yet??
It has been 2 years…