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

Collection::makeTree() hits database for each child #231

Closed
ritterg opened this issue May 11, 2020 · 4 comments
Closed

Collection::makeTree() hits database for each child #231

ritterg opened this issue May 11, 2020 · 4 comments

Comments

@ritterg
Copy link

ritterg commented May 11, 2020

I use ->getDescendants()->toTree() to get the whole tree and show it as a hierarchical list of checkboxes.
Up to version 5.1.1 this was one database hit (to get descendants).
select * from termsinner jointerm_closureonterm_closure.descendant=terms.idwhereterm_closure.ancestor= 1 andterm_closure.depth > 0

In version 6 it is now one database hit (to get descendants, same as above) and one database hit for each child
select * from termswhereterms.parent_id= 2 andterms.parent_id is not null

This makes building a larger tree very slow.

The problem seems to be on line 174 of Collection.php:
$result[$parentId]->children->add($item);
which used to be (in 5.1.1, Collection.php line 80)
$result[$parentId]->appendRelation($item->getChildrenRelationIndex(), $item);

Is there a way to get the tree without all these database hits?

@ritterg
Copy link
Author

ritterg commented May 11, 2020

I was able to improve the speed of ->toTree() by loading the children relation on the descendants collection before calling toTree().

So instead of
$root->getDescendants()->toTree()
I do
$descendants = $root->getDescendants();
$descendants->load('children');
$tree = $descendants->toTree();

Still not as good as it was in 5.1.1, but much better than without loading children (now 2 database queries instead of 1 + (number of descendants))

@franzose
Copy link
Owner

Hello, @ritterg! It seems it's a regression :/ I'm going to fix it and make a new release.

franzose added a commit that referenced this issue May 11, 2020
fixed #231 regression by utilizing relation methods as it was in ClosureTable 5
@ritterg
Copy link
Author

ritterg commented May 11, 2020

I can confirm that this (Version 6.0.1) fixes my issue.
Thanks for the quick fix.

@franzose
Copy link
Owner

You're welcome!

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

No branches or pull requests

2 participants