diff --git a/.changeset/stable-order-tiebreaker.md b/.changeset/stable-order-tiebreaker.md new file mode 100644 index 000000000..c80a3c7b6 --- /dev/null +++ b/.changeset/stable-order-tiebreaker.md @@ -0,0 +1,9 @@ +--- +'@tanstack/db-ivm': patch +--- + +Use row keys for stable tie-breaking in ORDER BY operations instead of hash-based object IDs. + +Previously, when multiple rows had equal ORDER BY values, tie-breaking used `globalObjectIdGenerator.getId(key)` which could produce hash collisions and wasn't stable across page reloads for object references. Now, the row key (which is always `string | number` and unique per row) is used directly for tie-breaking, ensuring deterministic and stable ordering. + +This also simplifies the internal `TaggedValue` type from a 3-tuple `[K, V, Tag]` to a 2-tuple `[K, V]`, removing unnecessary complexity. diff --git a/packages/db-ivm/src/index.ts b/packages/db-ivm/src/index.ts index 865b0a476..3ca1f26c6 100644 --- a/packages/db-ivm/src/index.ts +++ b/packages/db-ivm/src/index.ts @@ -2,3 +2,4 @@ export * from './d2.js' export * from './multiset.js' export * from './operators/index.js' export * from './types.js' +export { compareKeys } from './utils.js' diff --git a/packages/db-ivm/src/operators/topKWithFractionalIndex.ts b/packages/db-ivm/src/operators/topKWithFractionalIndex.ts index 4355800d1..3609703e5 100644 --- a/packages/db-ivm/src/operators/topKWithFractionalIndex.ts +++ b/packages/db-ivm/src/operators/topKWithFractionalIndex.ts @@ -2,11 +2,7 @@ import { generateKeyBetween } from 'fractional-indexing' import { DifferenceStreamWriter, UnaryOperator } from '../graph.js' import { StreamBuilder } from '../d2.js' import { MultiSet } from '../multiset.js' -import { - binarySearch, - diffHalfOpen, - globalObjectIdGenerator, -} from '../utils.js' +import { binarySearch, compareKeys, diffHalfOpen } from '../utils.js' import type { HRange } from '../utils.js' import type { DifferenceStreamReader } from '../graph.js' import type { IStreamBuilder, PipedOperator } from '../types.js' @@ -239,17 +235,18 @@ class TopKArray implements TopK { * This operator maintains fractional indices for sorted elements * and only updates indices when elements move position */ -export class TopKWithFractionalIndexOperator extends UnaryOperator< - [K, T], - [K, IndexedValue] -> { +export class TopKWithFractionalIndexOperator< + K extends string | number, + T, +> extends UnaryOperator<[K, T], [K, IndexedValue]> { #index: Map = new Map() // maps keys to their multiplicity /** * topK data structure that supports insertions and deletions * and returns changes to the topK. + * Elements are stored as [key, value] tuples for stable tie-breaking. */ - #topK: TopK> + #topK: TopK<[K, T]> constructor( id: number, @@ -261,21 +258,11 @@ export class TopKWithFractionalIndexOperator extends UnaryOperator< super(id, inputA, output) const limit = options.limit ?? Infinity const offset = options.offset ?? 0 - const compareTaggedValues = ( - a: TaggedValue, - b: TaggedValue, - ) => { - // First compare on the value - const valueComparison = comparator(getVal(a), getVal(b)) - if (valueComparison !== 0) { - return valueComparison - } - // If the values are equal, compare on the tag (object identity) - const tieBreakerA = getTag(a) - const tieBreakerB = getTag(b) - return tieBreakerA - tieBreakerB - } - this.#topK = this.createTopK(offset, limit, compareTaggedValues) + this.#topK = this.createTopK( + offset, + limit, + createKeyedComparator(comparator), + ) options.setSizeCallback?.(() => this.#topK.size) options.setWindowFn?.(this.moveTopK.bind(this)) } @@ -283,8 +270,8 @@ export class TopKWithFractionalIndexOperator extends UnaryOperator< protected createTopK( offset: number, limit: number, - comparator: (a: TaggedValue, b: TaggedValue) => number, - ): TopK> { + comparator: (a: [K, T], b: [K, T]) => number, + ): TopK<[K, T]> { return new TopKArray(offset, limit, comparator) } @@ -336,20 +323,18 @@ export class TopKWithFractionalIndexOperator extends UnaryOperator< ): void { const { oldMultiplicity, newMultiplicity } = this.addKey(key, multiplicity) - let res: TopKChanges> = { + let res: TopKChanges<[K, T]> = { moveIn: null, moveOut: null, } if (oldMultiplicity <= 0 && newMultiplicity > 0) { // The value was invisible but should now be visible // Need to insert it into the array of sorted values - const taggedValue = tagValue(key, value) - res = this.#topK.insert(taggedValue) + res = this.#topK.insert([key, value]) } else if (oldMultiplicity > 0 && newMultiplicity <= 0) { // The value was visible but should now be invisible // Need to remove it from the array of sorted values - const taggedValue = tagValue(key, value) - res = this.#topK.delete(taggedValue) + res = this.#topK.delete([key, value]) } else { // The value was invisible and it remains invisible // or it was visible and remains visible @@ -363,28 +348,22 @@ export class TopKWithFractionalIndexOperator extends UnaryOperator< } private handleMoveIn( - moveIn: IndexedValue> | null, + moveIn: IndexedValue<[K, T]> | null, result: Array<[[K, IndexedValue], number]>, ) { if (moveIn) { - const index = getIndex(moveIn) - const taggedValue = getValue(moveIn) - const k = getKey(taggedValue) - const val = getVal(taggedValue) - result.push([[k, [val, index]], 1]) + const [[key, value], index] = moveIn + result.push([[key, [value, index]], 1]) } } private handleMoveOut( - moveOut: IndexedValue> | null, + moveOut: IndexedValue<[K, T]> | null, result: Array<[[K, IndexedValue], number]>, ) { if (moveOut) { - const index = getIndex(moveOut) - const taggedValue = getValue(moveOut) - const k = getKey(taggedValue) - const val = getVal(taggedValue) - result.push([[k, [val, index]], -1]) + const [[key, value], index] = moveOut + result.push([[key, [value, index]], -1]) } } @@ -417,7 +396,7 @@ export class TopKWithFractionalIndexOperator extends UnaryOperator< * @param options - An optional object containing limit and offset properties * @returns A piped operator that orders the elements and limits the number of results */ -export function topKWithFractionalIndex( +export function topKWithFractionalIndex( comparator: (a: T, b: T) => number, options?: TopKWithFractionalIndexOptions, ): PipedOperator<[KType, T], [KType, IndexedValue]> { @@ -461,21 +440,21 @@ export function getIndex(indexedVal: IndexedValue): FractionalIndex { return indexedVal[1] } -export type Tag = number -export type TaggedValue = [K, V, Tag] - -function tagValue(key: K, value: V): TaggedValue { - return [key, value, globalObjectIdGenerator.getId(key)] -} - -function getKey(tieBreakerTaggedValue: TaggedValue): K { - return tieBreakerTaggedValue[0] -} - -function getVal(tieBreakerTaggedValue: TaggedValue): V { - return tieBreakerTaggedValue[1] -} - -function getTag(tieBreakerTaggedValue: TaggedValue): Tag { - return tieBreakerTaggedValue[2] +/** + * Creates a comparator for [key, value] tuples that first compares values, + * then uses the row key as a stable tie-breaker. + */ +function createKeyedComparator( + comparator: (a: T, b: T) => number, +): (a: [K, T], b: [K, T]) => number { + return ([aKey, aVal], [bKey, bVal]) => { + // First compare on the value + const valueComparison = comparator(aVal, bVal) + if (valueComparison !== 0) { + return valueComparison + } + // If the values are equal, use the row key as tie-breaker + // This provides stable, deterministic ordering since keys are string | number + return compareKeys(aKey, bKey) + } } diff --git a/packages/db-ivm/src/operators/topKWithFractionalIndexBTree.ts b/packages/db-ivm/src/operators/topKWithFractionalIndexBTree.ts index 173c707a5..8114325dc 100644 --- a/packages/db-ivm/src/operators/topKWithFractionalIndexBTree.ts +++ b/packages/db-ivm/src/operators/topKWithFractionalIndexBTree.ts @@ -10,7 +10,6 @@ import { import type { IStreamBuilder, PipedOperator } from '../types.js' import type { IndexedValue, - TaggedValue, TopK, TopKChanges, TopKWithFractionalIndexOptions, @@ -243,14 +242,14 @@ class TopKTree implements TopK { * and only updates indices when elements move position */ export class TopKWithFractionalIndexBTreeOperator< - K, + K extends string | number, T, > extends TopKWithFractionalIndexOperator { protected override createTopK( offset: number, limit: number, - comparator: (a: TaggedValue, b: TaggedValue) => number, - ): TopK> { + comparator: (a: [K, T], b: [K, T]) => number, + ): TopK<[K, T]> { if (BTree === undefined) { throw new Error( `B+ tree not loaded. You need to call loadBTree() before using TopKWithFractionalIndexBTreeOperator.`, @@ -275,7 +274,7 @@ export class TopKWithFractionalIndexBTreeOperator< * @param options - An optional object containing limit and offset properties * @returns A piped operator that orders the elements and limits the number of results */ -export function topKWithFractionalIndexBTree( +export function topKWithFractionalIndexBTree( comparator: (a: T, b: T) => number, options?: TopKWithFractionalIndexOptions, ): PipedOperator<[KType, T], [KType, IndexedValue]> { diff --git a/packages/db-ivm/src/utils.ts b/packages/db-ivm/src/utils.ts index e467fd2df..8f931c362 100644 --- a/packages/db-ivm/src/utils.ts +++ b/packages/db-ivm/src/utils.ts @@ -177,3 +177,18 @@ function range(start: number, end: number): Array { for (let i = start; i < end; i++) out.push(i) return out } + +/** + * Compares two keys (string | number) in a consistent, deterministic way. + * Handles mixed types by ordering strings before numbers. + */ +export function compareKeys(a: string | number, b: string | number): number { + // Same type: compare directly + if (typeof a === typeof b) { + if (a < b) return -1 + if (a > b) return 1 + return 0 + } + // Different types: strings come before numbers + return typeof a === `string` ? -1 : 1 +}