Db ivm groupBy bug #1035
Draft
Db ivm groupBy bug #1035
+969
−0
Conversation
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
Add tests to verify that the groupBy operator correctly emits paired delete+insert messages when aggregates change. These tests investigate a reported bug where the D2 pipeline might emit an insert without a corresponding delete in certain edge cases. The tests verify: - Incremental updates emit paired delete+insert for aggregate changes - Rapid incremental updates maintain correct message pairing - Multiple groups with interleaved updates emit correct pairs - Message accumulation correctly pairs deletes and inserts All tests pass at the db-ivm level, suggesting the issue may be at the db package layer or involves specific timing conditions.
Add tests at the db package level to investigate the reported bug where groupBy might emit inserts without corresponding deletes. These tests complement the db-ivm level tests and verify: - Basic incremental updates with same groupBy key - Multiple incremental updates to same group - Incremental updates with sum aggregate - Multiple groups with incremental updates - Batch then incremental updates All tests pass, indicating the standard scenarios work correctly. The bug may only manifest in specific async or timing conditions.
|
Cursor Agent can help with this pull request. Just |
|
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-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: |
Contributor
|
Size Change: 0 B Total Size: 88.5 kB ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 3.35 kB ℹ️ View Unchanged
|
Extended the test file to cover all scenarios from PR 1030: - D2 output tracing to verify paired delete+insert emissions - Direct bug reproduction tests for sync layer behavior - Subquery pattern (groupBy as source for orderBy/limit) - Rapid sequential inserts - Multiple events in single batch Investigation findings: - All tests pass without the PR's defensive fix - D2 correctly emits paired delete+insert for aggregate updates - The accumulator correctly consolidates them into update operations - The bug may only manifest in specific async/timing edge cases not covered by synchronous tests These tests serve as regression tests for groupBy incremental updates.
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.
🎯 Changes
Added comprehensive tests to
packages/db-ivm/tests/operators/groupBy.test.tsandpackages/db/tests/query/group-by-incremental-test.test.tsto verify the correct behavior ofgroupByincremental updates.This investigation aimed to reproduce a reported bug where
groupByemitted duplicate inserts without corresponding deletes, but the bug could not be reproduced in synchronous test scenarios at either thedb-ivmordblevel. The new tests confirm that thegroupByoperator correctly emits paired delete and insert messages when aggregate values change.✅ Checklist
pnpm test:pr.🚀 Release Impact