perf: Enhance objects using constant sql queries count to get rid of N+1 queries - #2857
perf: Enhance objects using constant sql queries count to get rid of N+1 queries#2857Koc wants to merge 4 commits into
Conversation
9b6bd68 to
4033598
Compare
Koc
changed the title
enhanceTables for a multiple tables at once to get rid of N+1 queries
Koc
changed the title
29cbb47 to
44e315b
Compare
| * @return Share[] | ||
| * @throws Exception | ||
| */ | ||
| public function findAllSharesForNodes(string $nodeType, array $nodeIds, string $sender = '', array $excluded = []): array { |
There was a problem hiding this comment.
Is sender applied to the query? otherwise not sure why defined
There was a problem hiding this comment.
This filters by sender, but the singular findAllSharesForNode above ignores its $sender arg entirely. So countSharesForTables/countSharesForViews now count only shares I sent, while the old findAll path counted all shares on the node. This flips hasShares to false for a table/view I own that a different manager shared.
Is this intended? If yes, maybe make findAllSharesForNode honor $sender too so both paths agree.
There was a problem hiding this comment.
findAllSharesForNode() now honors $sender
| $view->setHasShares($sharesCount > 0); | ||
| } else { | ||
| try { | ||
| $allShares = $this->shareService->findAll('view', $view->getId(), $userId); |
There was a problem hiding this comment.
The batch path (countSharesForViews) is sender filtered, but this calls findAll('view', ...) which is not. The same view can get a different hasShares depending on which path runs?
There was a problem hiding this comment.
Now fallback uses countSharesForViews(...) instead of findAll(...)
| $table = $this->tableService->find($tableId); | ||
| return new DataResponse($this->viewService->formatViews($this->viewService->findAll($table, tableRowsCount: $table->getRowsCount()))); |
There was a problem hiding this comment.
thinking about performance, tableService->find() already enhances the table and loads its views, then findAll(...) loads and enhances them again. Is it possible to avoid this duplicate work or no?
There was a problem hiding this comment.
I did changes, so, now we're calling $table->getViews(). Tests are green, but I'm not fully confident that this endpoint is covered by tests. And it seems like we aren't using this endpoint on FE side
…f N+1 queries Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
…1 queries Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
…rid of N+1 queries Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
44e315b to
0aa2fac
Compare
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
2686136 to
e9b83fb
Compare
2 participants
In our organization we operate a large amount of tables/views, so there are performance bottleneck here:
TableService::enhanceTable()in a loop with a single call of theenhanceTables()ViewService::enhanceView()in a loop with a single call of theenhanceViews()UserHelperfor row/column counts, share counts and owner display names.🖼️ Screenshots
Here profiler results for
GET /apps/tables/tableendpoint for 18 tables and 9 views:🏁 Checklist
/backport to stableX.X🤖 AI (if applicable)