-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Improve group queries #20776
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
Improve group queries #20776
Conversation
Before we'd also get the diplayname for each group in the backend. In a separate query. This is of course not ideal as this information is obtained on each and every query. Now this is queried once and properly cached. Also added more caching to the manager. Signed-off-by: Roeland Jago Douma <[email protected]>
cf5eb75 to
5ebb535
Compare
|
I guess this one here solves already part of the issue: #20815 |
MorrisJobke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works 👍
| $this->groupCache[$row['gid']] = $row['gid']; | ||
| $this->groupCache[$row['gid']] = [ | ||
| 'gid' => $row['gid'], | ||
| 'displayname' => $row['displayname'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it this always displayname? I had this issue in Mail recently and I went for selectAlias to ensure the database does not return the selected column, like g.displayname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but there is only 1 displayname... so it should not prefix it right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess so.
|
One more review please :) |
|
php-cs check failure seems unrelated to that change 😉 |
|
I guess a backport makes sense here. At least to stable19 for 19.0.1 |
|
/backport to stable19 |
Before we'd also get the diplayname for each group in the backend. In a
separate query. This is of course not ideal as this information is
obtained on each and every query. Now this is queried once and properly
cached.
Also added more caching to the manager.
Signed-off-by: Roeland Jago Douma [email protected]