asc/desc index bug (#623)

* add repo of asc/desc index bug

* Fix bug with asc/desc indexes by tracking compare options in each index

* Fix and add unit tests for ordering options

* Modify BTreeIndex.take to properly handle null values

* Fix unit test

* Changeset

---------

Co-authored-by: Kevin De Porre <kevin@electric-sql.com>

Author: samwillis

.changeset/quick-tigers-talk.md

@@ -0,0 +1,5 @@
+---
+"@tanstack/db": patch
+---
+
+Fix bug where optimized queries would use the wrong index because the index is on the right column but was built using different comparison options (e.g. different direction, string sort, or null ordering).

packages/db/src/collection/index.ts

@@ -428,7 +428,10 @@ export class CollectionImpl<
    * // Create a ordered index with custom options
    * const ageIndex = collection.createIndex((row) => row.age, {
    *   indexType: BTreeIndex,
-   *   options: { compareFn: customComparator },
+   *   options: {
+   *     compareFn: customComparator,
+   *     compareOptions: { direction: 'asc', nulls: 'first', stringSort: 'lexical' }
+   *   },
    *   name: 'age_btree'
    * })
    *

packages/db/src/indexes/auto-index.ts

@@ -1,4 +1,6 @@
+import { DEFAULT_COMPARE_OPTIONS } from "../utils"
 import { BTreeIndex } from "./btree-index"
+import type { CompareOptions } from "../query/builder/types"
 import type { BasicExpression } from "../query/ir"
 import type { CollectionImpl } from "../collection/index.js"
 
@@ -30,15 +32,18 @@ export function ensureIndexForField<
   fieldName: string,
   fieldPath: Array<string>,
   collection: CollectionImpl<T, TKey, any, any, any>,
+  compareOptions: CompareOptions = DEFAULT_COMPARE_OPTIONS,
   compareFn?: (a: any, b: any) => number
 ) {
   if (!shouldAutoIndex(collection)) {
     return
   }
 
   // Check if we already have an index for this field
-  const existingIndex = Array.from(collection.indexes.values()).find((index) =>
-    index.matchesField(fieldPath)
+  const existingIndex = Array.from(collection.indexes.values()).find(
+    (index) =>
+      index.matchesField(fieldPath) &&
+      index.matchesCompareOptions(compareOptions)
   )
 
   if (existingIndex) {
@@ -50,7 +55,7 @@ export function ensureIndexForField<
     collection.createIndex((row) => (row as any)[fieldName], {
       name: `auto_${fieldName}`,
       indexType: BTreeIndex,
-      options: compareFn ? { compareFn } : {},
+      options: compareFn ? { compareFn, compareOptions } : {},
     })
   } catch (error) {
     console.warn(`Failed to create auto-index for field "${fieldName}":`, error)

packages/db/src/indexes/base-index.ts

@@ -1,5 +1,7 @@
 import { compileSingleRowExpression } from "../query/compiler/evaluators.js"
 import { comparisonFunctions } from "../query/builder/functions.js"
+import { DEFAULT_COMPARE_OPTIONS, deepEquals } from "../utils.js"
+import type { CompareOptions } from "../query/builder/types.js"
 import type { BasicExpression } from "../query/ir.js"
 
 /**
@@ -36,6 +38,7 @@ export abstract class BaseIndex<
   protected lookupCount = 0
   protected totalLookupTime = 0
   protected lastUpdated = new Date()
+  protected compareOptions: CompareOptions
 
   constructor(
     id: number,
@@ -45,6 +48,7 @@ export abstract class BaseIndex<
   ) {
     this.id = id
     this.expression = expression
+    this.compareOptions = DEFAULT_COMPARE_OPTIONS
     this.name = name
     this.initialize(options)
   }
@@ -76,6 +80,10 @@ export abstract class BaseIndex<
     )
   }
 
+  matchesCompareOptions(compareOptions: CompareOptions): boolean {
+    return deepEquals(this.compareOptions, compareOptions)
+  }
+
   getStats(): IndexStats {
     return {
       entryCount: this.keyCount,

packages/db/src/indexes/btree-index.ts

@@ -1,6 +1,7 @@
 import { BTree } from "../utils/btree.js"
 import { defaultComparator, normalizeValue } from "../utils/comparison.js"
 import { BaseIndex } from "./base-index.js"
+import type { CompareOptions } from "../query/builder/types.js"
 import type { BasicExpression } from "../query/ir.js"
 import type { IndexOperation } from "./base-index.js"
 
@@ -9,6 +10,7 @@ import type { IndexOperation } from "./base-index.js"
  */
 export interface BTreeIndexOptions {
   compareFn?: (a: any, b: any) => number
+  compareOptions?: CompareOptions
 }
 
 /**
@@ -53,6 +55,9 @@ export class BTreeIndex<
   ) {
     super(id, expression, name, options)
     this.compareFn = options?.compareFn ?? defaultComparator
+    if (options?.compareOptions) {
+      this.compareOptions = options!.compareOptions
+    }
     this.orderedEntries = new BTree(this.compareFn)
   }
 
@@ -248,10 +253,12 @@ export class BTreeIndex<
   take(n: number, from?: any, filterFn?: (key: TKey) => boolean): Array<TKey> {
     const keysInResult: Set<TKey> = new Set()
     const result: Array<TKey> = []
-    const nextKey = (k?: any) => this.orderedEntries.nextHigherKey(k)
+    const nextPair = (k?: any) => this.orderedEntries.nextHigherPair(k)
+    let pair: [any, any] | undefined
     let key = normalizeValue(from)
 
-    while ((key = nextKey(key)) && result.length < n) {
+    while ((pair = nextPair(key)) !== undefined && result.length < n) {
+      key = pair[0]
       const keys = this.valueMap.get(key)
       if (keys) {
         const it = keys.values()

packages/db/src/query/compiler/order-by.ts

@@ -132,6 +132,7 @@ export function processOrderBy(
           fieldName,
           followRefResult.path,
           followRefCollection,
+          clause.compareOptions,
           compare
         )
       }
@@ -152,7 +153,8 @@ export function processOrderBy(
 
       const index: BaseIndex<string | number> | undefined = findIndexForField(
         followRefCollection.indexes,
-        followRefResult.path
+        followRefResult.path,
+        clause.compareOptions
       )
 
       if (index && index.supports(`gt`)) {

packages/db/src/utils.ts

@@ -2,6 +2,8 @@
  * Generic utility functions
  */
 
+import type { CompareOptions } from "./query/builder/types"
+
 interface TypedArray {
   length: number
   [index: number]: number
@@ -209,3 +211,9 @@ export function isTemporal(a: any): boolean {
   const tag = getStringTag(a)
   return typeof tag === `string` && temporalTypes.includes(tag)
 }
+
+export const DEFAULT_COMPARE_OPTIONS: CompareOptions = {
+  direction: `asc`,
+  nulls: `first`,
+  stringSort: `locale`,
+}

packages/db/src/utils/index-optimization.ts

@@ -15,6 +15,8 @@
  * - Optimizes IN array expressions
  */
 
+import { DEFAULT_COMPARE_OPTIONS } from "../utils.js"
+import type { CompareOptions } from "../query/builder/types.js"
 import type { BaseIndex, IndexOperation } from "../indexes/base-index.js"
 import type { BasicExpression } from "../query/ir.js"
 
@@ -31,10 +33,14 @@ export interface OptimizationResult<TKey> {
  */
 export function findIndexForField<TKey extends string | number>(
   indexes: Map<number, BaseIndex<TKey>>,
-  fieldPath: Array<string>
+  fieldPath: Array<string>,
+  compareOptions: CompareOptions = DEFAULT_COMPARE_OPTIONS
 ): BaseIndex<TKey> | undefined {
   for (const index of indexes.values()) {
-    if (index.matchesField(fieldPath)) {
+    if (
+      index.matchesField(fieldPath) &&
+      index.matchesCompareOptions(compareOptions)
+    ) {
       return index
     }
   }