Wrong Eloquent model ID after query with JOIN

[Grey2k]

In last version of laravel

im using two models:

1. contact
2. document, which belongsTo contact by contact_id field

For order by relation field (for example last_name)

im using (may be isnt correct , but not anything about it in documentation)

$documents = Document::with('contact')->join('contacts', 'contacts.id', '=', 'documents.id')->orderBy('contacts.name')->get()

after that query in loop

foreach($documents AS $document){
    if($document->id == $document->contact->id){
        // always matched here!  but in database its not like that
    }
}

if i remove join() - all correct,
please this is a bug or feature and advice about orderBy relation fields.

Thanks

[taylorotwell]

It's because of your join the contacts.id is overwriting the documents.id because they have the same column name. Use a select clause.

[Grey2k]

solution is ->get('documents.*') in fetch, thanks!

[enricobono]

Or use ->select('documents.*')

[brenelz]

Should this not at least throw an ambiguous column error? Its kind of weird it just uses the wrong id and keeps going.

[Samyoul]

I have to say, I agree with @brenelz silently overwriting a column value is unexpected behaviour. An ambiguous column name exception should be thrown in this case. Or the joined to table has priority and doesn't loose its values to overwriting.

Doing neither seems counter intuitive.

[tanteng]

今天也遇到了，通过 selec t指定字段解决了

[nerdo]

I agree with Samyoul and the others who upvoted his post.

I ran into this issue when using a belongsToMany relation. The pivot table had it's own id that was overwriting the model's silently. Doing ->select('models.*') works, but, at least in the case of the relation, it seems like it should do this automatically. I'm using L5.3 btw. Not sure if it's changed in 5.4

[architech99]

I'm adding my vote that this needs to be addressed in the framework. I'm running into an issue where both lazy loading and eager loading relationships are resulting in an incorrect id property on the related model. For example, when I have a Post model and a Comment model with the Comment model belonging to the Post model:

class Post extends \Eloquent
{
    public function comments()
    {
        return $this->hasMany(Comment::class);
    }
}

class Comment extends \Eloquent
{
    public function post()
    {
        return $this->belongsTo(Post::class);
    }
}

When I retrieve the Post model and attempt to eager load (and limit the total number or relations loaded), like so:

$post->load([ 'comments' => function($query)
{
    return $query->join('comments', 'comments.post_id', '=', 'comments.id')->orderBy('comments.created_at', 'DESC')->take(15)
        ;
}]);

If I grab the first element of the comments property in my view, not only do I have the incorrect comment ID, it's for IDs that aren't even related to the Post model I'm viewing. It's just grabbing the first 15 comments from the database and using their IDs to populate the relations. The other related data is correct as far as I can tell, but the ID is wrong entirely.

This needs to be addressed.

[architech99]

@nerdo To answer your questions about L5.4, it has not been fixed. I'm upgrading a client to L5.4 currently and just ran into this. I ran into it L4.2 and it's been an on-going issue for quite some time.

[vedmant]

There is a simlpe fix for this:

User::select('joined_table.*', 'users.*')

This way, users fields will overwrite joined table fields

[architech99]

@vedmant I did find that option eventually, however I still believe this is a bug and should be fixed. When you don't make the call to select, it's overwriting a model's value in the code such that it does not match the real value that is stored in the database.

As a developer, I expect it to leave my primary model's values as representative of what's in the database when I don't include a call to the select method. Why is it defaulting to the joined table's ID column value when you're selecting from a completely different model (i.e. defaulting to roles.id when you're querying the User model)? It's an unexpected behavior that makes no sense and should not be occurring.

[vedmant]

@architech99 I think it's done this way to make Eloquent more flexible, Eloquent doesn't store full list of columns (unlike Doctrine or Kohana ORM) so it can't map DB columns to appropriate Model attributes.

I'm not sure if Mysql strict mode can throw errors in the case of join columns overlap, but there is certainly no way Eloquent can detect this overlaps without having exact data about all table columns.

[architech99]

@vedmant You should be able to check and see if there's an "id" column in the select list already when building the query. Even without MySQL strict mode I get an error about ambiguous columns if I try to select a non-namespaced field. That tells me that Eloquent is doing something as it builds the query's field list such that it picks the id column from whichever table is the latest to be added. If it's doing that, it can check to see if there's an "id" column already in the field list and prevent this entire problem.

[vedmant]

@architech99 It actually doesn't add id column to select query unless you specifically add it with select(). By default it selects all columns:

select * from `users` where `id` = '123'

Also for joins it basically doing nothing, this is how my query with join looks:

select * from `hotels` inner join `hotel_translations` as `t` on `hotels`.`id` = `t`.`hotel_id` and `t`.`locale` = 'en' order by `id` asc limit 10 offset 0

This approach allows maximum flexibility, and there is no way Eloquent can know that it got id field from hotel_translations table instead of hotels.