asc/desc index bug

[samwillis]

repo of #616

[changeset-bot]

🦋 Changeset detected

Latest commit: 5a26e2d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages

Name	Type
@tanstack/db	Patch
@tanstack/angular-db	Patch
@tanstack/electric-db-collection	Patch
@tanstack/query-db-collection	Patch
@tanstack/react-db	Patch
@tanstack/rxdb-db-collection	Patch
@tanstack/solid-db	Patch
@tanstack/svelte-db	Patch
@tanstack/trailbase-db-collection	Patch
@tanstack/vue-db	Patch
todos	Patch
@tanstack/db-example-react-todo	Patch

Not sure what this means? Click here to learn what changesets are.

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

[pkg-pr-new]

More templates

• todos
• tanstack-start-db-projects-example
• @tanstack/db-example-react-todo
• @tanstack/db-example-solid-todo

@tanstack/angular-db

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

@tanstack/db

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

@tanstack/db-ivm

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

@tanstack/electric-db-collection

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

@tanstack/query-db-collection

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

@tanstack/react-db

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

@tanstack/rxdb-db-collection

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

@tanstack/solid-db

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

@tanstack/svelte-db

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

@tanstack/trailbase-db-collection

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

@tanstack/vue-db

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

commit: 5a26e2d

[github-actions]

Size Change: +346 B (+0.47%)

Total Size: 74.5 kB

Filename	Size	Change
./packages/db/dist/esm/collection/index.js	3.18 kB	+43 B (+1.37%)
./packages/db/dist/esm/indexes/auto-index.js	806 B	+61 B (+8.19%)	🔍
./packages/db/dist/esm/indexes/base-index.js	673 B	+68 B (+11.24%)	⚠️
./packages/db/dist/esm/indexes/btree-index.js	1.85 kB	+33 B (+1.81%)
./packages/db/dist/esm/query/compiler/order-by.js	1.24 kB	+7 B (+0.57%)
./packages/db/dist/esm/utils.js	1.01 kB	+65 B (+6.89%)	🔍
./packages/db/dist/esm/utils/index-optimization.js	1.69 kB	+69 B (+4.26%)

ℹ️ View Unchanged

Filename	Size
./packages/db/dist/esm/collection/change-events.js	958 B
./packages/db/dist/esm/collection/changes.js	1.01 kB
./packages/db/dist/esm/collection/events.js	683 B
./packages/db/dist/esm/collection/indexes.js	1.16 kB
./packages/db/dist/esm/collection/lifecycle.js	1.8 kB
./packages/db/dist/esm/collection/mutations.js	2.59 kB
./packages/db/dist/esm/collection/state.js	3.81 kB
./packages/db/dist/esm/collection/subscription.js	1.69 kB
./packages/db/dist/esm/collection/sync.js	1.32 kB
./packages/db/dist/esm/deferred.js	230 B
./packages/db/dist/esm/errors.js	3.1 kB
./packages/db/dist/esm/index.js	1.56 kB
./packages/db/dist/esm/indexes/lazy-index.js	1.25 kB
./packages/db/dist/esm/local-only.js	827 B
./packages/db/dist/esm/local-storage.js	2.02 kB
./packages/db/dist/esm/optimistic-action.js	294 B
./packages/db/dist/esm/proxy.js	3.87 kB
./packages/db/dist/esm/query/builder/functions.js	615 B
./packages/db/dist/esm/query/builder/index.js	3.93 kB
./packages/db/dist/esm/query/builder/ref-proxy.js	938 B
./packages/db/dist/esm/query/compiler/evaluators.js	1.56 kB
./packages/db/dist/esm/query/compiler/expressions.js	631 B
./packages/db/dist/esm/query/compiler/group-by.js	2.11 kB
./packages/db/dist/esm/query/compiler/index.js	2.04 kB
./packages/db/dist/esm/query/compiler/joins.js	2.54 kB
./packages/db/dist/esm/query/compiler/select.js	1.28 kB
./packages/db/dist/esm/query/ir.js	785 B
./packages/db/dist/esm/query/live-query-collection.js	340 B
./packages/db/dist/esm/query/live/collection-config-builder.js	2.68 kB
./packages/db/dist/esm/query/live/collection-subscriber.js	1.91 kB
./packages/db/dist/esm/query/optimizer.js	3.1 kB
./packages/db/dist/esm/SortedMap.js	1.24 kB
./packages/db/dist/esm/transactions.js	3.03 kB
./packages/db/dist/esm/utils/browser-polyfills.js	365 B
./packages/db/dist/esm/utils/btree.js	6.02 kB
./packages/db/dist/esm/utils/comparison.js	754 B

_{compressed-size-action::db-package-size}

[github-actions]

Size Change: 0 B

Total Size: 1.44 kB

ℹ️ View Unchanged

Filename	Size
./packages/react-db/dist/esm/index.js	152 B
./packages/react-db/dist/esm/useLiveQuery.js	1.29 kB

_{compressed-size-action::react-db-package-size}

[kevin-dp]

The bug occurs because we store an index for a given column but we don't track the comparison options (direction, nulls, stringSort) that were used to construct the index. Hence, when we do a first query we may create an index on a given column (say in ascending order) and then later another query may order by the same column but in descending order but it will re-use the same index because the index lookup function only checks the column name and not the comparison options. I solved this bug by adding the comparison options to the index' state and making sure that ensureIndexForField and findIndexForField also check the comparison options.

I added some additional unit tests and modified the unit tests for stringSort and nulls such that they also break (because the wrong index is used). With the fix, these tests now pass. However, the tests showed that there is another bug when the indexed column contains null values and the query uses orderBy + limit + eager indexing mode. Under those circumstances the returned results become empty.

With the current fix, we would create 2 indexes: one for ASC order and one for DESC order. That is suboptimal. As a follow up we should re-use the existing index and walk it in reverse order. We can write a reverseIndex(index) function that takes an index and returns a new index that wraps the old one but traverses it in reverse.

[samwillis]

This is all good and ready to merge. I can't approve as I originally opened the PR with the tests... "I approve"