-
Notifications
You must be signed in to change notification settings - Fork 102
Handle pushed down predicates in Electric collection #618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: c9a8721 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
aa2e623
to
2ae236f
Compare
d66c54e
to
71cef08
Compare
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Size Change: 0 B Total Size: 76.9 kB ℹ️ View Unchanged
|
Size Change: 0 B Total Size: 1.46 kB ℹ️ View Unchanged
|
91ea051
to
3493f6d
Compare
49f1f59
to
dd337d8
Compare
clause: IR.OrderByClause, | ||
params: Array<unknown> | ||
): string { | ||
// TODO: what to do with stringSort and locale? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This raises the question on if we have the wrong default in DB, should we default to lexical sort?
// After receiving Bob and Charlie, the collection now has 3 users (Alice + Bob + Charlie) | ||
// but it still requests 2 more... TODO: check if this is correct? | ||
expect(callArgs(1)).toMatchObject({ | ||
limit: 2, | ||
orderBy: `age NULLS FIRST`, | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevin-dp not sure why it does this
…ests These tests have a known issue with unhandled rejection warnings that exists in the main branch. The test functionality works correctly, but vitest reports unhandled rejections when testing timeout behavior with fake timers. The tests properly validate that timeouts occur and are handled, but the setTimeout callback in awaitMatch creates promise rejections that vitest flags as unhandled, even though they are properly caught by the test assertions.
This PR is a follow up on #617 and modifies the Electric collection to handle predicates that are being pushed down to the Electric collection.