diff --git a/.changeset/validate-duplicate-subquery-aliases.md b/.changeset/validate-duplicate-subquery-aliases.md new file mode 100644 index 000000000..4a3643d0d --- /dev/null +++ b/.changeset/validate-duplicate-subquery-aliases.md @@ -0,0 +1,5 @@ +--- +"@tanstack/db": patch +--- + +Validate against duplicate collection aliases in subqueries. Prevents a bug where using the same alias for a collection in both parent and subquery causes empty results or incorrect aggregation values. Now throws a clear `DuplicateAliasInSubqueryError` when this pattern is detected, guiding users to rename the conflicting alias. diff --git a/packages/db/src/errors.ts b/packages/db/src/errors.ts index 7ec6dd478..64180f521 100644 --- a/packages/db/src/errors.ts +++ b/packages/db/src/errors.ts @@ -369,6 +369,22 @@ export class CollectionInputNotFoundError extends QueryCompilationError { } } +/** + * Error thrown when a subquery uses the same alias as its parent query. + * This causes issues because parent and subquery would share the same input streams, + * leading to empty results or incorrect data (aggregation cross-leaking). + */ +export class DuplicateAliasInSubqueryError extends QueryCompilationError { + constructor(alias: string, parentAliases: Array) { + super( + `Subquery uses alias "${alias}" which is already used in the parent query. ` + + `Each alias must be unique across parent and subquery contexts. ` + + `Parent query aliases: ${parentAliases.join(`, `)}. ` + + `Please rename "${alias}" in either the parent query or subquery to avoid conflicts.` + ) + } +} + export class UnsupportedFromTypeError extends QueryCompilationError { constructor(type: string) { super(`Unsupported FROM type: ${type}`) diff --git a/packages/db/src/query/compiler/index.ts b/packages/db/src/query/compiler/index.ts index 1cd033453..621647206 100644 --- a/packages/db/src/query/compiler/index.ts +++ b/packages/db/src/query/compiler/index.ts @@ -3,6 +3,7 @@ import { optimizeQuery } from "../optimizer.js" import { CollectionInputNotFoundError, DistinctRequiresSelectError, + DuplicateAliasInSubqueryError, HavingRequiresGroupByError, LimitOffsetRequireOrderByError, UnsupportedFromTypeError, @@ -99,6 +100,11 @@ export function compileQuery( return cachedResult } + // Validate the raw query BEFORE optimization to check user's original structure. + // This must happen before optimization because the optimizer may create internal + // subqueries (e.g., for predicate pushdown) that reuse aliases, which is fine. + validateQueryStructure(rawQuery) + // Optimize the query before compilation const { optimizedQuery: query, sourceWhereClauses } = optimizeQuery(rawQuery) @@ -375,6 +381,74 @@ export function compileQuery( return compilationResult } +/** + * Collects aliases used for DIRECT collection references (not subqueries). + * Used to validate that subqueries don't reuse parent query collection aliases. + * Only direct CollectionRef aliases matter - QueryRef aliases don't cause conflicts. + */ +function collectDirectCollectionAliases(query: QueryIR): Set { + const aliases = new Set() + + // Collect FROM alias only if it's a direct collection reference + if (query.from.type === `collectionRef`) { + aliases.add(query.from.alias) + } + + // Collect JOIN aliases only for direct collection references + if (query.join) { + for (const joinClause of query.join) { + if (joinClause.from.type === `collectionRef`) { + aliases.add(joinClause.from.alias) + } + } + } + + return aliases +} + +/** + * Validates the structure of a query and its subqueries. + * Checks that subqueries don't reuse collection aliases from parent queries. + * This must be called on the RAW query before optimization. + */ +function validateQueryStructure( + query: QueryIR, + parentCollectionAliases: Set = new Set() +): void { + // Collect direct collection aliases from this query level + const currentLevelAliases = collectDirectCollectionAliases(query) + + // Check if any current alias conflicts with parent aliases + for (const alias of currentLevelAliases) { + if (parentCollectionAliases.has(alias)) { + throw new DuplicateAliasInSubqueryError( + alias, + Array.from(parentCollectionAliases) + ) + } + } + + // Combine parent and current aliases for checking nested subqueries + const combinedAliases = new Set([ + ...parentCollectionAliases, + ...currentLevelAliases, + ]) + + // Recursively validate FROM subquery + if (query.from.type === `queryRef`) { + validateQueryStructure(query.from.query, combinedAliases) + } + + // Recursively validate JOIN subqueries + if (query.join) { + for (const joinClause of query.join) { + if (joinClause.from.type === `queryRef`) { + validateQueryStructure(joinClause.from.query, combinedAliases) + } + } + } +} + /** * Processes the FROM clause, handling direct collection references and subqueries. * Populates `aliasToCollectionId` and `aliasRemapping` for per-alias subscription tracking. diff --git a/packages/db/tests/query/discord-alias-bug.test.ts b/packages/db/tests/query/discord-alias-bug.test.ts new file mode 100644 index 000000000..8c5533bc0 --- /dev/null +++ b/packages/db/tests/query/discord-alias-bug.test.ts @@ -0,0 +1,121 @@ +import { beforeEach, describe, expect, test } from "vitest" +import { createLiveQueryCollection, eq } from "../../src/query/index.js" +import { createCollection } from "../../src/collection/index.js" +import { mockSyncCollectionOptions } from "../utils.js" + +type Lock = { _id: number; name: string } +type Vote = { _id: number; lockId: number; percent: number } + +const locks: Array = [ + { _id: 1, name: `Lock A` }, + { _id: 2, name: `Lock B` }, +] + +const votes: Array = [ + { _id: 1, lockId: 1, percent: 10 }, + { _id: 2, lockId: 1, percent: 20 }, + { _id: 3, lockId: 2, percent: 30 }, +] + +function createTestCollections() { + return { + locksCollection: createCollection( + mockSyncCollectionOptions({ + id: `locks`, + getKey: (lock) => lock._id, + initialData: locks, + autoIndex: `eager`, + }) + ), + votesCollection: createCollection( + mockSyncCollectionOptions({ + id: `votes`, + getKey: (vote) => vote._id, + initialData: votes, + autoIndex: `eager`, + }) + ), + } +} + +describe(`Discord Bug: Same Alias in Parent and Subquery`, () => { + let locksCollection: ReturnType< + typeof createTestCollections + >[`locksCollection`] + let votesCollection: ReturnType< + typeof createTestCollections + >[`votesCollection`] + + beforeEach(() => { + const collections = createTestCollections() + locksCollection = collections.locksCollection + votesCollection = collections.votesCollection + }) + + test(`should throw error when subquery uses same alias as parent (Discord bug)`, () => { + expect(() => { + createLiveQueryCollection({ + startSync: true, + query: (q) => { + const locksAgg = q + .from({ lock: locksCollection }) + .join({ vote: votesCollection }, ({ lock, vote }) => + eq(lock._id, vote.lockId) + ) + .select(({ lock }) => ({ + _id: lock._id, + lockName: lock.name, + })) + + return q + .from({ vote: votesCollection }) // CONFLICT: "vote" alias used here + .join({ lock: locksAgg }, ({ vote, lock }) => + eq(lock._id, vote.lockId) + ) + .select(({ vote, lock }) => ({ + voteId: vote._id, + lockName: lock!.lockName, + })) + }, + }) + }).toThrow(/Subquery uses alias "vote"/) + }) + + test(`workaround: rename alias in one of the queries`, () => { + const query = createLiveQueryCollection({ + startSync: true, + query: (q) => { + const locksAgg = q + .from({ lock: locksCollection }) + .join( + { v: votesCollection }, + ( + { lock, v } // Renamed to "v" + ) => eq(lock._id, v.lockId) + ) + .select(({ lock }) => ({ + _id: lock._id, + lockName: lock.name, + })) + + return q + .from({ vote: votesCollection }) + .join({ lock: locksAgg }, ({ vote, lock }) => + eq(lock._id, vote.lockId) + ) + .select(({ vote, lock }) => ({ + voteId: vote._id, + lockName: lock!.lockName, + })) + }, + }) + + const results = query.toArray + // Each lock (2) joins with each vote for that lock + // Lock 1 has 2 votes, Lock 2 has 1 vote + // But locksAgg groups by lock, so we get 2 aggregated lock records + // Each of the 3 votes joins with its corresponding lock aggregate + expect(results.length).toBeGreaterThan(0) + expect(results.every((r) => r.lockName)).toBe(true) + }) +})