jeremyharris / cakephp-lazyload

A lazy loader for CakePHP entities.
MIT License
61 stars 10 forks source link

Useless queries when primary key is unset #26

Closed fabiofdsantos closed 3 years ago

fabiofdsantos commented 3 years ago

It generates useless queries to the database.

For example, if Orders.id is unset:

SELECT ...
FROM orders Orders 
LEFT JOIN delivery_orders DeliveryOrders ON DeliveryOrders.id = (Orders.delivery_order_id) 
WHERE (Orders.id IN (NULL) AND Orders.deleted IS NULL)
jeremyharris commented 3 years ago

Would you be able to provide a small test case that replicates this issue? It's likely not an issue with this plugin but with the underlying method in the Cake ORM, but I can see if I can try to mitigate it.

fabiofdsantos commented 3 years ago

@jeremyharris

I think we just need to replace: https://github.com/jeremyharris/cakephp-lazyload/blob/1319c1ff51476176f5b7d0231c0970085be9cbd3/src/ORM/LazyLoadEntityTrait.php#L131

With:

if ($association === null || $this->get($association->getBindingKey()) === null) {
    return null;
}

If you agree, I will open a PR.

jeremyharris commented 3 years ago

I think $this->get($this->getPrimaryKey()) === null will suffice and is a little more clear. This could be at the very top of _lazyLoad as it can't really lazy load data if the PK is missing.

Feel free to open a PR! I appreciate it. Tests are required but if you aren't able to add one I'll take care of it.

fabiofdsantos commented 3 years ago

I prefer $this->get($association->getBindingKey()) because you can assign an id e.g. $article->authod_id = 123 and then use the lazy load feature.

Feel free to review the PR. Many thanks!