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 3 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.
| $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?
| foreach (array_chunk($tableIds, 1000 - 1) as $tableIdsChunk) { | ||
| $qb = $this->db->getQueryBuilder(); | ||
| $qb->select('table_id', $qb->func()->count('*', 'counter')) | ||
| ->from($this->table, 't1') |
There was a problem hiding this comment.
not sure we need the t1 alias
| $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?
…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
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)