laravel / ideas

Issues board used for Laravel internals discussions.
939 stars 28 forks source link

Add constraints in relation definition #1584

Open tteze opened 5 years ago

tteze commented 5 years ago

We may can define constraints on eager or query relationships call. This can be usefull to complete query scopes wich depend of the parent model.

This will be apply in addEagerConstraints by the Relation method :

public function myRelation()
{
    return $this->hasMany('App\User')->addEagerConstraint(function ($query, $models) {
            // add constraints depending of returned models
    });
}

This will be apply in addEagerConstraints by the Relation method :

public function myRelation()
{
    return $this->hasMany('App\User')->addConstraint(function ($query) {
            // add constraints only on query relations (not in eagers loading)
    });
}

This is the code to add in the Relation Class

 /**
     * All callables to apply on query relations.
     *
     * @var array
     */
    protected $queryConstraints = [];

    /**
     * All constraints to apply on eager loading.
     *
     * @var array
     */
    protected $eagerConstraints = [];

    /**
     * Add a constraint on the relation query.
     *
     * @param Closure $constraint
     * @return static
     */
    public function addConstraint(Closure $constraint)
    {
        $this->queryConstraints[] = $constraint;

        return $this;
    }

    /**
     * Add a constraint on the eager load of the relation.
     *
     * @param Closure $constraint
     * @return static
     */
    public function addEagerConstraint(Closure $constraint)
    {
        $this->eagerConstraints[] = $constraint;

        return $this;
    }

    /**
     * Add a constraint on the relation query.
     *
     * @param Builder $query
     * @return Builder
     */
    public function applyQueryConstraints(Builder $query)
    {
        foreach ($this->queryConstraints as $constraint) {
            call_user_func($constraint, $query);
        }

        return $query;
    }

    /**
     * Add a constraint on the relation query.
     *
     * @param Builder $query
     * @param array $models
     * @return Builder
     */
    public function applyEagerConstraints(Builder $query, array $models)
    {
        foreach ($this->eagerConstraints as $constraint) {
            call_user_func($constraint, $query, $models);
        }

        return $query;
    }
staudenmeir commented 5 years ago

Can you provide some examples where this would be useful?

tteze commented 5 years ago

Actually you can't filter on posts depending of a user parameter in the declaration of his relationship. I think we may use this:

public function posts()
{
    return $this->hasMany(Post::class)
        ->addEagerConstraint(function ($query, $models) {
            $user = array_first($models);
            return $query->where('type', $user->is_cool ? 'someType' : 'anotherType' );
        })
        ->addConstraint(function ($query) {
            $caseWhen = "case when users.is_cool = 1 then 'someType' else 'anotherType'";
            return $query->whereRaw("posts.type = ($caseWhen)");
        });
}

It may also can be used as an alternative to Comboships https://github.com/topclaudy/compoships

ludo237 commented 5 years ago

Like a scope? πŸ˜•

tteze commented 5 years ago

Like a scope? πŸ˜•

Yes but for relations ! Here you can use the parent model to filter your relation.

Namoshek commented 3 years ago

I support this idea, but maybe slightly differently than the author intended. I find myself regularly in situations where I have a model for static (immutable) data which has many entries of another model holding dynamic data with a validity time period ($dynamic->valid_from and $dynamic->valid_to). If now, for good reasons, an entry of the static model is referenced by a third model, but the dynamic data valid at a certain point in time is required, fetching this data is quite cumbersome.

Here an example to illustrate the problem:

class StaticModel extends Model
{
    // properties: id, name

    public function dynamicModels(): HasMany
    {
        return $this->hasMany(DynamicModel::class);
    }

    public function thirdModels(): HasMany
    {
        return $this->hasMany(ThirdModel::class);
    }
}

class DynamicModel extends Model
{
    // properties: id, static_model_id, valid_from, valid_to, address

    public function staticModel(): BelongsTo
    {
        return $this->belongsTo(StaticModel::class);
    }
}

class ThirdModel extends Model
{
    // properties: id, static_model_id, some_date

    public function staticModel(): BelongsTo
    {
        return $this->belongsTo(StaticModel::class);
    }

    // This is what I would like to see working!
    public function dynamicModel(): HasOne
    {
        return $this->hasOne(DynamicModel::class, 'static_model_id', 'static_model_id')
            ->whereColumn('valid_from', '<=', 'some_date')
            ->whereColumn('valid_to', '>=', 'some_date');
    }
}

Currently, getting the dynamicModel from ThirdModel requires either eager loading all dynamicModels of the references staticModel and filtering in the getter, or loading the dynamic data for each model individually, which is quite bad performance-wise.

When using

ThirdModel::query()->with('dynamicModel')->take(3)->get();

assuming above code, the following error is thrown at the moment (I'm using SQL Server, but this should have nothing to do with the DBMS):

Illuminate/Database/QueryException with message 'SQLSTATE[42000]: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The multi-part identifier "third_models.some_date" could not be bound. (SQL: select * from [dynamic_models] where [dynamic_models].[valid_from] <= [third_models].[some_date] and [dynamic_models].[valid_to] >= [third_models].[some_date]) and [dynamic_models].[static_model_id] in (105, 175, 177))'

Obviously, performing described query would require the eager loading to use the third_models.id column as reference and not only the static_model_id. And until now, this can only be done manually in a service/repository, which is quite cumbersome.


Edit: After some more research, the following is a working relationship doing what I described. But it feels hacky. πŸ˜„

class ThirdModel extends Model
{
    // properties: id, static_model_id, some_date

    public function staticModel(): BelongsTo
    {
        return $this->belongsTo(StaticModel::class);
    }

    public function dynamicModel(): HasOneThrough
    {
        return $this->hasOneThrough(DynamicModel::class, ThirdModel::class, 'id', 'static_model_id', 'id', 'static_model_id')
            ->whereColumn('valid_from', '<=', 'some_date')
            ->whereColumn('valid_to', '>=', 'some_date');
    }
}

The trick is to self-reference the origin model of the relationship. It might also be necessary to prefix the columns with the respective table names. Anyway, the following query is produced by this relationship:

select * from [dynamic_models] inner join [third_models] on [third_models].[static_model_id] = [dynamic_models].[static_model_id] where [third_models].[id] = ? and [valid_from] <= [some_date] and [valid_to] >= [some_date])