laravel/framework

[Bug] Querying relations with extra conditions not working as expected

NoelDeMartin opened this issue · 7 comments

NoelDeMartin commented

I have been some time playing with the querying system for eloquent to get something done, and recently I found something that I think is a bug.

I have a table "Teams", with id and name columns. And I have a table "Matches" with local_id and visitant_id. What I want to achieve is to get all the matches a team has played, be it as local or visitant. And finally I come up with this.

On the Teams model:

class Team extends Eloquent {
    public function matches_as_visitant() {
        return $this->hasMany('Match', 'visitant_id');
    }
}

On the Teams controller:

$team = Team::with(['matches_as_visitant' => function($query) {
            $query->orWhere('local_id', '=', '2');
            $query->orderBy('local_id', 'asc');
        }])->find(2);

The query that is run with this is:

select * from `matches` where `matches`.`visitant_id` in (?) or `local_id` = ? order by `local_id` asc

After this I expect $team->matches_as_visitant to be actually all the matches, because the eloquent model already matches the visitant_id field and I add a orWhere part where it checks for the local_id. This should work but doesn't, I've been inspecting the framework code and I found at which point this actually "stops working".

In the HasOneOrMany class we have a method matchOneOrMany:

protected function matchOneOrMany(array $models, Collection $results, $relation, $type)
{
    $dictionary = $this->buildDictionary($results);

    // Once we have the dictionary we can simply spin through the parent models to
    // link them up with their children using the keyed dictionary to make the
    // matching very convenient and easy work. Then we'll just return them.
    foreach ($models as $model)
    {
        $key = $model->getKey();

        if (isset($dictionary[$key]))
        {
            $value = $this->getRelationValue($dictionary, $key, $type);

            $model->setRelation($relation, $value);
        }
    }

    return $models;
}

Basically the $results variable is a Collection with all the matches, but then the $models variable (that is then returned) only contains the matches played as visitant, and the local matches are lost.

taylorotwell commented

Yeah, the matcher is matching on the foreign key to associate the child models with their parents, so it's losing the ones where you're matching on local_id, since you're essentially matching on two foreign keys at the same time.

I think you could accomplish this by defining a separate relationship called "allMatches" and then loading that, though you still wouldn't be able to eager load it. Eager loading is not really helping you in terms of performance in this particular use case anyways as you're only loading matches for one team, but you could do something like this:

public function allMatches()
{
     return $this->hasMany('Match', 'visitant_id')->orWhere('local_id', $this->id);
}

Then...

$team = Team::find(2);
$matches = $team->all_matches;
  • 👍9
  • 👎2
NoelDeMartin commented

Ok I see, I really ended up eager loading them because I didn't know I could add more conditions to the eloquent method and this is the only I could think of to achieve what I wanted. I think it's a bit confusing that some query results are lost, because many times I guide myself dumping the raw query that is called to see what's happening.

Thank you for you help, this works great!

Deleted user commented

For people interested, I managed to get this to work in reverse. The original wasn't working for me.

class NewTest extends Eloquent {
    public function allDropdowns()
    {
        return $this->belongsTo('Refmas',$this->col1);
    }

$t = NewTest::find(1);
        $a = $t->allDropdowns->where('id',$t->col1)
            ->orWhere('id',$t->col2)
            ->orWhere('id',$t->col3)
            ->orWhere('id',$t->col4)
            ->get();

       foreach($a as $b){
           echo $b->col4 ."<br>"; };
SunilEver commented

@taylorotwell
Let's assume Match has an extra column sponsor. If I use following query builder.
$team = Team::find(1);
$team->allMatches()->where('sponsor' ,'company')->get();
It is translated as
select * from matches where visitant_id = matches.id and matches.id is not null or local_id = matches.id and matches.id is not null and sponsor = 'company';

For the case where visitant_id matches matches.id and sponsor is not company I still get that record.
Please give a way out.

tisuchi commented

@taylorotwell

That's really nice. But what if I need to load allMatches as a eager loading? Does it work?
For instance-

$team = Team::where('id', 2)->with('allMatches')->first();
  • 👍3
your-baby-dev commented

have you got answer to use it with eager loading ?