Fix order by with limit and where clause#461
Merged
Conversation
🦋 Changeset detectedLatest commit: a4b8401 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
709cde7 to
a5c25f6
Compare
More templates
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Contributor
|
Size Change: +128 B (+0.2%) Total Size: 64.6 kB
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 1.16 kB ℹ️ View Unchanged
|
…lled and a live update inserts a row that is bigger than max sent value
|
I'm having this exact issue with v0.1.9 with this code @kevin-dp const { data } = useLiveQuery(
(q) =>
q
.from({ todoCollection })
.where(({ todoCollection }) =>
like(todoCollection.user_id, user.id)
)
.orderBy(({ todoCollection}) => todoCollection.updated_at, "desc")
.limit(5),
[user.id]
);
console.log(data.length) // 1, but the query without limit returns 60 rows. |
Contributor
Author
|
@wrong7 Could you try out the latest version to see if the problem persists. If it does, could you please open an issue such that we can track this bug and keep you posted when we fix it. |
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
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.
This PR fixes a problem that resulted in queries that have an
orderBy,limit, andwhereclause to initially load too few data if thewhereclauses filters out some of the firstlimit + offsetrows. Claude had a shot at fixing it in #459 but didn't do a good job. I stole the unit tests from that PR (thanks @samwillis for writing those) as they reproduced the issue and are now green in this PR.When the query contains an
orderByandlimitclause we optimize it by only loading theoffset + limitfirst items from the range index initially (introduced in #410). Then at runtime, when new updates come in that may affect the results we send them to the query pipeline and check if the result has enough rows. If not, we load more rows from the range index.The problem is that we should also check the result set after the initial load since
whereclauses may filter out some of the rows, leading to too few rows in the result. Hence, this PR introduces the necessary changes to check the size of the result after the initial load and load more rows from the range index if needed.When introducing this fix, the problem with infinite loading loops when the collection has not enough data to fill the result reappeared. This was fixed in #450 but that solution is incompatible with the changes in this PR. Instead, i found a simpler solution which consists of not passing the callback that loads more data if we know that we already exhausted the collection.