Optimize polls loading #3692
Optimize polls loading #3692
Conversation
Signed-off-by: dartcafe <[email protected]>
There was a problem hiding this comment.
Nice, should help with performance 👍
Ideally should be tested with OCI, not sure if you’re willing to add https://github.com/nextcloud/.github/blob/master/workflow-templates/phpunit-oci.yml to the workflows? Is can be slow, but oracle database differs on some things so for stuff like a manually written function like that it helps to test on it.
| $qb->selectAlias($qb->createFunction('(' . $this->subQueryOrphanedVotesCount(self::TABLE, $paramUser)->getSQL() . ')'), 'current_user_count_orphaned_votes'); | ||
|
|
||
| $this->joinOptionsForMaxDate($qb, self::TABLE); | ||
| $this->joinUserRole($qb, self::TABLE, $currentUserId); | ||
| $this->joinGroupShares($qb, self::TABLE); | ||
| \OC::$server->getLogger()->error(json_encode($qb->getSQL())); |
There was a problem hiding this comment.
| \OC::$server->getLogger()->error(json_encode($qb->getSQL())); | |
| \OCP\Server::get(\Psr\Log\LoggerInterface::class)->error(json_encode($qb->getSQL())); |
\OC::$server and getLogger are deprecated.
That said I know realise this one is left over debug and will be removed before merge.
There was a problem hiding this comment.
Hehe. Yeah, I know. Thats just an old macro I use for quick logging of implementation steps. Should not have made it into the repo.
| /** | ||
| * Subquery for votes count | ||
| */ | ||
| protected function joinVotesCount(IQueryBuilder &$qb, string $fromAlias, IParameter $currentUserId, ?IParameter $answerFilter = null): IQueryBuilder { |
There was a problem hiding this comment.
| protected function joinVotesCount(IQueryBuilder &$qb, string $fromAlias, IParameter $currentUserId, ?IParameter $answerFilter = null): IQueryBuilder { | |
| protected function joinVotesCount(IQueryBuilder $qb, string $fromAlias, IParameter $currentUserId, ?IParameter $answerFilter = null): IQueryBuilder { |
You only need a reference if you intend to replace the object with another one ($qb = xxx).
There was a problem hiding this comment.
Yes, right. But I would say for consistency the return should be removed, since all joins here base on references.
There was a problem hiding this comment.
But even the joins do not require the parameter to be a reference.
I hope so. But I did not have the chance to test, if the outcome is the same as with the subquery. But it is just at half way, since I plan to replace the other subquery, too. But it's a pain, since the complexity gets risen by every join; and more error-prone
Ah nice. I never thought about testing against OCI, although I have in mind, that this is a real use case.
Yes definitely worth it. I would say, we should add it to the databases. |
|
If only someone could take care of the the test suite. 😁 |
MigrateSubQueries to joins to avoid heavy subqueries
Hopefully fixes #3688
Migrate