jpfuentes2 / php-activerecord

ActiveRecord implementation for PHP
http://www.phpactiverecord.org/
Other
1.32k stars 443 forks source link

Eager loading does not utilize Model caching #467

Open EVILoptimist opened 9 years ago

EVILoptimist commented 9 years ago

In an effort to cut down build times on my scripts, I've been going through a LOT of SQL logs, and I found an incompatibility with Model caching and eager loading.

When I have caching enabled on my Models and run this:

$company = Company::find($company->id, ['include' => ['bes', 'des' => ['pos' => ['be']]]]);

I get this in my SQL logs:

SELECT * FROM `bes` WHERE `co_id`='14'
SELECT * FROM `des` WHERE `co_id`='14'

SELECT * FROM `pos` WHERE `de_id`='2'
SELECT * FROM `bes` WHERE `id`='2' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='2' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='5' LIMIT 0,1

SELECT * FROM `pos` WHERE `de_id`='3'
SELECT * FROM `bes` WHERE `id`='5' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='6' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='31' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='35' LIMIT 0,1

SELECT * FROM `pos` WHERE `de_id`='6'
SELECT * FROM `bes` WHERE `id`='5' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='5' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='5' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='6' LIMIT 0,1
SELECT * FROM `bes` WHERE `id`='31' LIMIT 0,1

When, really, these should be pulled from cache, right? The Company record is being pulled from cache, then the rest are queried N+1 style.

Then, I disable caching in the Model classes, with the same PHP code, I get this in my SQL logs:

SELECT * FROM `cos` WHERE `id`='14'
SELECT * FROM `bes` WHERE `co_id` IN('14')
SELECT * FROM `des` WHERE `co_id` IN('14')
SELECT * FROM `pos` WHERE `de_id` IN('2','3','6')
SELECT * FROM `bes` WHERE `id` IN('2','2','5','5','31','35','5','6','31','5','5','6')

Anyone more familiar with the code than I have any ideas?

EVILoptimist commented 9 years ago

@shmax Do you hate me yet :scream:

jpfuentes2 commented 9 years ago

Nice find, that's definitely a bug in my book.

jpfuentes2 commented 9 years ago

I think the best way to start with this is a failing test case.

shmax commented 9 years ago

Yes, if you open a PR with a test case I'll take a look. I've never worked with eager loading (and only rarely with associations), so in this case you're more familiar with the code than I am.