Modify DeduplicatedLoadSubset.loadSubset such that it only loads the missing data

kevin-dp · kevin-dp · commit 54c152c04280 · 2025-10-16T14:08:14.000+02:00
diff --git a/packages/db/src/query/subset-dedupe.ts b/packages/db/src/query/subset-dedupe.ts
@@ -1,6 +1,7 @@
 import {
   isPredicateSubset,
   isWhereSubset,
+  minusWherePredicates,
   unionWherePredicates,
 } from "./predicate-utils.js"
 import type { BasicExpression } from "./ir.js"
@@ -105,19 +106,31 @@ export class DeduplicatedLoadSubset {
       return matchingInflight.promise
     }
 
-    // Not covered by existing data - call underlying loadSubset
-    const resultPromise = this._loadSubset(options)
+    // Not fully covered by existing data
+    // Compute the subset of data that is not covered by the existing data
+    // such that we only have to load that subset of missing data
+    const clonedOptions = cloneOptions(options)
+    if (this.unlimitedWhere !== undefined && options.limit === undefined) {
+      // Compute difference to get only the missing data
+      // We can only do this for unlimited queries
+      // and we can only remove data that was loaded from unlimited queries
+      // because with limited queries we have no way to express that we already loaded part of the matching data
+      clonedOptions.where =
+        minusWherePredicates(clonedOptions.where, this.unlimitedWhere) ??
+        clonedOptions.where
+    }
+
+    // Call underlying loadSubset to load the missing data
+    const resultPromise = this._loadSubset(clonedOptions)
 
     // Handle both sync (true) and async (Promise<void>) return values
     if (resultPromise === true) {
       // Sync return - update tracking synchronously
       // Clone options before storing to protect against caller mutation
-      this.updateTracking(cloneOptions(options))
+      this.updateTracking(clonedOptions)
       return true
     } else {
       // Async return - track the promise and update tracking after it resolves
-      // Clone options BEFORE entering async context to prevent mutation issues
-      const clonedOptions = cloneOptions(options)
 
       // Capture the current generation - this lets us detect if reset() was called
       // while this request was in-flight, so we can skip updating tracking state
@@ -205,7 +218,7 @@ export class DeduplicatedLoadSubset {
  * properties like limit or where between calls. Without cloning, our stored history
  * would reflect the mutated values rather than what was actually loaded.
  */
-function cloneOptions(options: LoadSubsetOptions): LoadSubsetOptions {
+export function cloneOptions(options: LoadSubsetOptions): LoadSubsetOptions {
   return {
     where: options.where,
     orderBy: options.orderBy,
diff --git a/packages/db/tests/subset-dedupe.test.ts b/packages/db/tests/subset-dedupe.test.ts
@@ -1,6 +1,10 @@
 import { describe, expect, it } from "vitest"
-import { DeduplicatedLoadSubset } from "../src/query/subset-dedupe"
+import {
+  DeduplicatedLoadSubset,
+  cloneOptions,
+} from "../src/query/subset-dedupe"
 import { Func, PropRef, Value } from "../src/query/ir"
+import { minusWherePredicates } from "../src/query/predicate-utils"
 import type { BasicExpression, OrderBy } from "../src/query/ir"
 import type { LoadSubsetOptions } from "../src/types"
 
@@ -25,6 +29,22 @@ function eq(left: BasicExpression<any>, right: BasicExpression<any>): Func {
   return new Func(`eq`, [left, right])
 }
 
+function and(...expressions: Array<BasicExpression<boolean>>): Func {
+  return new Func(`and`, expressions)
+}
+
+function inOp(left: BasicExpression<any>, values: Array<any>): Func {
+  return new Func(`in`, [left, new Value(values)])
+}
+
+function lte(left: BasicExpression<any>, right: BasicExpression<any>): Func {
+  return new Func(`lte`, [left, right])
+}
+
+function not(expression: BasicExpression<boolean>): Func {
+  return new Func(`not`, [expression])
+}
+
 describe(`createDeduplicatedLoadSubset`, () => {
   it(`should call underlying loadSubset on first call`, async () => {
     let callCount = 0
@@ -322,4 +342,221 @@ describe(`createDeduplicatedLoadSubset`, () => {
     expect(calls[0]).toEqual({ where: eq(ref(`status`), val(`active`)) })
     expect(calls[1]).toEqual({ where: eq(ref(`status`), val(`inactive`)) })
   })
+
+  describe(`subset deduplication with minusWherePredicates`, () => {
+    it(`should request only the difference for range predicates`, async () => {
+      let callCount = 0
+      const calls: Array<LoadSubsetOptions> = []
+      const mockLoadSubset = (options: LoadSubsetOptions) => {
+        callCount++
+        calls.push(cloneOptions(options))
+        return Promise.resolve()
+      }
+
+      const deduplicated = new DeduplicatedLoadSubset(mockLoadSubset)
+
+      // First call: age > 20 (loads data for age > 20)
+      await deduplicated.loadSubset({ where: gt(ref(`age`), val(20)) })
+      expect(callCount).toBe(1)
+      expect(calls[0]).toEqual({ where: gt(ref(`age`), val(20)) })
+
+      // Second call: age > 10 (should request only age > 10 AND age <= 20)
+      await deduplicated.loadSubset({ where: gt(ref(`age`), val(10)) })
+      expect(callCount).toBe(2)
+      expect(calls[1]).toEqual({
+        where: and(gt(ref(`age`), val(10)), lte(ref(`age`), val(20))),
+      })
+    })
+
+    it(`should request only the difference for set predicates`, async () => {
+      let callCount = 0
+      const calls: Array<LoadSubsetOptions> = []
+      const mockLoadSubset = (options: LoadSubsetOptions) => {
+        callCount++
+        calls.push(cloneOptions(options))
+        return Promise.resolve()
+      }
+
+      const deduplicated = new DeduplicatedLoadSubset(mockLoadSubset)
+
+      // First call: status IN ['B', 'C'] (loads data for B and C)
+      await deduplicated.loadSubset({
+        where: inOp(ref(`status`), [`B`, `C`]),
+      })
+      expect(callCount).toBe(1)
+      expect(calls[0]).toEqual({ where: inOp(ref(`status`), [`B`, `C`]) })
+
+      // Second call: status IN ['A', 'B', 'C', 'D'] (should request only A and D)
+      await deduplicated.loadSubset({
+        where: inOp(ref(`status`), [`A`, `B`, `C`, `D`]),
+      })
+      expect(callCount).toBe(2)
+      expect(calls[1]).toEqual({
+        where: inOp(ref(`status`), [`A`, `D`]),
+      })
+    })
+
+    it(`should return true immediately for complete overlap`, async () => {
+      let callCount = 0
+      const calls: Array<LoadSubsetOptions> = []
+      const mockLoadSubset = (options: LoadSubsetOptions) => {
+        callCount++
+        calls.push(cloneOptions(options))
+        return Promise.resolve()
+      }
+
+      const deduplicated = new DeduplicatedLoadSubset(mockLoadSubset)
+
+      // First call: age > 10 (loads data for age > 10)
+      await deduplicated.loadSubset({ where: gt(ref(`age`), val(10)) })
+      expect(callCount).toBe(1)
+
+      // Second call: age > 20 (completely covered by first call)
+      const result = await deduplicated.loadSubset({
+        where: gt(ref(`age`), val(20)),
+      })
+      expect(result).toBe(true)
+      expect(callCount).toBe(1) // Should not make additional call
+    })
+
+    it(`should handle complex predicate differences`, async () => {
+      let callCount = 0
+      const calls: Array<LoadSubsetOptions> = []
+      const mockLoadSubset = (options: LoadSubsetOptions) => {
+        callCount++
+        calls.push(cloneOptions(options))
+        return Promise.resolve()
+      }
+
+      const deduplicated = new DeduplicatedLoadSubset(mockLoadSubset)
+
+      // First call: age > 20 AND status = 'active'
+      const firstPredicate = and(
+        gt(ref(`age`), val(20)),
+        eq(ref(`status`), val(`active`))
+      )
+      await deduplicated.loadSubset({ where: firstPredicate })
+      expect(callCount).toBe(1)
+      expect(calls[0]).toEqual({ where: firstPredicate })
+
+      // Second call: age > 10 AND status = 'active' (should request only age > 10 AND age <= 20 AND status = 'active')
+      const secondPredicate = and(
+        gt(ref(`age`), val(10)),
+        eq(ref(`status`), val(`active`))
+      )
+
+      const test = minusWherePredicates(secondPredicate, firstPredicate)
+      console.log(`test`, test)
+
+      await deduplicated.loadSubset({ where: secondPredicate })
+      expect(callCount).toBe(2)
+      expect(calls[1]).toEqual({
+        where: and(
+          eq(ref(`status`), val(`active`)),
+          gt(ref(`age`), val(10)),
+          lte(ref(`age`), val(20))
+        ),
+      })
+    })
+
+    it(`should not apply subset logic to limited calls`, async () => {
+      let callCount = 0
+      const calls: Array<LoadSubsetOptions> = []
+      const mockLoadSubset = (options: LoadSubsetOptions) => {
+        callCount++
+        calls.push(cloneOptions(options))
+        return Promise.resolve()
+      }
+
+      const deduplicated = new DeduplicatedLoadSubset(mockLoadSubset)
+
+      const orderBy1: OrderBy = [
+        {
+          expression: ref(`age`),
+          compareOptions: {
+            direction: `asc`,
+            nulls: `last`,
+            stringSort: `lexical`,
+          },
+        },
+      ]
+
+      // First call: unlimited age > 20
+      await deduplicated.loadSubset({ where: gt(ref(`age`), val(20)) })
+      expect(callCount).toBe(1)
+
+      // Second call: limited age > 10 with orderBy + limit
+      // Should request the full predicate, not the difference, because it's limited
+      await deduplicated.loadSubset({
+        where: gt(ref(`age`), val(10)),
+        orderBy: orderBy1,
+        limit: 10,
+      })
+      expect(callCount).toBe(2)
+      expect(calls[1]).toEqual({
+        where: gt(ref(`age`), val(10)),
+        orderBy: orderBy1,
+        limit: 10,
+      })
+    })
+
+    it(`should handle undefined where clauses in subset logic`, async () => {
+      let callCount = 0
+      const calls: Array<LoadSubsetOptions> = []
+      const mockLoadSubset = (options: LoadSubsetOptions) => {
+        callCount++
+        calls.push(cloneOptions(options))
+        return Promise.resolve()
+      }
+
+      const deduplicated = new DeduplicatedLoadSubset(mockLoadSubset)
+
+      // First call: age > 20
+      await deduplicated.loadSubset({ where: gt(ref(`age`), val(20)) })
+      expect(callCount).toBe(1)
+
+      // Second call: no where clause (all data)
+      // Should request all data except what we already loaded
+      // i.e. should request NOT (age > 20)
+      await deduplicated.loadSubset({})
+      expect(callCount).toBe(2)
+      expect(calls[1]).toEqual({ where: not(gt(ref(`age`), val(20))) }) // Should request all data except what we already loaded
+    })
+
+    it(`should handle multiple overlapping unlimited calls`, async () => {
+      let callCount = 0
+      const calls: Array<LoadSubsetOptions> = []
+      const mockLoadSubset = (options: LoadSubsetOptions) => {
+        callCount++
+        calls.push(cloneOptions(options))
+        return Promise.resolve()
+      }
+
+      const deduplicated = new DeduplicatedLoadSubset(mockLoadSubset)
+
+      // First call: age > 20
+      await deduplicated.loadSubset({ where: gt(ref(`age`), val(20)) })
+      expect(callCount).toBe(1)
+
+      // Second call: age < 10 (different range)
+      await deduplicated.loadSubset({ where: lt(ref(`age`), val(10)) })
+      expect(callCount).toBe(2)
+
+      // Third call: age > 5 (should request only age >= 10 AND age <= 20, since age < 10 is already covered)
+      await deduplicated.loadSubset({ where: gt(ref(`age`), val(5)) })
+      expect(callCount).toBe(3)
+
+      // Ideally it would be smart enough to optimize it to request only age >= 10 AND age <= 20, since age < 10 is already covered
+      // However, it doesn't do that currently, so it will not optimize and execute the original query
+      expect(calls[2]).toEqual({
+        where: gt(ref(`age`), val(5)),
+      })
+
+      /*
+      expect(calls[2]).toEqual({
+        where: and(gte(ref(`age`), val(10)), lte(ref(`age`), val(20))),
+      })
+      */
+    })
+  })
 })
