Framework: [Bug] Query find() giving "Column 'id' in where clause is ambiguous" if model joined to another table in newQuery()

[MarkBaker]

I have over-ridden the newQuery() method in my models to implement a set of security filters, typically joining to another table that identifies whether a user has permission to access specific records in the MySQL database table.... e.g. identifying if a user can see users for other companies/establishments, or only for their own company/establishment. This works perfectly well in most cases, but when doing a simple find(x) against the query,

$user = $this->getModel()->with('attributes.field.group')
    ->find($modelId, ['Users.id']);

it throws an exception:

SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'id' in where clause is ambiguous 
SQL: select `Users`.`id` from `Users` 
      inner join `UserEstablishments` as `ue0` 
         on `ue0`.`user_id` = `Users`.`id` 
        and `ue0`.`active` = 1 
        and `ue0`.`establishment_id` IN (0,1,2,5,9) 
      where `id` = 245 
      limit 1

As you can see, the find() is implementing a where id = x; and as both the Users table and the UserEstablishments table have a column called id, it is ambiguous in this context.

Note that my example only returns the id column from Users, but the potential ambiguity can be eliminated in the select list simply by ensuring that the columns are passed to the find() method as ['Users.id'] in the column array. It is only outside of developer control in the where clause.

Modifying the Query Builder to apply the table name in the where clause applied for a find() (or findMany()) would eliminate this problem, while causing no additional problems even if there were no additional joins implemented.

The change in Illuminate/Database/Eloquent/Builder.php is as simple as:

/**
 * 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('*'))
{
    if (is_array($id))
    {
        return $this->findMany($id, $columns);
    }

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

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

/**
 * Find a model by its primary key.
 *
 * @param  array  $id
 * @param  array  $columns
 * @return \Illuminate\Database\Eloquent\Model|Collection|static
 */
public function findMany($id, $columns = array('*'))
{
    $this->query->whereIn(
        $this->model->getTable() . '.' . $this->model->getKeyName(), 
        $id
    );

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

I suspect that the find() method in Illuminate/Database/Query/Builder.php would also need modifying to

public function find($id, $columns = array('*'))
{
    return $this->where($this->from . '.id', '=', $id)->first($columns);
}

though I haven't tested this latter change

[taylorotwell]

We actually cannot always prefix the table name. Some databases will throw errors if you prefix the table name and don't have a join. I suggest using where('table.id', $id)->first();