perf: query search results directly #11432
Conversation
Query the search results (messages) directly instead of querying their ids first by the search query and then doing another query for the actual messages. Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
| public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, string $sortOrder, ?int $limit, ?array $uids = null): array { | ||
| return array_map( | ||
| static fn (Message $message) => $message->getId(), | ||
| $this->findByQueryWithoutRelatedData( | ||
| $mailbox, | ||
| $query, | ||
| $sortOrder, | ||
| $limit, | ||
| $uids, | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Potential drawback: Overfetching
If only the IDs are required, the whole row is fetched without being required.
This method is currently only used in \OCA\Mail\Service\Sync\SyncService::getDatabaseSyncChanges().
There was a problem hiding this comment.
Depending on the number of emails shown in the client this will be 20 or more rows to overfetch.
What could we do to prevent this? Duplicate findByQueryWithoutRelatedData (ugly) or pass the columns as argument (even uglier)? 🤔
There was a problem hiding this comment.
Those are the two options I think.
There was a problem hiding this comment.
Or, we factor the body of the method (query logic) out and provide two entry points. This makes sense as the columns to be selected is the only thing changing.
EDIT: Ok, this is not much better than just passing the columns.
There was a problem hiding this comment.
I pushed the version with the columns-as-argument.
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
2 participants
For #10668
Ref #10723 (comment)
Get rid of one SQL query for every call to
MessagesController::index()(and other consumers ofIMailSearch::findMessages()).Query the search results (messages) directly instead of querying their ids first by the search filter and then doing another query for the actual messages by their ids.