diff --git a/.changeset/lazy-join-resolved-collection.md b/.changeset/lazy-join-resolved-collection.md new file mode 100644 index 0000000000..de05927abe --- /dev/null +++ b/.changeset/lazy-join-resolved-collection.md @@ -0,0 +1,7 @@ +--- +'@tanstack/db': patch +--- + +fix(query): drive lazy-join loading through the collection the join key resolves to + +When a subquery used in a JOIN clause selects its join key from a _joined_ source rather than from its own `from` clause, the lazy-join loader subscribed to the wrong inner source: it used the subquery's `from` alias while computing the index requirement against the collection the key actually resolves to. This produced a misleading `Join requires an index` warning naming an already-indexed collection and an unnecessary full-load fallback. `followRef` now reports the resolved source alias, so lazy loading subscribes to the correct collection and loads through its index. diff --git a/packages/db/src/query/compiler/lazy-targets.ts b/packages/db/src/query/compiler/lazy-targets.ts index ee37c144f8..f8affd1e63 100644 --- a/packages/db/src/query/compiler/lazy-targets.ts +++ b/packages/db/src/query/compiler/lazy-targets.ts @@ -49,9 +49,15 @@ export function getLazyLoadTargets( return [] } + // The subscription we drive lazy loading through must be the one for the + // collection the join key actually resolves to. When the key traces through a + // subquery's select into a *joined* source, that collection differs from the + // subquery's from clause (which is what `aliasRemapping[lazyAlias]` yields), + // so prefer the alias reported by `followRef`. Fall back to the from-clause + // remapping when the key resolves directly to the from source. return [ { - alias: aliasRemapping[lazyAlias] || lazyAlias, + alias: followRefResult.alias || aliasRemapping[lazyAlias] || lazyAlias, collection: followRefResult.collection, path: followRefResult.path, }, diff --git a/packages/db/src/query/ir.ts b/packages/db/src/query/ir.ts index 48019a23b2..0cfb15ea50 100644 --- a/packages/db/src/query/ir.ts +++ b/packages/db/src/query/ir.ts @@ -323,13 +323,17 @@ function getRefFromAlias( /** * Follows the given reference in a query * until its finds the root field the reference points to. - * @returns The collection, its alias, and the path to the root field in this collection + * @returns The collection, its alias, and the path to the root field in this collection. + * `alias` is the alias under which the resolved collection is referenced in the + * query it was reached from (when the ref crosses into a joined source). It is + * left undefined when the ref simply resolves to a field on the passed-in + * `collection`, in which case the caller already knows the alias. */ export function followRef( query: QueryIR, ref: PropRef, collection: Collection, -): { collection: Collection; path: Array } | void { +): { collection: Collection; path: Array; alias?: string } | void { if (ref.path.length === 0) { return } @@ -365,8 +369,10 @@ export function followRef( } else { // This is a reference to a collection // we can't follow it further - // so the field must be on the collection itself - return { collection: aliasRef.collection, path: rest } + // so the field must be on the collection itself. + // Report the alias too: when the ref crossed a join, this is the source + // that actually holds the field (which may differ from the from clause). + return { collection: aliasRef.collection, path: rest, alias } } } } diff --git a/packages/db/tests/query/join-subquery.test.ts b/packages/db/tests/query/join-subquery.test.ts index 093d6e62ae..eac49d6491 100644 --- a/packages/db/tests/query/join-subquery.test.ts +++ b/packages/db/tests/query/join-subquery.test.ts @@ -1,6 +1,7 @@ -import { beforeEach, describe, expect, test } from 'vitest' +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { createLiveQueryCollection, eq, gt } from '../../src/query/index.js' import { createCollection } from '../../src/collection/index.js' +import { BTreeIndex } from '../../src/indexes/btree-index.js' import { mockSyncCollectionOptions, stripVirtualProps } from '../utils.js' // Sample data types for join-subquery testing @@ -873,3 +874,82 @@ describe(`Join with Subqueries`, () => { createJoinSubqueryTests(`off`) createJoinSubqueryTests(`eager`) }) + +describe(`Lazy join: subquery whose join key resolves to an indexed collection`, () => { + type Team = { id: string } + type Member = { id: string; teamId: string } + + // `teams.id` is indexed; `members` has no index. + const makeTeamsCollection = () => + createCollection( + mockSyncCollectionOptions({ + id: `lazy-join-teams`, + getKey: (r) => r.id, + autoIndex: `eager`, + defaultIndexType: BTreeIndex, + initialData: [{ id: `t1` }], + }), + ) + const makeMembersCollection = () => + createCollection( + mockSyncCollectionOptions({ + id: `lazy-join-members`, + getKey: (r) => r.id, + autoIndex: `off`, + initialData: [{ id: `m1`, teamId: `t1` }], + }), + ) + + let teams: ReturnType + let members: ReturnType + let warnSpy: ReturnType + + beforeEach(() => { + teams = makeTeamsCollection() + members = makeMembersCollection() + warnSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + }) + + afterEach(() => { + warnSpy.mockRestore() + }) + + // When a subquery used in a JOIN clause selects its join key from the + // *joined* side of the subquery (here `team.id`) rather than from its own + // FROM side (`member`), the outer join key resolves to `teams.id`, which is + // indexed. The lazy-join loader should therefore load through that index and + // must not emit a "Join requires an index" warning that points at the + // already-indexed `teams` collection. + test(`does not warn about an index that the resolved collection already has`, () => { + const joinQuery = createLiveQueryCollection({ + startSync: true, + query: (q) => { + const teamByMember = q + .from({ member: members }) + .leftJoin({ team: teams }, ({ team, member }) => + eq(team.id, member.teamId), + ) + .select(({ team }) => ({ teamId: team.id })) + + return q + .from({ m: members }) + .leftJoin({ memberTeam: teamByMember }, ({ m, memberTeam }) => + eq(memberTeam.teamId, m.teamId), + ) + .select(({ m }) => ({ id: m.id })) + }, + }) + + // Data flows correctly regardless (via fallback full-load today). + expect(joinQuery.toArray.map((r) => r.id)).toEqual([`m1`]) + + const indexWarnings = warnSpy.mock.calls + .map((c) => String(c[0])) + .filter((m) => m.includes(`Join requires an index`)) + + // `teams.id` is already indexed, so no warning should advise indexing it. + expect(indexWarnings.filter((m) => m.includes(`lazy-join-teams`))).toEqual( + [], + ) + }) +})