fix(query-db): clean up empty ownership sets

.changeset/clean-query-ownership.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/query-db-collection': patch
+---
+
+Clean up empty query ownership state while preserving authoritative empty results and retained-row lifecycle behavior.

packages/query-db-collection/src/query.ts

@@ -737,7 +737,8 @@ export function queryCollectionOptions(
   // hashedQueryKey → queryKey
   const hashToQueryKey = new Map<string, QueryKey>()
 
-  // queryKey → Set<RowKey>
+  // queryKey → Set<RowKey>. Entry presence means ownership is resolved;
+  // an empty set represents a resolved query that currently owns no rows.
   const queryToRows = new Map<string, Set<string | number>>()
 
   // RowKey → Set<queryKey>
@@ -775,6 +776,11 @@ export function queryCollectionOptions(
   }
 
   const addRowOwners = (rowKey: string | number, owners: Set<string>) => {
+    if (owners.size === 0) {
+      rowToQueries.delete(rowKey)
+      return
+    }
+
     rowToQueries.set(rowKey, new Set(owners))
     owners.forEach((owner) => {
       const ownedRows = queryToRows.get(owner) || new Set<string | number>()
@@ -784,16 +790,16 @@ export function queryCollectionOptions(
   }
 
   const removeRowOwner = (rowKey: string | number, hashedQueryKey: string) => {
-    const owners = rowToQueries.get(rowKey) || new Set<string>()
-    owners.delete(hashedQueryKey)
-    rowToQueries.set(rowKey, owners)
+    const owners = rowToQueries.get(rowKey)
+    owners?.delete(hashedQueryKey)
+    if (!owners?.size) {
+      rowToQueries.delete(rowKey)
+    }
 
-    const ownedRows =
-      queryToRows.get(hashedQueryKey) || new Set<string | number>()
-    ownedRows.delete(rowKey)
-    queryToRows.set(hashedQueryKey, ownedRows)
+    const ownedRows = queryToRows.get(hashedQueryKey)
+    ownedRows?.delete(rowKey)
 
-    return owners.size === 0
+    return !owners?.size
   }
 
   const removeQueryOwnership = (hashedQueryKey: string) => {
@@ -1075,6 +1081,7 @@ export function queryCollectionOptions(
         `${QUERY_COLLECTION_GC_PREFIX}${hashedQueryKey}`,
       )
       commit()
+      queryToRows.delete(hashedQueryKey)
     }
 
     const schedulePersistedRetentionExpiry = (
@@ -1386,6 +1393,13 @@ export function queryCollectionOptions(
       const previouslyOwnedRows = shouldUsePersistedBaseline
         ? new Set(persistedBaseline.keys())
         : getHydratedOwnedRowsForQueryBaseline(hashedQueryKey)
+      // From this point onward the result, including an empty result, is the
+      // authoritative ownership baseline until this query is cleaned up.
+      queryToRows.set(
+        hashedQueryKey,
+        queryToRows.get(hashedQueryKey) ?? new Set<string | number>(),
+      )
+
       const newItemsMap = new Map<string | number, any>()
       newItemsArray.forEach((item) => {
         const key = getKey(item)
@@ -2014,6 +2028,15 @@ export function queryCollectionOptions(
     }
   }
 
+  if (typeof process !== `undefined` && process.env.NODE_ENV === `test`) {
+    Object.defineProperty(enhancedInternalSync, `__getOwnershipMapsForTests`, {
+      value: () => ({
+        rowToQueries,
+        queryToRows,
+      }),
+    })
+  }
+
   // Create write utils using the manual-sync module
   const writeUtils = createWriteUtils<any, string | number, any>(
     () => writeContext,

packages/query-db-collection/tests/query.test.ts

@@ -47,6 +47,28 @@ interface CategorisedItem {
 
 const getKey = (item: TestItem) => item.id
 
+type OwnershipMaps = {
+  rowToQueries: Map<string | number, Set<string>>
+  queryToRows: Map<string, Set<string | number>>
+}
+
+function inspectOwnershipMaps(options: {
+  sync: { sync: unknown }
+}): OwnershipMaps {
+  const sync = options.sync.sync as {
+    __getOwnershipMapsForTests?: () => OwnershipMaps
+  }
+  const maps = sync.__getOwnershipMapsForTests?.()
+  if (!maps) {
+    throw new Error(`Ownership-map test inspection is unavailable`)
+  }
+  return maps
+}
+
+function expectNoEmptyRowOwnershipSets(maps: OwnershipMaps): void {
+  maps.rowToQueries.forEach((owners) => expect(owners.size).toBeGreaterThan(0))
+}
+
 // Helper to advance timers and allow microtasks to flush
 const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0))
 
@@ -5074,6 +5096,193 @@ describe(`QueryCollection`, () => {
       })
     })
 
+    describe(`ownership lifecycle characterization`, () => {
+      it(`removes only rows whose final subset owner is unloaded`, async () => {
+        const queryFn = vi
+          .fn()
+          .mockResolvedValueOnce([
+            { id: `1`, name: `First only` },
+            { id: `2`, name: `Shared` },
+          ])
+          .mockResolvedValueOnce([
+            { id: `2`, name: `Shared` },
+            { id: `3`, name: `Second only` },
+          ])
+        const options = queryCollectionOptions<TestItem>({
+          id: `ownership-overlapping-subsets-test`,
+          queryClient,
+          queryKey: [`ownership-overlapping-subsets-test`],
+          queryFn,
+          getKey,
+          syncMode: `on-demand`,
+        })
+        const ownershipMaps = inspectOwnershipMaps(options)
+        const collection = createCollection(options)
+        const firstSubset = createLiveQueryCollection({
+          query: (q) =>
+            q
+              .from({ item: collection })
+              .where(({ item }) => inArray(item.id, [`1`, `2`])),
+        })
+        const secondSubset = createLiveQueryCollection({
+          query: (q) =>
+            q
+              .from({ item: collection })
+              .where(({ item }) => inArray(item.id, [`2`, `3`])),
+        })
+
+        await firstSubset.preload()
+        await secondSubset.preload()
+        expect(collection.size).toBe(3)
+
+        await firstSubset.cleanup()
+        await vi.waitFor(() => {
+          expect(collection.has(`1`)).toBe(false)
+          expect(collection.has(`2`)).toBe(true)
+          expect(collection.has(`3`)).toBe(true)
+        })
+        expectNoEmptyRowOwnershipSets(ownershipMaps)
+
+        await secondSubset.cleanup()
+        await vi.waitFor(() => {
+          expect(collection.size).toBe(0)
+        })
+        expectNoEmptyRowOwnershipSets(ownershipMaps)
+        expect(ownershipMaps.rowToQueries.size).toBe(0)
+        expect(ownershipMaps.queryToRows.size).toBe(0)
+      })
+
+      it(`expires the Query cache entry after unload without restoring deleted rows`, async () => {
+        const expiringQueryClient = new QueryClient({
+          defaultOptions: {
+            queries: { gcTime: 100, retry: false, staleTime: Infinity },
+          },
+        })
+        const queryKey = [`ownership-query-cache-expiry-test`]
+        const collection = createCollection(
+          queryCollectionOptions<TestItem>({
+            id: `ownership-query-cache-expiry-test`,
+            queryClient: expiringQueryClient,
+            queryKey,
+            queryFn: async () => [{ id: `1`, name: `Only row` }],
+            getKey,
+            syncMode: `on-demand`,
+          }),
+        )
+        const liveQuery = createLiveQueryCollection({
+          query: (q) => q.from({ item: collection }),
+        })
+
+        await liveQuery.preload()
+        expect(collection.has(`1`)).toBe(true)
+
+        vi.useFakeTimers()
+        try {
+          await liveQuery.cleanup()
+          expect(collection.size).toBe(0)
+          expect(
+            expiringQueryClient.getQueryCache().find({ queryKey }),
+          ).toBeDefined()
+
+          await vi.advanceTimersByTimeAsync(101)
+
+          expect(
+            expiringQueryClient.getQueryCache().find({ queryKey }),
+          ).toBeUndefined()
+          expect(collection.size).toBe(0)
+        } finally {
+          vi.useRealTimers()
+          expiringQueryClient.clear()
+        }
+      })
+
+      it(`hydrates a retained row and deletes it when its query revalidates empty`, async () => {
+        const queryKey = [`ownership-retained-hydration-test`]
+        const queryHash = hashKey(queryKey)
+        const retainedRow: CategorisedItem = {
+          id: `1`,
+          name: `Retained row`,
+          category: `A`,
+        }
+        let releaseRevalidation!: () => void
+        const revalidationReleased = new Promise<void>((resolve) => {
+          releaseRevalidation = resolve
+        })
+        const queryFn = vi.fn(async () => {
+          await revalidationReleased
+          return []
+        })
+        const baseOptions = queryCollectionOptions<CategorisedItem>({
+          id: `ownership-retained-hydration-test`,
+          queryClient,
+          queryKey,
+          queryFn,
+          getKey: (item) => item.id,
+          startSync: true,
+        })
+        const originalSync = baseOptions.sync
+        const ownershipMaps = inspectOwnershipMaps(baseOptions)
+        const metadataHarness = createInMemorySyncMetadataApi<
+          string | number,
+          CategorisedItem
+        >({
+          persistedRows: new Map([[retainedRow.id, retainedRow]]),
+          rowMetadata: new Map([
+            [
+              retainedRow.id,
+              {
+                queryCollection: { owners: { [queryHash]: true } },
+              },
+            ],
+          ]),
+          collectionMetadata: new Map([
+            [
+              `queryCollection:gc:${queryHash}`,
+              { queryHash, mode: `until-revalidated` },
+            ],
+          ]),
+        })
+        const collection = createCollection({
+          ...baseOptions,
+          sync: {
+            sync: (params: Parameters<typeof originalSync.sync>[0]) => {
+              params.begin({ immediate: true })
+              params.write({ type: `insert`, value: retainedRow })
+              params.commit()
+              return originalSync.sync({
+                ...params,
+                metadata: metadataHarness.api,
+              })
+            },
+          },
+        })
+
+        expect(collection.has(retainedRow.id)).toBe(true)
+        expect(
+          metadataHarness.collectionMetadata.has(
+            `queryCollection:gc:${queryHash}`,
+          ),
+        ).toBe(true)
+
+        releaseRevalidation()
+        await vi.waitFor(() => {
+          expect(queryFn).toHaveBeenCalledTimes(1)
+          expect(collection.has(retainedRow.id)).toBe(false)
+        })
+        expect(metadataHarness.rowMetadata.get(retainedRow.id)).toBeUndefined()
+        expectNoEmptyRowOwnershipSets(ownershipMaps)
+        expect(ownershipMaps.rowToQueries.size).toBe(0)
+        expect(ownershipMaps.queryToRows.get(queryHash)).toEqual(new Set())
+        expect(
+          metadataHarness.collectionMetadata.has(
+            `queryCollection:gc:${queryHash}`,
+          ),
+        ).toBe(false)
+
+        await collection.cleanup()
+      })
+    })
+
     it(`should handle duplicate subset loads correctly (refcount bug)`, async () => {
       // This test catches Bug 1: missing refcount increment when reusing existing observer
       // When two subscriptions load the same subset, unloading one should NOT destroy
@@ -5671,6 +5880,7 @@ describe(`QueryCollection`, () => {
 
         const baseOptions = queryCollectionOptions(config)
         const originalSync = baseOptions.sync
+        const ownershipMaps = inspectOwnershipMaps(baseOptions)
         const metadataHarness = createInMemorySyncMetadataApi<
           string | number,
           CategorisedItem
@@ -5703,6 +5913,13 @@ describe(`QueryCollection`, () => {
 
         await liveQuery.cleanup()
 
+        expect(ownershipMaps.queryToRows.has(retainedQueryHash)).toBe(true)
+        expect(ownershipMaps.queryToRows.get(retainedQueryHash)).toEqual(
+          new Set([`1`]),
+        )
+        expect(ownershipMaps.rowToQueries.get(`1`)).toEqual(
+          new Set([retainedQueryHash]),
+        )
         expect(
           metadataHarness.collectionMetadata.get(
             `queryCollection:gc:${retainedQueryHash}`,
@@ -5722,6 +5939,8 @@ describe(`QueryCollection`, () => {
           ),
         ).toBeUndefined()
         expect(collection.has(`1`)).toBe(false)
+        expect(ownershipMaps.queryToRows.has(retainedQueryHash)).toBe(false)
+        expect(ownershipMaps.rowToQueries.has(`1`)).toBe(false)
       } finally {
         vi.useRealTimers()
       }