Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.0 - Empty property when matching() and contain() on the same alias #5463

Closed
Laykou opened this issue Dec 22, 2014 · 13 comments
Closed

3.0 - Empty property when matching() and contain() on the same alias #5463

Laykou opened this issue Dec 22, 2014 · 13 comments
Labels
Milestone

Comments

@Laykou
Copy link

Laykou commented Dec 22, 2014

Contain doesn't work properly when using 'strategy' => 'subquery' defined in Table. It returns selected rows and also _matchingData properly but the contained property is empty.

This is the example code (I tried it on different one but it should do the same):

$query = new Query($this->connection, $this->table);
$table = TableRegistry::get('authors');
$table->hasMany('articles', ['strategy' => 'subquery']);
TableRegistry::get('articles')->belongsToMany('tags');

$result = $query->repository($table)
    ->select()
    ->matching('articles.tags', function ($q) {
        return $q->where(['tags.id' => 2]);
    })
    ->contain('articles');

$result->first()->articles; // This is empty

And it also doesn't work on belongsTo associations - the same issue, _matchingData is there but associated property is empty:

$query = new Query($this->connection, $this->table);
$table = TableRegistry::get('Articles');
$table->belongsTo('Categories');

$result = $query->repository($table)
    ->select()
    ->matching('Categories', function ($q) {
        return $q->where(['Categories.id' => 2]);
    })
    ->contain('Categories');

$result->first()->category; // This is empty

unset($row[$key]);

I think this unset is doing a problem for BelongsTo associations because the associated belongsTo object is then not attached to the master entity. When I removed it, it worked.

@lorenzo
Copy link
Member

lorenzo commented Dec 22, 2014

I hoped that people would understand that adding a matching on a joinable association would obviously put the data in matching. Apparently this is not the case...

@lorenzo lorenzo added this to the 3.0.0 milestone Dec 22, 2014
@lorenzo lorenzo added the defect label Dec 22, 2014
@Laykou
Copy link
Author

Laykou commented Dec 22, 2014

The case is that using matching works correctly according to the new specification but the contain doesn't. Or at least for me. I don't know if I did anything wrong. I just pulled newest version of Cake and left my code as it was (because it already had the contain as well as matching on the same alias as you can see in the examples). So I thought the contain should work as expected when using matching.

I tried it on hasMany association without subquery and it worked - matching and contain properties were there. So I think there is just a small bug.

@Laykou
Copy link
Author

Laykou commented Dec 22, 2014

But I understand that using only matching would put data only into _matchingData and not into the property. This is OK for me. Therefore I use extra contain() if I want to ensure that data are also in the associated property.

@lorenzo
Copy link
Member

lorenzo commented Dec 22, 2014

That's fine. I understand that it is not obvious so I will take care of it. As for the 'subquery' strategy I truly surprised that it makes a difference at all, I'm going to investigate that as well.

@Laykou
Copy link
Author

Laykou commented Dec 22, 2014

What is not obvious? I am not sure if we understand each other. I say the problem is when using this on belongsTo association:

$article = $articles->matching('Categories', function(){ ... })->contain('Categories')->first();
$article->category; // This is EMPTY although it is contained
$article->_matchingData['Categories']; // This is OK

@lorenzo
Copy link
Member

lorenzo commented Dec 22, 2014

Something that was obvious to me, but I can totally see now that it is not obvious for everyone not involved with the internals of how it works.

@Laykou
Copy link
Author

Laykou commented Dec 22, 2014

I also experienced this problem:

// Users belong to Cities, Cities belong to Countries
$user = $users->matching('Cities', function($query) { 
        return $query->where(['Cities.country_id' => 1]);
    })
    ->contain(['Cities' => ['Countries']])
    ->first();

$user->city // This is OK after my little fix described below
$user->city->country; // This is EMPTY

Little fix: I removed the unset($row) in ResultSet.php

To make it clear the SQL should look something like (without all the ORM stuff):

SELECT * FROM users
INNER JOIN cities ON cities.id = users.city_id AND cities.country_id = 1
INNER JOIN countries ON countries.id = cities.country_id

So it's basically filtering all users that belong to country with id = 1

@lorenzo
Copy link
Member

lorenzo commented Dec 23, 2014

Thanks for the issue report, test case and candidate fix, it was very helpful

@Laykou
Copy link
Author

Laykou commented Dec 23, 2014

👍 You are welcome. Thank's for the fix. Looking forward for merge :)

@nilsonpena
Copy link

@Laykouis this working for you? I'm using 3.4.7 and experiencing the same problem! the contain property is empty (actually NULL)

from the docs: "_The data from the association that is ‘matched’ will be available on the _matchingData property of entities. If you both match and contain the same association, you can expect to get both the matchingData and standard association properties in your results."

@hagen00
Copy link

hagen00 commented Dec 14, 2017

@nilsonpena this is old, but does using both matching and contain in the same query work? Not working for me.

@lorenzo
Copy link
Member

lorenzo commented Dec 14, 2017

@hagen00 It should work, there are tests showing that this is possible

@hagen00
Copy link

hagen00 commented Dec 15, 2017

Ok, I got a brand new Cake install, created some simple tables and tested the interaction between contain and matching. Seems to work as expected!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants