Skip to content

Conversation

@kevin-dp
Copy link
Contributor

@kevin-dp kevin-dp commented Jan 7, 2026

Fixes #1005

@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2026

⚠️ No Changeset found

Latest commit: b458750

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 7, 2026

More templates

@tanstack/angular-db

npm i https://pkg.pr.new/@tanstack/angular-db@1097

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@1097

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@1097

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@1097

@tanstack/offline-transactions

npm i https://pkg.pr.new/@tanstack/offline-transactions@1097

@tanstack/powersync-db-collection

npm i https://pkg.pr.new/@tanstack/powersync-db-collection@1097

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@1097

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@1097

@tanstack/rxdb-db-collection

npm i https://pkg.pr.new/@tanstack/rxdb-db-collection@1097

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@1097

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@1097

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@1097

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@1097

commit: b458750

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

Size Change: +127 B (+0.14%)

Total Size: 90.2 kB

Filename Size Change
./packages/db/dist/esm/query/compiler/index.js 2.02 kB +57 B (+2.9%)
./packages/db/dist/esm/query/compiler/joins.js 2.07 kB +70 B (+3.49%)
ℹ️ View Unchanged
Filename Size
./packages/db/dist/esm/collection/change-events.js 1.39 kB
./packages/db/dist/esm/collection/changes.js 1.17 kB
./packages/db/dist/esm/collection/events.js 388 B
./packages/db/dist/esm/collection/index.js 3.32 kB
./packages/db/dist/esm/collection/indexes.js 1.1 kB
./packages/db/dist/esm/collection/lifecycle.js 1.67 kB
./packages/db/dist/esm/collection/mutations.js 2.34 kB
./packages/db/dist/esm/collection/state.js 3.46 kB
./packages/db/dist/esm/collection/subscription.js 3.62 kB
./packages/db/dist/esm/collection/sync.js 2.38 kB
./packages/db/dist/esm/deferred.js 207 B
./packages/db/dist/esm/errors.js 4.49 kB
./packages/db/dist/esm/event-emitter.js 748 B
./packages/db/dist/esm/index.js 2.69 kB
./packages/db/dist/esm/indexes/auto-index.js 742 B
./packages/db/dist/esm/indexes/base-index.js 766 B
./packages/db/dist/esm/indexes/btree-index.js 1.93 kB
./packages/db/dist/esm/indexes/lazy-index.js 1.1 kB
./packages/db/dist/esm/indexes/reverse-index.js 513 B
./packages/db/dist/esm/local-only.js 837 B
./packages/db/dist/esm/local-storage.js 2.1 kB
./packages/db/dist/esm/optimistic-action.js 359 B
./packages/db/dist/esm/paced-mutations.js 496 B
./packages/db/dist/esm/proxy.js 3.75 kB
./packages/db/dist/esm/query/builder/functions.js 733 B
./packages/db/dist/esm/query/builder/index.js 4.01 kB
./packages/db/dist/esm/query/builder/ref-proxy.js 917 B
./packages/db/dist/esm/query/compiler/evaluators.js 1.35 kB
./packages/db/dist/esm/query/compiler/expressions.js 430 B
./packages/db/dist/esm/query/compiler/group-by.js 1.8 kB
./packages/db/dist/esm/query/compiler/order-by.js 1.46 kB
./packages/db/dist/esm/query/compiler/select.js 1.07 kB
./packages/db/dist/esm/query/expression-helpers.js 1.43 kB
./packages/db/dist/esm/query/ir.js 673 B
./packages/db/dist/esm/query/live-query-collection.js 360 B
./packages/db/dist/esm/query/live/collection-config-builder.js 5.33 kB
./packages/db/dist/esm/query/live/collection-registry.js 264 B
./packages/db/dist/esm/query/live/collection-subscriber.js 1.9 kB
./packages/db/dist/esm/query/live/internal.js 130 B
./packages/db/dist/esm/query/optimizer.js 2.56 kB
./packages/db/dist/esm/query/predicate-utils.js 2.97 kB
./packages/db/dist/esm/query/subset-dedupe.js 921 B
./packages/db/dist/esm/scheduler.js 1.3 kB
./packages/db/dist/esm/SortedMap.js 1.3 kB
./packages/db/dist/esm/strategies/debounceStrategy.js 247 B
./packages/db/dist/esm/strategies/queueStrategy.js 428 B
./packages/db/dist/esm/strategies/throttleStrategy.js 246 B
./packages/db/dist/esm/transactions.js 2.9 kB
./packages/db/dist/esm/utils.js 924 B
./packages/db/dist/esm/utils/browser-polyfills.js 304 B
./packages/db/dist/esm/utils/btree.js 5.61 kB
./packages/db/dist/esm/utils/comparison.js 852 B
./packages/db/dist/esm/utils/cursor.js 457 B
./packages/db/dist/esm/utils/index-optimization.js 1.51 kB
./packages/db/dist/esm/utils/type-guards.js 157 B

compressed-size-action::db-package-size

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

Size Change: 0 B

Total Size: 3.47 kB

ℹ️ View Unchanged
Filename Size
./packages/react-db/dist/esm/index.js 225 B
./packages/react-db/dist/esm/useLiveInfiniteQuery.js 1.17 kB
./packages/react-db/dist/esm/useLiveQuery.js 1.12 kB
./packages/react-db/dist/esm/useLiveSuspenseQuery.js 559 B
./packages/react-db/dist/esm/usePacedMutations.js 401 B

compressed-size-action::react-db-package-size

@KyleAMathews
Copy link
Collaborator

/review

@claude
Copy link
Contributor

claude bot commented Jan 20, 2026

🔍 PR Review Summary

PR #1097: Fix where clause missing in loadSubset on subquery collection

Overview

This PR correctly addresses the issue where WHERE clauses were not being passed to loadSubset for subquery collections. The implementation adds proper WHERE clause propagation from subqueries to parent scope in both index.ts and joins.ts.


Critical Issues (0 found)

None - the PR is safe to merge from a correctness standpoint.


Important Issues (4 found)

1. Comment Logic Error (Both Files)

Files: index.ts:513-515, joins.ts:480-482

The comment describing optimizer-created subquery detection is misleading:

// 1. The outer alias (from.alias) matches the inner alias (from.query.from.alias)
// 2. The subquery was found in queryMapping (it's a user-defined subquery, not optimizer-created)

Point 2 incorrectly states the condition. The code checks !isUserDefinedSubquery (NOT in queryMapping), not that it IS in queryMapping.

Suggestion: Rewrite to:

// 1. The subquery is NOT found in queryMapping (optimizer-created, not user-defined)
// 2. The outer alias matches the inner alias

2. Typo in Type Name

File: load-subset-subquery.test.ts:59

ChargersCollection should be ChargesCollection to match the Charge entity type.

3. Inconsistent Variable Naming

Files: index.ts:522 vs joins.ts:487

  • index.ts uses: subqueryFromAlias
  • joins.ts uses: fromInnerAlias

Suggestion: Use consistent naming (subqueryFromAlias is more descriptive).

4. Missing Comment Lines in joins.ts

File: joins.ts:484-485

The comment block in joins.ts is missing these lines that exist in index.ts:

// More importantly, pulling up for optimizer-created subqueries can cause issues
// when the optimizer has restructured the query.

Suggestions (5 found)

1. Extract Duplicated Logic into Shared Helper

The WHERE clause pull-up logic is duplicated between index.ts and joins.ts. Consider extracting to a shared function in types.ts:

export function pullUpSourceWhereClauses(
  outerAlias: string,
  innerQuery: QueryIR,
  queryMapping: QueryMapping,
  subqueryWhereClauses: Map<string, BasicExpression<boolean>>,
  targetWhereClauses: Map<string, BasicExpression<boolean>>,
): void { ... }

2. Add Test for Subquery in FROM Clause

The tests verify subqueries in JOIN clauses but not subqueries in FROM clauses. The processFrom changes in index.ts would benefit from a test exercising that path.

3. Add Test for Optimizer-Created Subquery Detection

No tests verify that the isOptimizerCreated detection correctly skips pull-up for optimizer-created subqueries.

4. Consider Logging on WHERE Clause Overwrites

The code silently overwrites if the same alias already exists in sourceWhereClauses. Consider logging or merging.

5. More Precise Test Assertions

Tests use expect(loadSubsetCalls.length).toBeGreaterThan(0) which is weak. Consider asserting exact expected call counts.


Test Coverage Assessment

Area Coverage
Direct query with WHERE ✅ Covered
Subquery in JOIN with WHERE ✅ Covered
Direct query with ORDER BY ✅ Covered
Subquery in JOIN with ORDER BY ✅ Covered
Subquery in FROM clause ⚠️ Not tested
Optimizer-created detection ⚠️ Not tested
Nested subqueries (3+ levels) ⚠️ Not tested

Strengths

  1. Correct Implementation: The core fix properly propagates WHERE clauses from subqueries to parent scope
  2. Good Optimizer Detection: The heuristic for detecting optimizer-created subqueries is sound
  3. Well-Structured Tests: Test file uses clear helper functions and realistic data models
  4. Thorough Comments: Detailed explanations help future maintainers understand the logic

Recommended Action

Ready for merge with minor fixes:

  1. Fix the comment logic error (both files)
  2. Fix the ChargersCollection typo
  3. Align variable naming between files
  4. Optional: Add test for subquery in FROM clause

🤖 Generated with Claude Code

Copy link
Collaborator

@samwillis samwillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

Query-driven sync doesn't work when collections are used in subqueries - loadSubsetOptions

4 participants