Repository navigation
Only request the total count when it is needed - #102
Merged
Merged
Conversation
LazyGraphQuery added meta.totalCount to the selection of every graph list query so that len() could answer without fetching. On the server that field is a count over the whole matching set, so every caller paid for a scan it had usually not asked for, and a caller that deliberately left meta out had no way to avoid it. Two changes remove it. A query whose limit already fits in one page has nothing to paginate, so it now skips the lazy wrapper and runs as a single plain query with the fields the caller chose. An unbounded query still goes through LazyGraphQuery, but the count is fetched on demand by len(), indexing and totalCount rather than up front, so a caller that only iterates never pays for one. Paging itself never needed the count, GraphQueryIterator stops when a page comes back short. Fetching the whole result to measure it instead was considered and rejected. That would defeat the point of the class on exactly the large tables it exists to protect. Callers that select meta keep the previous behaviour. The v1 LazyQuery behind GetScenes is untouched, it takes its total from the REST meta.
LLipter
marked this pull request as draft
September 3, 2026 06:58
woswos
reviewed
Sep 3, 2026
Review feedback: skipping the wrapper for a single page changed the payload from LazyGraphQuery to a plain list, so callers lost FetchAll(), totalCount and the other helpers. That made the API harder to use for a saving that can be had without it. UseLazyGraphQuery is back to what it was. Instead, the call the constructor already makes to learn the key name and type name now fetches the whole window when the requested limit fits in one page, and keeps it. The result is complete at that point, so len(), indexing and repr() answer from the items and still need no count, while the return type is unchanged. repr() on such a query is now the real list rather than the elided partial form, since the items are known to be the whole result.
Removing the up-front count from every lazy graph query was too broad. An unbounded query returns a LazyGraphQuery whose len() and truthiness need a total, and deferring that to an on-demand query changed the behaviour of callers that iterate such a result. The sensor link migration reads ListBodies that way, and with the total missing it stopped finding the attached sensors, which left sensorbridges.conf with an empty sensor list and broke the simulator test suite downstream. Narrow it to the case that is provably free. A limit that already fits in one page is satisfied by the call the constructor already makes, so fetch the whole window there and keep it. That query is complete, needs no total, and asks for no count. Every other query asks for the count up front exactly as before, so nothing that pages changes at all. This also removes the on-demand count and the len(), indexing and repr() overrides that existed only to cope with a missing total.
Limiting the saving to single page queries left the common case paying for a count nobody asked for. Fetch it on demand instead, and remove the reasons a caller ends up needing it. Truthiness now answers from the page fetched in the constructor rather than falling through to len(), so the "result or []" shape costs nothing. There are fifteen of those in migration and sensorbridges alone. An index that falls inside that page is returned from it for the same reason, which covers the "result[0]" shape. Paging already needed no total, GraphQueryIterator stops when a page comes back short. When a total really is required, the query keeps the caller's field selection and adds meta to it. That matters: a resolver such as ListBodies gates loading the rows on the data field being selected, so a query asking for meta alone counts an empty slice and reports zero. Asking for meta on its own is what previously made an unbounded result look empty and broke the sensor link migration.
Contributor
Author
|
Pipeline #2092933 |
LLipter
marked this pull request as ready for review
September 4, 2026 04:53
woswos
approved these changes
Sep 7, 2026
Member
|
Pipeline #2096092 |
Member
|
Thank you! |
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
2 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.
LazyGraphQueryaddedmeta.totalCountto every graph list query solen()could answer without fetching, so every caller paid for a count over the whole matching set even after explicitly leavingmetaout of itsfields.The count is now fetched on demand, and the things that used to force it no longer do:
len(), soresult['key'] or []costs nothing (15 call sites inmigrationandsensorbridgesuse that shape)result['key'][0]GraphQueryIteratorstops on a short pageWhen a total is genuinely required, the query keeps the caller's field selection and adds
metato it. This matters: a resolver such asListBodiesgates loading the rows on the data field being selected, so a query asking formetaalone counts an empty slice and reports zero.Verified against a real webstack, counting
SELECT count(...)in the server log:first=1, no metaor []truthiness[0]len()Only a real length or an explicit
metaselection now costs a count.The return type is unchanged, callers still get a
LazyGraphQuerywithFetchAll(),totalCountand the other list helpers. Behaviour on theListBodiesshapes used by the sensor link migration (or [],[0],len()) is identical to master. The v1LazyQuerybehindGetScenesis untouched.