-
Notifications
You must be signed in to change notification settings - Fork 100
change to a subscription per collection alias rather than collection inside a live query #625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
samwillis
wants to merge
14
commits into
main
Choose a base branch
from
samwillis/fix-self-join
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
1249a08
wip
samwillis 8baf80c
convert to a subscription per alias
samwillis 42f81bb
change to subscription per source alias, rather than per colleciton
samwillis 85f51e9
address gpt5 review
samwillis d9b2014
wip
samwillis 9a794a3
rename stuff
samwillis 04688eb
remove fallback to collecitonId
samwillis 4ee665a
better mapping of subquery aliases
samwillis 0f7798b
rename stuff and tidy
samwillis 17ffc31
changeset
samwillis 50316cd
better comments
samwillis 2f6ca4f
remove unnecessary second pass of the compiler
samwillis 88b3092
remove deplicate code
samwillis 7eca921
comments on aliasRemapping and additional tests to confirm
samwillis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@tanstack/db": patch | ||
--- | ||
|
||
Fix self-join bug by implementing per-alias subscriptions in live queries |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,32 +31,51 @@ import type { | |
import type { QueryCache, QueryMapping } from "./types.js" | ||
|
||
/** | ||
* Result of query compilation including both the pipeline and collection-specific WHERE clauses | ||
* Result of query compilation including both the pipeline and source-specific WHERE clauses | ||
*/ | ||
export interface CompilationResult { | ||
/** The ID of the main collection */ | ||
collectionId: string | ||
/** The compiled query pipeline */ | ||
|
||
/** The compiled query pipeline (D2 stream) */ | ||
pipeline: ResultStream | ||
/** Map of collection aliases to their WHERE clauses for index optimization */ | ||
collectionWhereClauses: Map<string, BasicExpression<boolean>> | ||
|
||
/** Map of source aliases to their WHERE clauses for index optimization */ | ||
sourceWhereClauses: Map<string, BasicExpression<boolean>> | ||
|
||
/** | ||
* Maps each source alias to its collection ID. Enables per-alias subscriptions for self-joins. | ||
* Example: `{ employee: 'employees-col-id', manager: 'employees-col-id' }` | ||
*/ | ||
aliasToCollectionId: Record<string, string> | ||
|
||
/** | ||
* Maps outer alias to inner alias for subqueries (e.g., `{ activeUser: 'user' }`). | ||
* Used to resolve subscriptions during lazy loading when aliases differ. | ||
*/ | ||
aliasRemapping: Record<string, string> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this query below the alias mapping is used to track that
|
||
} | ||
|
||
/** | ||
* Compiles a query2 IR into a D2 pipeline | ||
* Compiles a query IR into a D2 pipeline | ||
* @param rawQuery The query IR to compile | ||
* @param inputs Mapping of collection names to input streams | ||
* @param inputs Mapping of source aliases to input streams (e.g., `{ employee: input1, manager: input2 }`) | ||
* @param collections Mapping of collection IDs to Collection instances | ||
* @param subscriptions Mapping of source aliases to CollectionSubscription instances | ||
* @param callbacks Mapping of source aliases to lazy loading callbacks | ||
* @param lazySources Set of source aliases that should load data lazily | ||
* @param optimizableOrderByCollections Map of collection IDs to order-by optimization info | ||
* @param cache Optional cache for compiled subqueries (used internally for recursion) | ||
* @param queryMapping Optional mapping from optimized queries to original queries | ||
* @returns A CompilationResult with the pipeline and collection WHERE clauses | ||
* @returns A CompilationResult with the pipeline, source WHERE clauses, and alias metadata | ||
*/ | ||
export function compileQuery( | ||
rawQuery: QueryIR, | ||
inputs: Record<string, KeyedStream>, | ||
collections: Record<string, Collection<any, any, any, any, any>>, | ||
subscriptions: Record<string, CollectionSubscription>, | ||
callbacks: Record<string, LazyCollectionCallbacks>, | ||
lazyCollections: Set<string>, | ||
lazySources: Set<string>, | ||
optimizableOrderByCollections: Record<string, OrderByOptimizationInfo>, | ||
cache: QueryCache = new WeakMap(), | ||
queryMapping: QueryMapping = new WeakMap() | ||
|
@@ -68,8 +87,7 @@ export function compileQuery( | |
} | ||
|
||
// Optimize the query before compilation | ||
const { optimizedQuery: query, collectionWhereClauses } = | ||
optimizeQuery(rawQuery) | ||
const { optimizedQuery: query, sourceWhereClauses } = optimizeQuery(rawQuery) | ||
|
||
// Create mapping from optimized query to original for caching | ||
queryMapping.set(query, rawQuery) | ||
|
@@ -78,12 +96,24 @@ export function compileQuery( | |
// Create a copy of the inputs map to avoid modifying the original | ||
const allInputs = { ...inputs } | ||
|
||
// Create a map of table aliases to inputs | ||
const tables: Record<string, KeyedStream> = {} | ||
// Track alias to collection id relationships discovered during compilation. | ||
// This includes all user-declared aliases plus inner aliases from subqueries. | ||
const aliasToCollectionId: Record<string, string> = {} | ||
|
||
// Track alias remapping for subqueries (outer alias → inner alias) | ||
// e.g., when .join({ activeUser: subquery }) where subquery uses .from({ user: collection }) | ||
// we store: aliasRemapping['activeUser'] = 'user' | ||
const aliasRemapping: Record<string, string> = {} | ||
|
||
// Create a map of source aliases to input streams. | ||
// Inputs MUST be keyed by alias (e.g., `{ employee: input1, manager: input2 }`), | ||
// not by collection ID. This enables per-alias subscriptions where different aliases | ||
// of the same collection (e.g., self-joins) maintain independent filtered streams. | ||
const sources: Record<string, KeyedStream> = {} | ||
|
||
// Process the FROM clause to get the main table | ||
// Process the FROM clause to get the main source | ||
const { | ||
alias: mainTableAlias, | ||
alias: mainSource, | ||
input: mainInput, | ||
collectionId: mainCollectionId, | ||
} = processFrom( | ||
|
@@ -92,18 +122,20 @@ export function compileQuery( | |
collections, | ||
subscriptions, | ||
callbacks, | ||
lazyCollections, | ||
lazySources, | ||
optimizableOrderByCollections, | ||
cache, | ||
queryMapping | ||
queryMapping, | ||
aliasToCollectionId, | ||
aliasRemapping | ||
) | ||
tables[mainTableAlias] = mainInput | ||
sources[mainSource] = mainInput | ||
|
||
// Prepare the initial pipeline with the main table wrapped in its alias | ||
// Prepare the initial pipeline with the main source wrapped in its alias | ||
let pipeline: NamespacedAndKeyedStream = mainInput.pipe( | ||
map(([key, row]) => { | ||
// Initialize the record with a nested structure | ||
const ret = [key, { [mainTableAlias]: row }] as [ | ||
const ret = [key, { [mainSource]: row }] as [ | ||
string, | ||
Record<string, typeof row>, | ||
] | ||
|
@@ -116,19 +148,21 @@ export function compileQuery( | |
pipeline = processJoins( | ||
pipeline, | ||
query.join, | ||
tables, | ||
sources, | ||
mainCollectionId, | ||
mainTableAlias, | ||
mainSource, | ||
allInputs, | ||
cache, | ||
queryMapping, | ||
collections, | ||
subscriptions, | ||
callbacks, | ||
lazyCollections, | ||
lazySources, | ||
optimizableOrderByCollections, | ||
rawQuery, | ||
compileQuery | ||
compileQuery, | ||
aliasToCollectionId, | ||
aliasRemapping | ||
) | ||
} | ||
|
||
|
@@ -185,7 +219,7 @@ export function compileQuery( | |
map(([key, namespacedRow]) => { | ||
const selectResults = | ||
!query.join && !query.groupBy | ||
? namespacedRow[mainTableAlias] | ||
? namespacedRow[mainSource] | ||
: namespacedRow | ||
|
||
return [ | ||
|
@@ -286,7 +320,9 @@ export function compileQuery( | |
const compilationResult = { | ||
collectionId: mainCollectionId, | ||
pipeline: result, | ||
collectionWhereClauses, | ||
sourceWhereClauses, | ||
aliasToCollectionId, | ||
aliasRemapping, | ||
} | ||
cache.set(rawQuery, compilationResult) | ||
|
||
|
@@ -314,33 +350,43 @@ export function compileQuery( | |
const compilationResult = { | ||
collectionId: mainCollectionId, | ||
pipeline: result, | ||
collectionWhereClauses, | ||
sourceWhereClauses, | ||
aliasToCollectionId, | ||
aliasRemapping, | ||
} | ||
cache.set(rawQuery, compilationResult) | ||
|
||
return compilationResult | ||
} | ||
|
||
/** | ||
* Processes the FROM clause to extract the main table alias and input stream | ||
* Processes the FROM clause, handling direct collection references and subqueries. | ||
* Populates `aliasToCollectionId` and `aliasRemapping` for per-alias subscription tracking. | ||
*/ | ||
function processFrom( | ||
from: CollectionRef | QueryRef, | ||
allInputs: Record<string, KeyedStream>, | ||
collections: Record<string, Collection>, | ||
subscriptions: Record<string, CollectionSubscription>, | ||
callbacks: Record<string, LazyCollectionCallbacks>, | ||
lazyCollections: Set<string>, | ||
lazySources: Set<string>, | ||
optimizableOrderByCollections: Record<string, OrderByOptimizationInfo>, | ||
cache: QueryCache, | ||
queryMapping: QueryMapping | ||
queryMapping: QueryMapping, | ||
aliasToCollectionId: Record<string, string>, | ||
aliasRemapping: Record<string, string> | ||
): { alias: string; input: KeyedStream; collectionId: string } { | ||
switch (from.type) { | ||
case `collectionRef`: { | ||
const input = allInputs[from.collection.id] | ||
const input = allInputs[from.alias] | ||
if (!input) { | ||
throw new CollectionInputNotFoundError(from.collection.id) | ||
throw new CollectionInputNotFoundError( | ||
from.alias, | ||
from.collection.id, | ||
Object.keys(allInputs) | ||
) | ||
} | ||
aliasToCollectionId[from.alias] = from.collection.id | ||
return { alias: from.alias, input, collectionId: from.collection.id } | ||
} | ||
case `queryRef`: { | ||
|
@@ -354,12 +400,29 @@ function processFrom( | |
collections, | ||
subscriptions, | ||
callbacks, | ||
lazyCollections, | ||
lazySources, | ||
optimizableOrderByCollections, | ||
cache, | ||
queryMapping | ||
) | ||
|
||
// Pull up inner alias mappings from subquery compilation | ||
Object.assign(aliasToCollectionId, subQueryResult.aliasToCollectionId) | ||
Object.assign(aliasRemapping, subQueryResult.aliasRemapping) | ||
|
||
// Create remapping when outer alias differs from inner alias. | ||
// Example: .join({ activeUser: subquery }) where subquery uses .from({ user: ... }) | ||
// Creates: aliasRemapping['activeUser'] = 'user' | ||
// Needed for subscription resolution during lazy loading. | ||
const innerAlias = Object.keys(subQueryResult.aliasToCollectionId).find( | ||
(alias) => | ||
subQueryResult.aliasToCollectionId[alias] === | ||
subQueryResult.collectionId | ||
) | ||
if (innerAlias && innerAlias !== from.alias) { | ||
aliasRemapping[from.alias] = innerAlias | ||
} | ||
|
||
// Extract the pipeline from the compilation result | ||
const subQueryInput = subQueryResult.pipeline | ||
|
||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a mapping of the alias used in a
from
orjoin
to the collection id that is to be used for that source.