-
Notifications
You must be signed in to change notification settings - Fork 280
Remove invalid subset loading check #3436
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?
Remove invalid subset loading check #3436
Conversation
Removed the check that prevented requestSnapshot from being called in full mode. It's valid to load subsets while loading the full log, which is necessary for LiveQuery joins with collections using progressive syncMode. Fixes the error: "Snapshot requests are not supported in full mode, as the consumer is guaranteed to observe all data"
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3436 +/- ##
==========================================
+ Coverage 69.45% 69.54% +0.08%
==========================================
Files 182 182
Lines 9787 9786 -1
Branches 356 358 +2
==========================================
+ Hits 6798 6806 +8
+ Misses 2987 2978 -9
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
commit: |
|
Should we have an e2e test for this? |
|
Out of curiosity, what's the use of a snapshot in a full mode, given the client already holds all the same data? |
|
@icehaunter for the "progressive mode" https://tanstack.com/blog/tanstack-db-0.5-query-driven-sync#progressive-sync-fast-initial-paint--instant-client-queries @kevin-dp you mean in the tanstack db repo? |
Added a test to verify that requestSnapshot works correctly when the shape is in full mode (progressive sync). This test ensures the fix allows subset loading during progressive sync, which is needed for LiveQuery joins.
|
@KyleAMathews This will not fix the issue with Tanstack DB progressive mode, we need to fix it on the DB side rather than on the Electric Client side. The electric client ensures that the results of a The intention of |
No, I got the general idea, I'm just not sure why additional snapshots are needed on top of already full data. I thought the idea for progressive mode was to only use these subset snapshots, otherwise it seems to make little sense if the client already has all the data. Am I missing something? |
DB |
Removed the check that prevented requestSnapshot from being called in full mode. It's valid to load subsets while loading the full log, which is necessary for LiveQuery joins with collections using progressive syncMode.
Fixes the error: "Snapshot requests are not supported in full mode, as the consumer is guaranteed to observe all data"