Framework: Using find() in a HasManyThrough relation

[mcampa]

My table structure looks like this:

    id - integer
    name - string

menus
    id - integer
    user_id - integer
    name - string

items
    id - integer
    menu_id - integer
    title - string

My model relationship:

class User extends Eloquent {

    public function items()
    {
        return $this->hasManyThrough('Item', 'Menu');
    }

}

it works perfectly when I do this:

$user->items;

The problem is when I try using _find_ like this:

$user->items()->find(8);

I get this:

SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'id' in where clause is ambiguous (SQL: select * from `items` inner join `menus` on `menus`.`id` = `items`.`menu_id` where `menus`.`user_id` = 1 and `id` = 8 limit 1) 

Of course I can fetch the item like this:

$user->items()->get()->find(8);

But, it's fetching all items and then iterating in the code to get the item I need.

Analyzing the error I see that the table name is missing in the where clause. It should be like this:

...  `items`.`id` = 8 limit 1

I fixed the problem by modifying the method find in _/­vendor/­laravel/­framework/­src/­Illuminate/­Database/­Eloquent/­Builder.php_ from this

    public function find($id, $columns = array('*'))
    {
        if (is_array($id))
        {
            return $this->findMany($id, $columns);
        }

        $this->query->where($this->model->getKeyName(), '=', $id);

        return $this->first($columns);
    }

to this:

    public function find($id, $columns = array('*'))
    {
        if (is_array($id))
        {
            return $this->findMany($id, $columns);
        }

        $this->query->where($this->model->getTable().'.'.$this->model->getKeyName(), '=', $id);

        return $this->first($columns);
    }

As you can see, this method is not considering the table name to build the query.

[mcampa]

The modification I made above was not working properly.

What I ended doing was adding a find method to _Illuminate/Database/Eloquent/Relations/HasManyThrough.php_

    /**
     * Find a model by its primary key.
     *
     * @param  mixed  $id
     * @param  array  $columns
     * @return \Illuminate\Database\Eloquent\Model|static|null
     */
    public function find($id, $columns = array('*'))
    {
        $select = $this->getSelectColumns($columns);

        $model = $this->query->addSelect($select)->getModel();

        $this->query->where($model->getTable().'.'.$model->getKeyName(), '=', $id);

        return $this->first($columns);
    }

Now, when I do this

$user->items()->find(8);

the result query is:

select `items`.*, `menus`.`user_id` 
from `items` inner join `menus` on `menus`.`id` = `items`.`menu_id` 
where `menus`.`user_id` = '1' and `items`.`id` = '8' limit 1

[taylorotwell]

You can probably use:

$user->items()->where('items.id', 1)->first();

[mcampa]

Thank you Taylor,

I didn't think about that. But, wouldn't it be more consistent if we can use _find(1)_ in a hasManyThrough relation, just like in a hasMany relation?

What do you think about adding the method above?

[mcampa]

I just found that using your solution doesn't work.

if I use this $user->items()->where('items.id', 1)->first(); I get an object with attributes from the middle table.

The result query is this:

select * 
from `items` inner join `menus` on `menus`.`id` = `items`.`menu_id` 
where `menus`.`user_id` = ? and `items`.`id` = ? 
limit 1

Which is selecting all columns from all tables, instead of only the columns from the _items_ table

[erindru]

I am also experiencing this problem, after having the expectation of find() working on hasManyThrough() the same as on hasMany() (as @mcampa describes). I also tried @taylorotwell's suggestion, only to experience the same issue as @mcampa with the object being populated with data from the middle table.

Is there an official solution? Could @mcampa's patch be merged?

[erindru]

Ok so I have discovered a workaround, thats messy but might help someone. The problem with the object being populated from the middle table was due to the select not being narrow enough, and including the fields from the middle table that conflict with the fields from the target table.

So, the solution is to modify the select() as well to only include the required fields. eg:

$user->items()->select("items.*")->where("items.id", $id)->first();

It seems very roundabout to being able to do $user->items()->find($id) but it does work.

[mcampa]

@erindru Thank you for the update.

I ended using your solution to avoid patching the Laravel core.

I added an addSelect

    $user->items()
        ->select("items.*")
        ->addSelect("menus.user_id")
        ->where("items.id", $id)
        ->first();

[sebastiaanluca]

I don't know, seems like a messy, unnecessary workaround. If I use $user->items()->find(8);, I don't care what relation the items have to the user or the other way around, I just want to use the find or findOrFail method like any other relation/query.

---

And for the record, I just used a whereIn clause, as so: $user->items()->whereIn('items.id', $id)->first(). Which is essentially an elaborate find(), but doesn't return the extra data from the pass-through table.

[mneuber]

I think this is a pretty serious issue, cause you might not notice it for a while, when ids are valid for other items.

$user->items()->each(function($item)
{
        $item->delete();
}

might result in deleting wrong items.

[neronmoon]

I'm getting

Column 'id' in where clause is ambiguous (SQL: select `projects`.*, `project_user`.`user_id` as `pivot_user_id`, `project_user`.`project_id` as `pivot_project_id` from `projects` inner join `project_user` on `projects`.`id` = `project_user`.`project_id` where `project_user`.`user_id` = 1 and `id` = 1)

on $user->projects()->where('id', $id)->firstOrFail()
Yeah, i can use projects.id, but if eloquent claims to its name this issue should be resolved.
So +1 here

[erindru]

A bit of a shame to see this still isnt fixed 2 years later. It might have something to do with this ticket being closed. You might need to open a new one and reference this one to get it back on the radar, if you cant convince @taylorotwell to reopen it