Skip to content

Commit 51c73aa

Browse files
samwilliscursoragent
samwillis
and
cursoragent
authored
Fix deduplication of limited subset requests (#914)
* Fix: Correctly handle limited queries in isPredicateSubset Co-authored-by: sam.willis <[email protected]> * Fix: Adjust live query deduplication logic and test expectations Co-authored-by: sam.willis <[email protected]> * Refactor deduplication tests for limited queries Co-authored-by: sam.willis <[email protected]> * Fix: Deduplicate limited queries based on where clause Co-authored-by: sam.willis <[email protected]> --------- Co-authored-by: Cursor Agent <[email protected]>
1 parent 954c8fe commit 51c73aa

5 files changed

Lines changed: 320 additions & 34 deletions

File tree

‎.changeset/fix-limited-query-deduplication.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@tanstack/db": patch
3+
---
4+
5+
Fixed incorrect deduplication of limited queries with different where clauses. Previously, a query like `{where: searchFilter, limit: 10}` could be incorrectly deduplicated against a prior query `{where: undefined, limit: 10}`, causing search/filter results to only show cached data. Now, limited queries are only deduplicated when their where clauses are structurally equal.

‎packages/db/src/query/predicate-utils.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,13 +802,57 @@ export function isPredicateSubset(
802802
subset: LoadSubsetOptions,
803803
superset: LoadSubsetOptions
804804
): boolean {
805+
// When the superset has a limit, we can only determine subset relationship
806+
// if the where clauses are equal (not just subset relationship).
807+
//
808+
// This is because a limited query only loads a portion of the matching rows.
809+
// A more restrictive where clause might require rows outside that portion.
810+
//
811+
// Example: superset = {where: undefined, limit: 10, orderBy: desc}
812+
// subset = {where: LIKE 'search%', limit: 10, orderBy: desc}
813+
// The top 10 items matching 'search%' might include items outside the overall top 10.
814+
//
815+
// However, if the where clauses are equal, then the subset relationship can
816+
// be determined by orderBy and limit alone:
817+
// Example: superset = {where: status='active', limit: 10, orderBy: desc}
818+
// subset = {where: status='active', limit: 5, orderBy: desc}
819+
// The top 5 active items ARE contained in the top 10 active items.
820+
if (superset.limit !== undefined) {
821+
// For limited supersets, where clauses must be equal
822+
if (!areWhereClausesEqual(subset.where, superset.where)) {
823+
return false
824+
}
825+
return (
826+
isOrderBySubset(subset.orderBy, superset.orderBy) &&
827+
isLimitSubset(subset.limit, superset.limit)
828+
)
829+
}
830+
831+
// For unlimited supersets, use the normal subset logic
805832
return (
806833
isWhereSubset(subset.where, superset.where) &&
807834
isOrderBySubset(subset.orderBy, superset.orderBy) &&
808835
isLimitSubset(subset.limit, superset.limit)
809836
)
810837
}
811838

839+
/**
840+
* Check if two where clauses are structurally equal.
841+
* Used for limited query subset checks where subset relationship isn't sufficient.
842+
*/
843+
function areWhereClausesEqual(
844+
a: BasicExpression<boolean> | undefined,
845+
b: BasicExpression<boolean> | undefined
846+
): boolean {
847+
if (a === undefined && b === undefined) {
848+
return true
849+
}
850+
if (a === undefined || b === undefined) {
851+
return false
852+
}
853+
return areExpressionsEqual(a, b)
854+
}
855+
812856
// ============================================================================
813857
// Helper functions
814858
// ============================================================================

‎packages/db/tests/query/predicate-utils.test.ts‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,8 @@ describe(`isLimitSubset`, () => {
648648
})
649649

650650
describe(`isPredicateSubset`, () => {
651-
it(`should check all components`, () => {
651+
it(`should check all components for unlimited superset`, () => {
652+
// For unlimited supersets, where-subset logic applies
652653
const subset: LoadSubsetOptions = {
653654
where: gt(ref(`age`), val(20)),
654655
orderBy: [orderByClause(ref(`age`), `asc`)],
@@ -660,11 +661,66 @@ describe(`isPredicateSubset`, () => {
660661
orderByClause(ref(`age`), `asc`),
661662
orderByClause(ref(`name`), `desc`),
662663
],
664+
// No limit - unlimited superset
665+
}
666+
expect(isPredicateSubset(subset, superset)).toBe(true)
667+
})
668+
669+
it(`should require equal where clauses for limited supersets`, () => {
670+
// For limited supersets, where clauses must be EQUAL
671+
const sameWhere = gt(ref(`age`), val(10))
672+
673+
const subset: LoadSubsetOptions = {
674+
where: sameWhere,
675+
orderBy: [orderByClause(ref(`age`), `asc`)],
676+
limit: 5,
677+
}
678+
const superset: LoadSubsetOptions = {
679+
where: sameWhere, // Same where clause
680+
orderBy: [
681+
orderByClause(ref(`age`), `asc`),
682+
orderByClause(ref(`name`), `desc`),
683+
],
663684
limit: 20,
664685
}
665686
expect(isPredicateSubset(subset, superset)).toBe(true)
666687
})
667688

689+
it(`should return false for limited superset with different where clause`, () => {
690+
// Even if subset's where is more restrictive, it can't be a subset
691+
// of a limited superset with a different where clause.
692+
// The top N items of "age > 20" may not be in the top M items of "age > 10"
693+
const subset: LoadSubsetOptions = {
694+
where: gt(ref(`age`), val(20)), // More restrictive
695+
orderBy: [orderByClause(ref(`age`), `asc`)],
696+
limit: 5,
697+
}
698+
const superset: LoadSubsetOptions = {
699+
where: gt(ref(`age`), val(10)), // Less restrictive but LIMITED
700+
orderBy: [orderByClause(ref(`age`), `asc`)],
701+
limit: 20,
702+
}
703+
// This should be FALSE because the top 5 of "age > 20"
704+
// might include items outside the top 20 of "age > 10"
705+
expect(isPredicateSubset(subset, superset)).toBe(false)
706+
})
707+
708+
it(`should return false for limited superset with no where vs subset with where`, () => {
709+
// This is the reported bug case: pagination with search filter
710+
const subset: LoadSubsetOptions = {
711+
where: gt(ref(`age`), val(20)), // Has a filter
712+
orderBy: [orderByClause(ref(`age`), `asc`)],
713+
limit: 10,
714+
}
715+
const superset: LoadSubsetOptions = {
716+
where: undefined, // No filter but LIMITED
717+
orderBy: [orderByClause(ref(`age`), `asc`)],
718+
limit: 10,
719+
}
720+
// The filtered results might include items outside the unfiltered top 10
721+
expect(isPredicateSubset(subset, superset)).toBe(false)
722+
})
723+
668724
it(`should return false if where is not subset`, () => {
669725
const subset: LoadSubsetOptions = {
670726
where: gt(ref(`age`), val(5)),

‎packages/db/tests/query/subset-dedupe.test.ts‎

Lines changed: 147 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,24 +153,68 @@ describe(`createDeduplicatedLoadSubset`, () => {
153153
},
154154
]
155155

156+
const whereClause = gt(ref(`age`), val(10))
157+
156158
// First call: age > 10, orderBy age asc, limit 10
157159
await deduplicated.loadSubset({
158-
where: gt(ref(`age`), val(10)),
160+
where: whereClause,
159161
orderBy: orderBy1,
160162
limit: 10,
161163
})
162164
expect(callCount).toBe(1)
163165

164-
// Second call: age > 20, orderBy age asc, limit 5 (subset)
166+
// Second call: SAME where clause, same orderBy, smaller limit (subset)
167+
// For limited queries, where clauses must be EQUAL for subset relationship
165168
const result = await deduplicated.loadSubset({
166-
where: gt(ref(`age`), val(20)),
169+
where: whereClause, // Same where clause
167170
orderBy: orderBy1,
168171
limit: 5,
169172
})
170173
expect(result).toBe(true)
171174
expect(callCount).toBe(1) // Should not call - subset of first
172175
})
173176

177+
it(`should NOT dedupe limited calls with different where clauses`, async () => {
178+
let callCount = 0
179+
const mockLoadSubset = () => {
180+
callCount++
181+
return Promise.resolve()
182+
}
183+
184+
const deduplicated = new DeduplicatedLoadSubset({
185+
loadSubset: mockLoadSubset,
186+
})
187+
188+
const orderBy1: OrderBy = [
189+
{
190+
expression: ref(`age`),
191+
compareOptions: {
192+
direction: `asc`,
193+
nulls: `last`,
194+
stringSort: `lexical`,
195+
},
196+
},
197+
]
198+
199+
// First call: age > 10, orderBy age asc, limit 10
200+
await deduplicated.loadSubset({
201+
where: gt(ref(`age`), val(10)),
202+
orderBy: orderBy1,
203+
limit: 10,
204+
})
205+
expect(callCount).toBe(1)
206+
207+
// Second call: DIFFERENT where clause (age > 20) - should NOT be deduped
208+
// even though age > 20 is "more restrictive" than age > 10,
209+
// the top 5 of age > 20 might not be in the top 10 of age > 10
210+
await deduplicated.loadSubset({
211+
where: gt(ref(`age`), val(20)),
212+
orderBy: orderBy1,
213+
limit: 5,
214+
})
215+
expect(callCount).toBe(2) // Should call - different where clause
216+
})
217+
174218
it(`should call underlying for non-subset limited calls`, async () => {
175219
let callCount = 0
176220
const mockLoadSubset = () => {
@@ -671,17 +715,20 @@ describe(`createDeduplicatedLoadSubset`, () => {
671715
},
672716
]
673717

718+
const whereClause = gt(ref(`age`), val(10))
719+
674720
// First limited call
675721
await deduplicated.loadSubset({
676-
where: gt(ref(`age`), val(10)),
722+
where: whereClause,
677723
orderBy: orderBy1,
678724
limit: 10,
679725
})
680726
expect(callCount).toBe(1)
681727

682-
// Second limited call is a subset (stricter where and smaller limit)
728+
// Second limited call is a subset (SAME where clause and smaller limit)
729+
// For limited queries, where clauses must be EQUAL for subset relationship
683730
const subsetOptions = {
684-
where: gt(ref(`age`), val(20)),
731+
where: whereClause, // Same where clause
685732
orderBy: orderBy1,
686733
limit: 5,
687734
}
@@ -741,4 +788,98 @@ describe(`createDeduplicatedLoadSubset`, () => {
741788
expect(onDeduplicate).toHaveBeenCalledWith(subsetOptions)
742789
})
743790
})
791+
792+
describe(`limited queries with different where clauses`, () => {
793+
// When a query has a limit, only the top N rows (by orderBy) are loaded.
794+
// A subsequent query with a different where clause cannot reuse that data,
795+
// even if the new where clause is "more restrictive", because the filtered
796+
// top N might include rows outside the original unfiltered top N.
797+
798+
it(`should NOT dedupe when where clause differs on limited queries`, async () => {
799+
let callCount = 0
800+
const calls: Array<LoadSubsetOptions> = []
801+
const mockLoadSubset = (options: LoadSubsetOptions) => {
802+
callCount++
803+
calls.push(options)
804+
return Promise.resolve()
805+
}
806+
807+
const deduplicated = new DeduplicatedLoadSubset({
808+
loadSubset: mockLoadSubset,
809+
})
810+
811+
const orderByCreatedAt: OrderBy = [
812+
{
813+
expression: ref(`created_at`),
814+
compareOptions: {
815+
direction: `desc`,
816+
nulls: `last`,
817+
stringSort: `lexical`,
818+
},
819+
},
820+
]
821+
822+
// First query: top 10 items with no filter
823+
await deduplicated.loadSubset({
824+
where: undefined,
825+
orderBy: orderByCreatedAt,
826+
limit: 10,
827+
})
828+
expect(callCount).toBe(1)
829+
830+
// Second query: top 10 items WITH a filter
831+
// This requires a separate request because the filtered top 10
832+
// might include items outside the unfiltered top 10
833+
const searchWhere = and(eq(ref(`title`), val(`test`)))
834+
await deduplicated.loadSubset({
835+
where: searchWhere,
836+
orderBy: orderByCreatedAt,
837+
limit: 10,
838+
})
839+
840+
expect(callCount).toBe(2)
841+
expect(calls[1]?.where).toEqual(searchWhere)
842+
})
843+
844+
it(`should dedupe when where clause is identical on limited queries`, async () => {
845+
let callCount = 0
846+
const mockLoadSubset = () => {
847+
callCount++
848+
return Promise.resolve()
849+
}
850+
851+
const deduplicated = new DeduplicatedLoadSubset({
852+
loadSubset: mockLoadSubset,
853+
})
854+
855+
const orderByCreatedAt: OrderBy = [
856+
{
857+
expression: ref(`created_at`),
858+
compareOptions: {
859+
direction: `desc`,
860+
nulls: `last`,
861+
stringSort: `lexical`,
862+
},
863+
},
864+
]
865+
866+
// First query: top 10 items with no filter
867+
await deduplicated.loadSubset({
868+
where: undefined,
869+
orderBy: orderByCreatedAt,
870+
limit: 10,
871+
})
872+
expect(callCount).toBe(1)
873+
874+
// Second query: same where clause (undefined), smaller limit
875+
// The top 5 are contained within the already-loaded top 10
876+
const result = await deduplicated.loadSubset({
877+
where: undefined,
878+
orderBy: orderByCreatedAt,
879+
limit: 5,
880+
})
881+
expect(result).toBe(true)
882+
expect(callCount).toBe(1)
883+
})
884+
})
744885
})

0 commit comments

Comments
 (0)