diff --git a/src/indexes/columnStore/ColumnSegmentCursor.ts b/src/indexes/columnStore/ColumnSegmentCursor.ts index 2990994e..550d0773 100644 --- a/src/indexes/columnStore/ColumnSegmentCursor.ts +++ b/src/indexes/columnStore/ColumnSegmentCursor.ts @@ -120,33 +120,71 @@ export class ColumnSegmentCursor { } /** - * Find the range of positions where values fall within [lo, hi]. - * - * @param lo - Lower bound (inclusive) - * @param hi - Upper bound (inclusive) - * @returns Start (inclusive) and end (exclusive) positions + * First index whose value is >= target (textbook lower_bound). + * O(log n) over the sorted column. */ - binarySearchRange(lo: number | string, hi: number | string): { startIdx: number, endIdx: number } { - const startResult = this.binarySearchValue(lo) - const startIdx = startResult.index - - // Find the first position AFTER hi + private lowerBound(target: number | string): number { const isString = this.header.valueType === ValueType.String - let endLo = startIdx - let endHi = this.values.length - while (endLo < endHi) { - const mid = (endLo + endHi) >>> 1 + let lo = 0 + let hi = this.values.length + while (lo < hi) { + const mid = (lo + hi) >>> 1 const cmp = isString - ? compareCodePoints(String(this.values[mid]), String(hi)) - : (this.values[mid] as number) - (hi as number) - if (cmp <= 0) { - endLo = mid + 1 - } else { - endHi = mid - } + ? compareCodePoints(String(this.values[mid]), String(target)) + : (this.values[mid] as number) - (target as number) + if (cmp < 0) lo = mid + 1 + else hi = mid } + return lo + } - return { startIdx, endIdx: endLo } + /** + * First index whose value is > target (textbook upper_bound). + * O(log n) over the sorted column. + */ + private upperBound(target: number | string): number { + const isString = this.header.valueType === ValueType.String + let lo = 0 + let hi = this.values.length + while (lo < hi) { + const mid = (lo + hi) >>> 1 + const cmp = isString + ? compareCodePoints(String(this.values[mid]), String(target)) + : (this.values[mid] as number) - (target as number) + if (cmp <= 0) lo = mid + 1 + else hi = mid + } + return lo + } + + /** + * Find the range of positions where values fall within the bounds. + * + * @param lo - Lower bound + * @param hi - Upper bound + * @param includeMin - Include values equal to `lo` (default: true) + * @param includeMax - Include values equal to `hi` (default: true) + * @returns Start (inclusive) and end (exclusive) positions. When the bounds + * collapse to empty (e.g. exclusive bounds on adjacent values, or lo > hi), + * startIdx >= endIdx and the caller's loop yields nothing. + * + * The default (inclusive/inclusive) path is byte-identical to the previous + * implementation: lowerBound(lo) equals the old binarySearchValue(lo).index, + * and upperBound(hi) equals the old "first position after hi" scan. + */ + binarySearchRange( + lo: number | string, + hi: number | string, + includeMin: boolean = true, + includeMax: boolean = true + ): { startIdx: number, endIdx: number } { + // startIdx: skip values strictly below the lower bound. Exclusive lower also + // skips values equal to `lo`, so it advances past the last == lo. + const startIdx = includeMin ? this.lowerBound(lo) : this.upperBound(lo) + // endIdx: first position to exclude. Inclusive upper keeps values == hi + // (stop after the last == hi); exclusive upper drops them (stop at first == hi). + const endIdx = includeMax ? this.upperBound(hi) : this.lowerBound(hi) + return { startIdx, endIdx } } /** @@ -171,12 +209,19 @@ export class ColumnSegmentCursor { /** * Get all non-tombstoned entity IDs in a value range. * - * @param lo - Lower bound (inclusive) - * @param hi - Upper bound (inclusive) + * @param lo - Lower bound + * @param hi - Upper bound + * @param includeMin - Include values equal to `lo` (default: true) + * @param includeMax - Include values equal to `hi` (default: true) * @returns Array of matching entity int IDs */ - getEntityIdsInRange(lo: number | string, hi: number | string): number[] { - const { startIdx, endIdx } = this.binarySearchRange(lo, hi) + getEntityIdsInRange( + lo: number | string, + hi: number | string, + includeMin: boolean = true, + includeMax: boolean = true + ): number[] { + const { startIdx, endIdx } = this.binarySearchRange(lo, hi, includeMin, includeMax) const result: number[] = [] for (let i = startIdx; i < endIdx; i++) { if (!this.tombstones.has(i)) { diff --git a/src/indexes/columnStore/ColumnStore.ts b/src/indexes/columnStore/ColumnStore.ts index 0ef955f7..6c8a7d8b 100644 --- a/src/indexes/columnStore/ColumnStore.ts +++ b/src/indexes/columnStore/ColumnStore.ts @@ -264,17 +264,42 @@ export class ColumnStore implements ColumnStoreProvider { } /** - * Range filter: find entities where field is within [min, max]. + * Range filter: find entities where field is within the bounds. + * + * @param field - Field name to filter on + * @param min - Lower bound (undefined/null = unbounded below) + * @param max - Upper bound (undefined/null = unbounded above) + * @param includeMin - Include values equal to `min` (default: true). Lets + * callers express strict `greaterThan` (exclusive lower) vs `gte`. + * @param includeMax - Include values equal to `max` (default: true). Lets + * callers express strict `lessThan` (exclusive upper) vs `lte`. */ - async rangeQuery(field: string, min?: unknown, max?: unknown): Promise { + async rangeQuery( + field: string, + min?: unknown, + max?: unknown, + includeMin: boolean = true, + includeMax: boolean = true + ): Promise { const result = new RoaringBitmap32() const cursors = await this.getSegmentCursors(field) + const hasMin = min !== undefined && min !== null + const hasMax = max !== undefined && max !== null + for (const cursor of cursors) { - const lo = (min !== undefined && min !== null) ? min as number | string : cursor.minValue - const hi = (max !== undefined && max !== null) ? max as number | string : cursor.maxValue + const lo = hasMin ? min as number | string : cursor.minValue + const hi = hasMax ? max as number | string : cursor.maxValue if (lo === undefined || hi === undefined) continue - const ids = cursor.getEntityIdsInRange(lo, hi) + // Exclusivity applies only to an explicitly provided bound. A bound taken + // from the segment's own min/max is a real stored value and must stay + // inclusive, or the segment's boundary entities would be wrongly dropped. + const ids = cursor.getEntityIdsInRange( + lo, + hi, + hasMin ? includeMin : true, + hasMax ? includeMax : true + ) for (const id of ids) result.add(id) } @@ -282,9 +307,10 @@ export class ColumnStore implements ColumnStoreProvider { const tailCursor = this.getTailBufferCursor(field) if (tailCursor) { for (const entry of tailCursor.iterateForward()) { - const v = entry.value - const inRange = (min == null || v >= min) && (max == null || v <= max) - if (inRange) result.add(entry.entityIntId) + const v = entry.value as any + const loOk = !hasMin || (includeMin ? v >= (min as any) : v > (min as any)) + const hiOk = !hasMax || (includeMax ? v <= (max as any) : v < (max as any)) + if (loOk && hiOk) result.add(entry.entityIntId) } } diff --git a/src/indexes/columnStore/types.ts b/src/indexes/columnStore/types.ts index 58e1c4d6..a9f79059 100644 --- a/src/indexes/columnStore/types.ts +++ b/src/indexes/columnStore/types.ts @@ -212,14 +212,16 @@ export interface ColumnStoreProvider { filter(field: string, value: unknown): Promise /** - * Range filter: find all entities where field is within [min, max]. + * Range filter: find all entities where field is within the bounds. * * @param field - Field name to filter on * @param min - Lower bound (undefined = no lower bound) * @param max - Upper bound (undefined = no upper bound) + * @param includeMin - Include values equal to `min` (default: true; false = strict `greaterThan`) + * @param includeMax - Include values equal to `max` (default: true; false = strict `lessThan`) * @returns Roaring bitmap of matching entity int IDs */ - rangeQuery(field: string, min?: unknown, max?: unknown): Promise + rangeQuery(field: string, min?: unknown, max?: unknown, includeMin?: boolean, includeMax?: boolean): Promise /** * Sort top-K: return the K entity int IDs with the highest or lowest diff --git a/src/utils/metadataIndex.ts b/src/utils/metadataIndex.ts index e66ad772..b246f8b5 100644 --- a/src/utils/metadataIndex.ts +++ b/src/utils/metadataIndex.ts @@ -1023,8 +1023,10 @@ export class MetadataIndexManager implements MetadataIndexProvider { // Column store path: O(log n) binary search on sorted column. // Use raw values (no normalization) — column store stores exact values. + // Thread includeMin/includeMax so strict lessThan/greaterThan stay strict + // (the column store is no longer inclusive-only). if (this.columnStore && this.columnStore.hasField(field)) { - const bitmap = await this.columnStore.rangeQuery(field, min, max) + const bitmap = await this.columnStore.rangeQuery(field, min, max, includeMin, includeMax) return this.idMapper.intsIterableToUuids(bitmap) } diff --git a/tests/unit/indexes/columnStore/column-store.test.ts b/tests/unit/indexes/columnStore/column-store.test.ts index b9404fa6..c08198be 100644 --- a/tests/unit/indexes/columnStore/column-store.test.ts +++ b/tests/unit/indexes/columnStore/column-store.test.ts @@ -162,6 +162,79 @@ describe('ColumnStore', () => { expect(bitmap.has(ids[2])).toBe(false) expect(bitmap.has(ids[8])).toBe(false) }) + + // Exclusive-bound coverage — regression guard for the column-store bug where + // includeMin/includeMax were dropped, so strict greaterThan/lessThan silently + // behaved as gte/lte whenever the column store served the query. + const seedPrices = async (): Promise => { + const ids: number[] = [] + for (let i = 0; i < 10; i++) { + ids.push(idMapper.getOrAssign(`x${i}`)) + store.addEntity(BigInt(ids[i]), { price: i * 10 }) // 0,10,...,90 + } + await store.flush() + return ids + } + + it('honors exclusive lower bound — greaterThan (30, 70]', async () => { + const ids = await seedPrices() + const bm = await store.rangeQuery('price', 30, 70, false, true) + // 40,50,60,70 → e4..e7 (30 excluded) + expect(bm.size).toBe(4) + expect(bm.has(ids[3])).toBe(false) + expect(bm.has(ids[4])).toBe(true) + expect(bm.has(ids[7])).toBe(true) + }) + + it('honors exclusive upper bound — lessThan [30, 70)', async () => { + const ids = await seedPrices() + const bm = await store.rangeQuery('price', 30, 70, true, false) + // 30,40,50,60 → e3..e6 (70 excluded) + expect(bm.size).toBe(4) + expect(bm.has(ids[7])).toBe(false) + expect(bm.has(ids[3])).toBe(true) + expect(bm.has(ids[6])).toBe(true) + }) + + it('honors both exclusive bounds — (30, 70)', async () => { + const ids = await seedPrices() + const bm = await store.rangeQuery('price', 30, 70, false, false) + // 40,50,60 → e4..e6 + expect(bm.size).toBe(3) + expect(bm.has(ids[3])).toBe(false) + expect(bm.has(ids[7])).toBe(false) + expect(bm.has(ids[4])).toBe(true) + expect(bm.has(ids[6])).toBe(true) + }) + + it('exclusive lower excludes ALL duplicates of the boundary value', async () => { + const a = idMapper.getOrAssign('dup_a'); store.addEntity(BigInt(a), { price: 30 }) + const b = idMapper.getOrAssign('dup_b'); store.addEntity(BigInt(b), { price: 30 }) + const c = idMapper.getOrAssign('dup_c'); store.addEntity(BigInt(c), { price: 30 }) + const d = idMapper.getOrAssign('dup_d'); store.addEntity(BigInt(d), { price: 40 }) + await store.flush() + const bm = await store.rangeQuery('price', 30, 100, false, true) // (30, 100] + expect(bm.has(a)).toBe(false) + expect(bm.has(b)).toBe(false) + expect(bm.has(c)).toBe(false) + expect(bm.has(d)).toBe(true) + }) + + it('honors exclusive bounds in the unflushed tail buffer', async () => { + // No flush → values live only in the tail buffer (separate code path from + // the persisted segment cursor). + const ids: number[] = [] + for (let i = 0; i < 5; i++) { + ids.push(idMapper.getOrAssign(`tail${i}`)) + store.addEntity(BigInt(ids[i]), { score: i * 10 }) // 0,10,20,30,40 + } + const bm = await store.rangeQuery('score', 0, 40, false, false) // (0, 40) + expect(bm.size).toBe(3) // 10,20,30 + expect(bm.has(ids[0])).toBe(false) + expect(bm.has(ids[4])).toBe(false) + expect(bm.has(ids[1])).toBe(true) + expect(bm.has(ids[3])).toBe(true) + }) }) // -----------------------------------------------------------------------