fix(rows): keep row counters fresh and reduce count queries#2781
Open
hweihwang wants to merge 2 commits into
Open
fix(rows): keep row counters fresh and reduce count queries#2781hweihwang wants to merge 2 commits into
hweihwang wants to merge 2 commits into
Conversation
hweihwang
force-pushed
the
fix/451-rows-counter
branch
from
c4c972a to
c652292
Compare
Member
|
Thanks for the fix! Which performance impact can we expect from this change? |
The navigation and dashboard row counters are read from the table/view objects in the tables store, which were only refreshed on a full table list reload. Row create/update/delete updated the loaded rows in the data store but never the stored rowsCount, so counters stayed stale until the page was reloaded. Frontend: sync the active table/view rowsCount from the loaded rows length after each row load/create/update/delete, and adjust the parent table count by +/-1 when rows are created or deleted through a view. Because a row change can also add or remove the row from any filtered view of the same table - which the client cannot determine locally - the affected views' counts are reloaded from the backend, debounced to coalesce bursts of edits. A Playwright test covers the table and a sibling view counter both updating after adding a row, without reload. Backend: countRowsForView() fetched every matching row id into PHP and counted the array. It now counts matching sleeves directly in SQL - a view without filters reuses the cheap table count, and filtered views run a single COUNT(*) reusing the existing filter expressions. Because filters are applied as `sleeves.id IN (subselect)` the outer count is never inflated and no GROUP BY is required. Related to #451, #725, #731 Signed-off-by: Hoang Pham <hoangmaths96@gmail.com>
Listing tables enhanced each table individually, running one COUNT query per table to populate its row counter. For a user with many tables this is a classic N+1. Fetch all table row counts in a single grouped query before the enhance loop and pass the result into enhanceTable(). The per-table read-rows permission gate is preserved (unreadable tables report 0, matching the previous getRowsCount() fallback), and the single-table enhance path is unchanged. Context tables are no longer enhanced twice. This addresses the table side of the listing N+1. View counts still run one query per view, as a cheap stored/cached view counter depends on per-user and value-dependent filters and needs a separate design. Related to #451 Signed-off-by: Hoang Pham <hoangmaths96@gmail.com>
hweihwang
force-pushed
the
fix/451-rows-counter
branch
from
c652292 to
dd943b9
Compare
Contributor
Author
@blizzz Ah thanks. I expect this to be a net positive: table counters: N per-table count queries become one grouped count query for readable tables. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes stale row counters in Tables after row changes.
Closes #451
Related: #725, #731
Performance impact
This should be a net improvement for counter-heavy paths:
COUNT(*)query per table to one groupedCOUNT(*) ... GROUP BY table_idquery for all readable tables, chunked at 1000 IDs.COUNT(*); previously they loaded all matching row IDs into PHP and counted the array.Tests
countRowsForView()matching filteredfindAll()results.