From 6439bb30935a39d8498d4313518c039eac6b84b3 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 13 Aug 2026 12:19:02 -0600 Subject: [PATCH 01/13] Use storage-level statistical range estimates in the query planner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Range comparators (starts_with/prefix, between and friends, lt/le/gt/ge) have always estimated as fixed fractions of the table size (5%/10%/30%), which makes condition ordering — and #2147 count=estimated totals — wildly wrong for any real range. When the store provides rocksdb-js estimateCount ({ count, confidence }), estimateCondition now estimates the actual range the search would iterate (mirroring searchByIndex range construction), blended with the old fraction heuristic by the estimate confidence, so low-confidence estimates (block-granular tiny ranges, open-ended complement subtraction) degrade gracefully to the previous behavior. RocksIndexStore translates value-space bounds to its composite [indexedValue, primaryKey] keys ([value, MAXIMUM_KEY]), mirroring its getRange. estimatedEntryCount switches from an exact full-store getKeysCount scan (every 10s per store) to the O(1) estimate-num-keys read. The capability is feature-detected (typeof store.estimateCount), so behavior is unchanged until the rocksdb-js dependency ships HarperFast/rocksdb-js#778; the end-to-end tests self-skip until then. --- resources/RocksIndexStore.ts | 16 ++ resources/search.ts | 91 +++++++- .../resources/estimateRangeCondition.test.js | 194 ++++++++++++++++++ 3 files changed, 295 insertions(+), 6 deletions(-) create mode 100644 unitTests/resources/estimateRangeCondition.test.js diff --git a/resources/RocksIndexStore.ts b/resources/RocksIndexStore.ts index c223f84090..4815476eed 100644 --- a/resources/RocksIndexStore.ts +++ b/resources/RocksIndexStore.ts @@ -66,6 +66,22 @@ export class RocksIndexStore extends RocksDatabase { } } +/** + * Index entries are composite [indexedValue, primaryKey] keys, so inclusive/exclusive bounds on + * indexed *values* translate to [value, MAXIMUM_KEY] composite bounds (mirroring getRange above) + * rather than the encoded-byte successor the base implementation would apply. Defined only when + * the installed rocksdb-js provides estimateCount, so `typeof store.estimateCount === 'function'` + * remains a capability check on older versions. + */ +if (typeof (RocksDatabase.prototype as any).estimateCount === 'function') { + (RocksIndexStore.prototype as any).estimateCount = function estimateCount(options?: any) { + let { start, end, exclusiveStart, inclusiveEnd } = options ?? {}; + if (exclusiveStart && start !== undefined) start = [start, MAXIMUM_KEY]; + if (inclusiveEnd && end !== undefined) end = [end, MAXIMUM_KEY]; + return (RocksDatabase.prototype as any).estimateCount.call(this, { start, end }); + }; +} + /** * Add `getValuesCount` to the DBI prototype which is used by the `RocksDatabase` and `Transaction` * classes. diff --git a/resources/search.ts b/resources/search.ts index c1f7dcafdf..044639fd03 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1112,6 +1112,75 @@ export function filterByType(searchCondition, Table, context, filtered, isPrimar } } +/** + * Estimates the entry count of a range comparator from the storage engine's statistical + * range estimate (rocksdb-js estimateCount), blended with the table-fraction heuristic by the + * estimate's confidence — a low-confidence estimate (tiny range at data-block granularity, + * open-ended complement subtraction) leans on the old heuristic, a high-confidence one replaces + * it. The range construction mirrors searchByIndex's comparator switch so the estimate covers + * the same keys the search would iterate. Returns undefined when the store cannot estimate + * ranges (LMDB, older rocksdb-js) or the comparator has no bounded range. + */ +function estimateRangeCondition(table, condition, searchType, fraction) { + const attributeName = condition[0] ?? condition.attribute; + const isPrimaryKey = attributeName === table.primaryKey; + const store = isPrimaryKey ? table.primaryStore : table.indices[attributeName]; + if (typeof store?.estimateCount !== 'function') return undefined; + let value = condition[1] ?? condition.value; + if (value instanceof Date) value = value.getTime(); + let range; + switch (searchType) { + case 'lt': + range = { end: value }; + break; + case 'le': + range = { end: value, inclusiveEnd: true }; + break; + case 'gt': + range = { start: value, exclusiveStart: true }; + break; + case 'ge': + range = { start: value }; + break; + case 'between': + case 'gele': + case 'gelt': + case 'gtlt': + case 'gtle': { + if (!Array.isArray(value)) return undefined; + let [start, end] = value; + if (start instanceof Date) start = start.getTime(); + if (end instanceof Date) end = end.getTime(); + range = { + start, + end, + inclusiveEnd: searchType === 'between' || searchType === 'gele' || searchType === 'gtle', + exclusiveStart: searchType === 'gtlt' || searchType === 'gtle', + }; + break; + } + case 'starts_with': { + const prefix = value?.toString(); + if (!prefix) return undefined; + range = { start: prefix, end: getStringPrefixUpperBound(prefix) }; + break; + } + case 'prefix': { + let start = Array.isArray(value) ? value : [value, null]; + if (start[start.length - 1] != null) start = start.concat(null); + const end = start.slice(0); + end[end.length - 1] = MAXIMUM_KEY; + range = { start, end }; + break; + } + default: + return undefined; + } + const { count, confidence } = store.estimateCount(range); + const heuristic = fraction * estimatedEntryCount(table.primaryStore) + 1; + return Math.max(1, Math.round(confidence * count + (1 - confidence) * heuristic)); +} + export function estimateCondition(table) { function estimateConditionForTable(condition) { if (condition.estimated_count === undefined) { @@ -1190,11 +1259,16 @@ export function estimateCondition(table) { } else if (Array.isArray(condition.value)) { condition.estimated_count = Infinity; } else condition.estimated_count = Infinity; - // for range queries (betweens, startsWith, greater, etc.), just arbitrarily guess + // for range queries, use the storage engine's statistical range estimate when + // available, falling back to an arbitrary fraction of the table } else if (searchType === 'starts_with' || searchType === 'prefix') - condition.estimated_count = STARTS_WITH_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; + condition.estimated_count = + estimateRangeCondition(table, condition, searchType, STARTS_WITH_ESTIMATE) ?? + STARTS_WITH_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; else if (searchType === 'between') - condition.estimated_count = BETWEEN_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; + condition.estimated_count = + estimateRangeCondition(table, condition, searchType, BETWEEN_ESTIMATE) ?? + BETWEEN_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; else if (searchType === 'sort') { const attribute_name = condition[0] ?? condition.attribute; const index = table.indices[attribute_name]; @@ -1209,7 +1283,10 @@ export function estimateCondition(table) { if (index?.customIndex?.estimateCount) // allow custom index to define its own estimation of counts condition.estimated_count = index.customIndex.estimateCount(condition.value); - else condition.estimated_count = OPEN_RANGE_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; + else + condition.estimated_count = + estimateRangeCondition(table, condition, searchType, OPEN_RANGE_ESTIMATE) ?? + OPEN_RANGE_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; } // we give a condition significantly more weight/preference if we will be ordering by it if (typeof condition.descending === 'boolean') condition.estimated_count /= 2; @@ -1622,8 +1699,10 @@ export function flattenKey(key) { function estimatedEntryCount(store) { const now = Date.now(); if ((store.estimatedEntryCountExpires || 0) < now) { - // use getStats for LMDB because it is fast path, otherwise RocksDB can handle fast path on its own - store.estimatedEntryCount = store instanceof RocksDatabase ? store.getKeysCount() : store.getStats().entryCount; + // getStats is the LMDB fast path; for RocksDB, estimate-num-keys is O(1) where an exact + // getKeysCount() would iterate the entire store + store.estimatedEntryCount = + store instanceof RocksDatabase ? store.getEstimatedKeyCount() : store.getStats().entryCount; store.estimatedEntryCountExpires = now + 10000; } return store.estimatedEntryCount; diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js new file mode 100644 index 0000000000..fdbf13d9af --- /dev/null +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -0,0 +1,194 @@ +require('../testUtils'); +const assert = require('node:assert'); +const { estimateCondition } = require('#src/resources/search'); +const { MAXIMUM_KEY } = require('ordered-binary'); + +// Range comparators historically estimated as fixed fractions of the table +// (0.05 starts_with / 0.1 between / 0.3 open range). When the store provides +// rocksdb-js estimateCount, the planner now uses the statistical range +// estimate, blended with the fraction heuristic by the estimate's confidence. +// These tests drive estimateCondition with synthetic stores so the dispatch, +// range construction, and blend are covered regardless of the installed +// rocksdb-js version (the capability is feature-detected). + +const ENTRY_COUNT = 1000; + +function makeStore(estimate) { + return { + calls: [], + getStats: () => ({ entryCount: ENTRY_COUNT }), + estimateCount(range) { + this.calls.push(range); + return estimate; + }, + }; +} + +function makeTable({ indexEstimate, primaryEstimate } = {}) { + const primaryStore = { + getStats: () => ({ entryCount: ENTRY_COUNT }), + }; + if (primaryEstimate) { + primaryStore.calls = []; + primaryStore.estimateCount = function (range) { + this.calls.push(range); + return primaryEstimate; + }; + } + const index = + indexEstimate === undefined ? { getStats: () => ({ entryCount: ENTRY_COUNT }) } : makeStore(indexEstimate); + return { + primaryKey: 'id', + primaryStore, + indices: { attr: index }, + attributes: [], + }; +} + +function estimate(table, condition) { + return estimateCondition(table)(condition); +} + +describe('estimateCondition range estimates', () => { + it('uses the statistical estimate outright at confidence 1', () => { + const table = makeTable({ indexEstimate: { count: 120, confidence: 1 } }); + const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); + assert.strictEqual(estimated, 120); + assert.deepStrictEqual(table.indices.attr.calls[0], { + start: 5, + end: 10, + inclusiveEnd: true, + exclusiveStart: false, + }); + }); + + it('falls back to the fraction heuristic at confidence 0', () => { + const table = makeTable({ indexEstimate: { count: 120, confidence: 0 } }); + const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); + assert.strictEqual(estimated, 0.1 * ENTRY_COUNT + 1); + }); + + it('blends estimate and heuristic by confidence', () => { + const table = makeTable({ indexEstimate: { count: 120, confidence: 0.5 } }); + const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); + assert.strictEqual(estimated, Math.round(0.5 * 120 + 0.5 * (0.1 * ENTRY_COUNT + 1))); + }); + + it('constructs a prefix upper bound for starts_with', () => { + const table = makeTable({ indexEstimate: { count: 7, confidence: 1 } }); + const estimated = estimate(table, { attribute: 'attr', comparator: 'starts_with', value: 'ab' }); + assert.strictEqual(estimated, 7); + const range = table.indices.attr.calls[0]; + assert.strictEqual(range.start, 'ab'); + assert.ok(range.end instanceof Uint8Array); + }); + + it('constructs composite bounds for prefix', () => { + const table = makeTable({ indexEstimate: { count: 9, confidence: 1 } }); + const estimated = estimate(table, { attribute: 'attr', comparator: 'prefix', value: 'a' }); + assert.strictEqual(estimated, 9); + assert.deepStrictEqual(table.indices.attr.calls[0], { start: ['a', null], end: ['a', MAXIMUM_KEY] }); + }); + + it('constructs open ranges for gt/lt', () => { + const table = makeTable({ indexEstimate: { count: 40, confidence: 1 } }); + assert.strictEqual(estimate(table, { attribute: 'attr', comparator: 'gt', value: 5 }), 40); + assert.deepStrictEqual(table.indices.attr.calls[0], { start: 5, exclusiveStart: true }); + + const table2 = makeTable({ indexEstimate: { count: 40, confidence: 1 } }); + assert.strictEqual(estimate(table2, { attribute: 'attr', comparator: 'lt', value: 5 }), 40); + assert.deepStrictEqual(table2.indices.attr.calls[0], { end: 5 }); + }); + + it('estimates primary-key ranges against the primary store', () => { + const table = makeTable({ primaryEstimate: { count: 33, confidence: 1 } }); + const estimated = estimate(table, { attribute: 'id', comparator: 'ge', value: 100 }); + assert.strictEqual(estimated, 33); + assert.deepStrictEqual(table.primaryStore.calls[0], { start: 100 }); + }); + + it('keeps the fraction heuristics when the store cannot estimate', () => { + const table = makeTable(); + assert.strictEqual( + estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }), + 0.1 * ENTRY_COUNT + 1 + ); + assert.strictEqual( + estimate(table, { attribute: 'attr', comparator: 'starts_with', value: 'ab' }), + 0.05 * ENTRY_COUNT + 1 + ); + assert.strictEqual(estimate(table, { attribute: 'attr', comparator: 'gt', value: 5 }), 0.3 * ENTRY_COUNT + 1); + }); + + it('never estimates below 1', () => { + const table = makeTable({ indexEstimate: { count: 0, confidence: 1 } }); + const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); + assert.strictEqual(estimated, 1); + }); +}); + +// End-to-end against real stores; requires a rocksdb-js with estimateCount +// (feature-detected — skipped until the dependency ships it). +const { RocksDatabase } = require('@harperfast/rocksdb-js'); +const supportsEstimateCount = typeof RocksDatabase.prototype.estimateCount === 'function'; + +(supportsEstimateCount ? describe : describe.skip)('estimateCondition range estimates (real stores)', () => { + const { setupTestDBPath } = require('../testUtils'); + const { table } = require('#src/resources/databases'); + const { setMainIsWorker } = require('#js/server/threads/manageThreads'); + const N = 20000; + let T; + + before(async function () { + this.timeout(120000); + setupTestDBPath(); + setMainIsWorker(true); + T = table({ + table: 'EstimateTest', + database: 'test', + attributes: [ + { name: 'id', isPrimaryKey: true }, + { name: 'score', type: 'Int', indexed: true }, + { name: 'name', indexed: true }, + ], + }); + let last; + for (let i = 0; i < N; i++) { + last = T.put({ id: i, score: i, name: `name-${String(i).padStart(6, '0')}` }); + } + await last; + await T.primaryStore.flush(); + await T.indices.score.flush(); + await T.indices.name.flush(); + }); + + it('scales between estimates with the real range width', () => { + const est = estimateCondition(T); + const narrow = est({ attribute: 'score', comparator: 'between', value: [1000, 1100] }); + const wide = est({ attribute: 'score', comparator: 'between', value: [1000, 11000] }); + assert.ok(narrow < wide, `narrow (${narrow}) should be < wide (${wide})`); + // the wide range is half the table; the old heuristic would report + // 0.1 * N + 1 for both + assert.ok(wide > 0.15 * N, `wide (${wide}) should exceed the flat between heuristic`); + }); + + it('estimates starts_with from the real prefix range', () => { + const est = estimateCondition(T); + // all names share the "name-0" prefix up to 9999 + const broad = est({ attribute: 'name', comparator: 'starts_with', value: 'name-0' }); + const narrow = est({ attribute: 'name', comparator: 'starts_with', value: 'name-000' }); + assert.ok(narrow < broad, `narrow (${narrow}) should be < broad (${broad})`); + }); + + it('orders open-range estimates by real range width', () => { + // The flat heuristic reported 30% + 1 for every open range; the + // statistical estimate must at least order them. (Absolute accuracy at + // this scale is block-granular — index entries are tiny, so a 20k-row + // index spans few data blocks.) + const est = estimateCondition(T); + const tail10 = est({ attribute: 'score', comparator: 'gt', value: N - 2000 }); + const tail50 = est({ attribute: 'score', comparator: 'gt', value: N / 2 }); + assert.ok(tail10 < tail50, `10% tail (${tail10}) should be < 50% tail (${tail50})`); + assert.ok(tail10 <= 0.3 * N + 1, `10% tail (${tail10}) should not exceed the flat heuristic`); + }); +}); From bdc1e37482842979ac3c09dc81e1b56396a920d4 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 13 Aug 2026 12:36:45 -0600 Subject: [PATCH 02/13] Address cross-model review: negation inversion, dependency-shape guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - negated conditions now estimate the complement of their positive estimate (root fix in estimateConditionForTable — also covers the pre-existing negated-equals defect): a narrow negated range previously looked highly selective, won the condition ordering, and executed as a full scan - the estimate path validates {count, confidence} shape and wraps the native call in try/catch, so a future dependency bump (or a concurrently closing store) degrades to the fraction heuristic instead of NaN-poisoning plan ordering or failing the request - over-length string bounds fall back (execution truncates at MAX_SEARCH_KEY_LENGTH + filters, so the executed range is wider than the estimable one) - intersectionEstimate divisor floored at 1 - comment trims per review --- resources/RocksIndexStore.ts | 10 +-- resources/search.ts | 39 ++++++++--- .../resources/estimateRangeCondition.test.js | 70 ++++++++++++++----- 3 files changed, 84 insertions(+), 35 deletions(-) diff --git a/resources/RocksIndexStore.ts b/resources/RocksIndexStore.ts index 4815476eed..3ac458b7af 100644 --- a/resources/RocksIndexStore.ts +++ b/resources/RocksIndexStore.ts @@ -66,13 +66,9 @@ export class RocksIndexStore extends RocksDatabase { } } -/** - * Index entries are composite [indexedValue, primaryKey] keys, so inclusive/exclusive bounds on - * indexed *values* translate to [value, MAXIMUM_KEY] composite bounds (mirroring getRange above) - * rather than the encoded-byte successor the base implementation would apply. Defined only when - * the installed rocksdb-js provides estimateCount, so `typeof store.estimateCount === 'function'` - * remains a capability check on older versions. - */ +// Bounds on indexed values must widen to [value, MAXIMUM_KEY] composite bounds (as in getRange), +// not the base implementation's encoded-byte successor. Assigned conditionally so +// `typeof store.estimateCount === 'function'` stays a capability probe on older rocksdb-js. if (typeof (RocksDatabase.prototype as any).estimateCount === 'function') { (RocksIndexStore.prototype as any).estimateCount = function estimateCount(options?: any) { let { start, end, exclusiveStart, inclusiveEnd } = options ?? {}; diff --git a/resources/search.ts b/resources/search.ts index 044639fd03..0128b4cd44 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1113,13 +1113,10 @@ export function filterByType(searchCondition, Table, context, filtered, isPrimar } /** - * Estimates the entry count of a range comparator from the storage engine's statistical - * range estimate (rocksdb-js estimateCount), blended with the table-fraction heuristic by the - * estimate's confidence — a low-confidence estimate (tiny range at data-block granularity, - * open-ended complement subtraction) leans on the old heuristic, a high-confidence one replaces - * it. The range construction mirrors searchByIndex's comparator switch so the estimate covers - * the same keys the search would iterate. Returns undefined when the store cannot estimate - * ranges (LMDB, older rocksdb-js) or the comparator has no bounded range. + * Estimates a range comparator's entry count from the store's statistical estimate, blended with + * the table-fraction heuristic by the estimate's confidence so low-confidence estimates degrade + * to the old behavior. Returns undefined (caller falls back to the heuristic) when the store + * cannot estimate, the shape is unexpected, or the executed range wouldn't match this one. */ function estimateRangeCondition(table, condition, searchType, fraction) { const attributeName = condition[0] ?? condition.attribute; @@ -1176,7 +1173,23 @@ function estimateRangeCondition(table, condition, searchType, fraction) { default: return undefined; } - const { count, confidence } = store.estimateCount(range); + // Long string bounds get truncated + filtered at execution (searchByIndex), so the + // executed range is wider than this one; don't estimate what won't be iterated. + if ( + (typeof range.start === 'string' && range.start.length > MAX_SEARCH_KEY_LENGTH) || + (typeof range.end === 'string' && range.end.length > MAX_SEARCH_KEY_LENGTH) + ) { + return undefined; + } + let count, confidence; + try { + ({ count, confidence } = store.estimateCount(range) ?? {}); + } catch { + // a concurrently closing/dropped store must degrade the plan, not fail the query + return undefined; + } + // The dependency pin can activate this path on an image rebuild; never trust the shape blind. + if (!Number.isFinite(count) || !(confidence >= 0 && confidence <= 1)) return undefined; const heuristic = fraction * estimatedEntryCount(table.primaryStore) + 1; return Math.max(1, Math.round(confidence * count + (1 - confidence) * heuristic)); } @@ -1259,8 +1272,6 @@ export function estimateCondition(table) { } else if (Array.isArray(condition.value)) { condition.estimated_count = Infinity; } else condition.estimated_count = Infinity; - // for range queries, use the storage engine's statistical range estimate when - // available, falling back to an arbitrary fraction of the table } else if (searchType === 'starts_with' || searchType === 'prefix') condition.estimated_count = estimateRangeCondition(table, condition, searchType, STARTS_WITH_ESTIMATE) ?? @@ -1288,6 +1299,12 @@ export function estimateCondition(table) { estimateRangeCondition(table, condition, searchType, OPEN_RANGE_ESTIMATE) ?? OPEN_RANGE_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; } + // a negated condition matches the complement of its positive estimate — without this + // inversion a narrow negated range (or negated equals) looks highly selective, wins + // the condition ordering, and then executes as a full scan + if (condition.negated && isFinite(condition.estimated_count)) { + condition.estimated_count = Math.max(estimatedEntryCount(table.primaryStore) - condition.estimated_count, 1); + } // we give a condition significantly more weight/preference if we will be ordering by it if (typeof condition.descending === 'boolean') condition.estimated_count /= 2; } @@ -1709,5 +1726,5 @@ function estimatedEntryCount(store) { } export function intersectionEstimate(store, left, right) { - return (left * right) / estimatedEntryCount(store); + return (left * right) / Math.max(estimatedEntryCount(store), 1); } diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index fdbf13d9af..f5dc69a03d 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -3,14 +3,6 @@ const assert = require('node:assert'); const { estimateCondition } = require('#src/resources/search'); const { MAXIMUM_KEY } = require('ordered-binary'); -// Range comparators historically estimated as fixed fractions of the table -// (0.05 starts_with / 0.1 between / 0.3 open range). When the store provides -// rocksdb-js estimateCount, the planner now uses the statistical range -// estimate, blended with the fraction heuristic by the estimate's confidence. -// These tests drive estimateCondition with synthetic stores so the dispatch, -// range construction, and blend are covered regardless of the installed -// rocksdb-js version (the capability is feature-detected). - const ENTRY_COUNT = 1000; function makeStore(estimate) { @@ -125,10 +117,59 @@ describe('estimateCondition range estimates', () => { const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); assert.strictEqual(estimated, 1); }); + + it('inverts negated conditions to the complement of the positive estimate', () => { + // a narrow negated range must not look highly selective — it executes as a full scan + const table = makeTable({ indexEstimate: { count: 10, confidence: 1 } }); + const estimated = estimate(table, { + attribute: 'attr', + comparator: 'between', + value: [5, 10], + negated: true, + }); + assert.strictEqual(estimated, ENTRY_COUNT - 10); + + // pre-existing defect: negated equals also estimated its positive count + const eqTable = makeTable(); + eqTable.indices.attr.getValuesCount = () => 3; + const negatedEquals = estimate(eqTable, { + attribute: 'attr', + comparator: 'equals', + value: 'x', + negated: true, + }); + assert.strictEqual(negatedEquals, ENTRY_COUNT - 3); + }); + + it('falls back when the estimate shape is unexpected', () => { + for (const bad of [42, { count: NaN, confidence: 1 }, { count: 10 }, { count: 10, confidence: 2 }, null]) { + const table = makeTable({ indexEstimate: bad }); + const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); + assert.strictEqual(estimated, 0.1 * ENTRY_COUNT + 1, `shape ${JSON.stringify(bad)} must fall back`); + } + }); + + it('falls back when estimateCount throws', () => { + const table = makeTable({ indexEstimate: { count: 1, confidence: 1 } }); + table.indices.attr.estimateCount = () => { + throw new Error('store closed'); + }; + const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); + assert.strictEqual(estimated, 0.1 * ENTRY_COUNT + 1); + }); + + it('falls back for over-length string bounds (executed range is truncated + filtered)', () => { + const table = makeTable({ indexEstimate: { count: 1, confidence: 1 } }); + const estimated = estimate(table, { + attribute: 'attr', + comparator: 'starts_with', + value: 'x'.repeat(2000), + }); + assert.strictEqual(estimated, 0.05 * ENTRY_COUNT + 1); + assert.strictEqual(table.indices.attr.calls.length, 0); + }); }); -// End-to-end against real stores; requires a rocksdb-js with estimateCount -// (feature-detected — skipped until the dependency ships it). const { RocksDatabase } = require('@harperfast/rocksdb-js'); const supportsEstimateCount = typeof RocksDatabase.prototype.estimateCount === 'function'; @@ -167,24 +208,19 @@ const supportsEstimateCount = typeof RocksDatabase.prototype.estimateCount === ' const narrow = est({ attribute: 'score', comparator: 'between', value: [1000, 1100] }); const wide = est({ attribute: 'score', comparator: 'between', value: [1000, 11000] }); assert.ok(narrow < wide, `narrow (${narrow}) should be < wide (${wide})`); - // the wide range is half the table; the old heuristic would report - // 0.1 * N + 1 for both assert.ok(wide > 0.15 * N, `wide (${wide}) should exceed the flat between heuristic`); }); it('estimates starts_with from the real prefix range', () => { const est = estimateCondition(T); - // all names share the "name-0" prefix up to 9999 const broad = est({ attribute: 'name', comparator: 'starts_with', value: 'name-0' }); const narrow = est({ attribute: 'name', comparator: 'starts_with', value: 'name-000' }); assert.ok(narrow < broad, `narrow (${narrow}) should be < broad (${broad})`); }); it('orders open-range estimates by real range width', () => { - // The flat heuristic reported 30% + 1 for every open range; the - // statistical estimate must at least order them. (Absolute accuracy at - // this scale is block-granular — index entries are tiny, so a 20k-row - // index spans few data blocks.) + // absolute accuracy at this scale is block-granular (tiny index entries, few data + // blocks), so assert ordering, which is what condition planning consumes const est = estimateCondition(T); const tail10 = est({ attribute: 'score', comparator: 'gt', value: N - 2000 }); const tail50 = est({ attribute: 'score', comparator: 'gt', value: N / 2 }); From 415c42144f81b636a42fa67870f0967d10e7bb77 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 13 Aug 2026 12:37:44 -0600 Subject: [PATCH 03/13] Document query-plan range estimation invariants in DESIGN.md --- DESIGN.md | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index ecc83e5cc0..98c660c85c 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1213,7 +1213,6 @@ An `{}` in the config system is context-dependent, and conflating the contexts i Removal therefore prunes: `deleteNestedValue` removes ancestors the deletion emptied, only when it actually deleted an existing leaf, and reports what it pruned. The overlap case — a file-declared empty scope an env layer temporarily populated — is tracked in the state file's `emptyScopeOriginals` (separate from `originalValues` so a marker can never mask or be consumed as a real leaf original at the same path; older state files lacking the field are defaulted). Restore consumes a marker only for a path the prune actually removed, so a scalar overwrite or an absent-leaf no-op can never resurrect a scope over live env-layer content. Note there are two coexisting mechanisms for "file `{}` is user content": `restoreBaseEmptyObjects` on the stateless compose path and the marker pair on the stateful removal path — if you touch one, check the other. Two durable limitations of the marker mechanism, both with user config-file content as the blast radius: markers can only be recorded at populate time, so a scope an env layer populated _before_ `emptyScopeOriginals` existed (any pre-upgrade boot) has no marker and prunes away on its first post-upgrade vacate; and a corrupt config-state file resets to fresh state — dropping `originalValues` and `emptyScopeOriginals` for every tracked path — after which the next removal prunes those scopes for good; `saveConfigState` writes via temp+rename precisely so a torn write cannot be the trigger, leaving genuine corruption (disk faults, hand edits) as the remaining path. - ## Every path handed to a native file watch must be canonicalized (`utility/watchPath.ts`) libuv's Windows fs-event callback rebuilds each event's absolute path, expands it with @@ -1248,7 +1247,6 @@ absolute paths built from `cwd`, so its bases must be derived from the same spel paths are relative to `cwd` and reads stay on the configured `component.directory`. And a watcher that degrades to polling stays there for its lifetime, so a caller with no polling story of its own (`resources/blob.ts`) needs one — there it polls `readMore` on the existing no-progress deadline. - ## No descriptor on the root config may outlive a turn (`config/configUtils.ts`, `config/RootConfigWatcher.ts`, `components/OptionsWatcher.ts`) `atomicWriteFile` replaces `harper-config.yaml` by rename-over and retries `EPERM`/`EACCES` with a @@ -1272,3 +1270,28 @@ half-written file into a valid-looking env-only config. Its three outcomes are d one matters: a usable read withdraws the file's give-up report and restores the budget; giving up restores the budget (the write that repairs the file can itself be read mid-write) but leaves the report standing, since it is shared with every other watcher of that file; closing is terminal. + +## Query-plan range estimation blends statistical estimates by confidence (`search.ts`) + +`estimateCondition` estimates range comparators (`starts_with`/`prefix`, the `between` family, +`lt`/`le`/`gt`/`ge`) via the store's `estimateCount({start, end, …}) → { count, confidence }` +(rocksdb-js ≥ #778) instead of flat table fractions, blended as +`round(confidence × count + (1 − confidence) × fraction-heuristic)` so a low-confidence estimate +degrades to the historical behavior rather than replacing it. Invariants that are easy to break: + +- **Capability is feature-detected per store** (`typeof store.estimateCount === 'function'`), and + the `RocksIndexStore` override is assigned conditionally for the same reason — do not define it + unconditionally or the probe lies on older rocksdb-js. The result shape is validated + (`Number.isFinite(count)`, `0 ≤ confidence ≤ 1`) and the native call is try/caught because a + caret bump can activate this path on an image rebuild without a code change. +- **The estimated range must be the executed range.** Construction mirrors `searchByIndex`'s + comparator switch; bounds longer than `MAX_SEARCH_KEY_LENGTH` fall back entirely because + execution truncates + filters (wider range than the estimable one). `RocksIndexStore` widens + value-space bounds to `[value, MAXIMUM_KEY]` composite bounds — the base implementation's + byte-successor semantics would exclude the wrong entries on composite `[value, primaryKey]` keys. +- **Negated conditions invert at the root** (`estimateConditionForTable`): the estimate of the + positive range becomes `entryCount − estimate`. Without this a narrow negated range looks highly + selective, wins condition ordering, and executes as a full scan. +- `estimatedEntryCount` reads `estimate-num-keys` (O(1)) rather than iterating; it skews high on + overwrite/delete-heavy data until compaction, which is acceptable for the relative-ordering and + explicitly-estimated consumers it feeds (and it is a divisor — keep the ≥1 floor). From 062956642b5534ee547a26f880cbf23afab0844b Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 13 Aug 2026 12:40:12 -0600 Subject: [PATCH 04/13] Negated conditions estimate Infinity, matching the full-scan convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The complement inversion still let a wide positive range make its negation look cheap while executing as a full scan. estimated_count feeds driving-condition ordering, and the codebase already encodes full-scan cost as Infinity for the filter-only comparators (contains/ends_with) — negated conditions (which always force needFullScan) now follow the same convention. --- DESIGN.md | 7 ++++--- resources/search.ts | 11 +++++------ unitTests/resources/estimateRangeCondition.test.js | 11 ++++++----- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 98c660c85c..0455da38ab 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1289,9 +1289,10 @@ degrades to the historical behavior rather than replacing it. Invariants that ar execution truncates + filters (wider range than the estimable one). `RocksIndexStore` widens value-space bounds to `[value, MAXIMUM_KEY]` composite bounds — the base implementation's byte-successor semantics would exclude the wrong entries on composite `[value, primaryKey]` keys. -- **Negated conditions invert at the root** (`estimateConditionForTable`): the estimate of the - positive range becomes `entryCount − estimate`. Without this a narrow negated range looks highly - selective, wins condition ordering, and executes as a full scan. +- **Negated conditions estimate `Infinity` at the root** (`estimateConditionForTable`), following + the filter-only convention (`contains`/`ends_with`): the negated flag always forces + `needFullScan`, so `estimated_count` here is execution-cost ordering, not result cardinality — + a narrow negated range must never look selective enough to become the driving condition. - `estimatedEntryCount` reads `estimate-num-keys` (O(1)) rather than iterating; it skews high on overwrite/delete-heavy data until compaction, which is acceptable for the relative-ordering and explicitly-estimated consumers it feeds (and it is a divisor — keep the ≥1 floor). diff --git a/resources/search.ts b/resources/search.ts index 0128b4cd44..fbe32aede3 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1299,12 +1299,11 @@ export function estimateCondition(table) { estimateRangeCondition(table, condition, searchType, OPEN_RANGE_ESTIMATE) ?? OPEN_RANGE_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; } - // a negated condition matches the complement of its positive estimate — without this - // inversion a narrow negated range (or negated equals) looks highly selective, wins - // the condition ordering, and then executes as a full scan - if (condition.negated && isFinite(condition.estimated_count)) { - condition.estimated_count = Math.max(estimatedEntryCount(table.primaryStore) - condition.estimated_count, 1); - } + // a negated condition always executes as a full scan (searchByIndex forces + // needFullScan), so follow the filter-only convention used by contains/ends_with: + // estimate Infinity so its positive-range estimate can never win the + // driving-condition ordering + if (condition.negated) condition.estimated_count = Infinity; // we give a condition significantly more weight/preference if we will be ordering by it if (typeof condition.descending === 'boolean') condition.estimated_count /= 2; } diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index f5dc69a03d..2c78ab5a33 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -118,8 +118,9 @@ describe('estimateCondition range estimates', () => { assert.strictEqual(estimated, 1); }); - it('inverts negated conditions to the complement of the positive estimate', () => { - // a narrow negated range must not look highly selective — it executes as a full scan + it('estimates negated conditions as Infinity (they always full-scan)', () => { + // a narrow negated range must not look highly selective — the full-scan + // convention (contains/ends_with) keeps it out of the driving-condition slot const table = makeTable({ indexEstimate: { count: 10, confidence: 1 } }); const estimated = estimate(table, { attribute: 'attr', @@ -127,9 +128,9 @@ describe('estimateCondition range estimates', () => { value: [5, 10], negated: true, }); - assert.strictEqual(estimated, ENTRY_COUNT - 10); + assert.strictEqual(estimated, Infinity); - // pre-existing defect: negated equals also estimated its positive count + // pre-existing defect: negated equals estimated its (possibly tiny) positive count const eqTable = makeTable(); eqTable.indices.attr.getValuesCount = () => 3; const negatedEquals = estimate(eqTable, { @@ -138,7 +139,7 @@ describe('estimateCondition range estimates', () => { value: 'x', negated: true, }); - assert.strictEqual(negatedEquals, ENTRY_COUNT - 3); + assert.strictEqual(negatedEquals, Infinity); }); it('falls back when the estimate shape is unexpected', () => { From 4cc850f734a89ef49b42940c5e7cedd347b7244e Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 25 Aug 2026 16:09:34 -0600 Subject: [PATCH 05/13] Bump rocksdb-js to 2.8.0 and use the shipped estimateCount API rocksdb-js 2.8.0 ships the statistical range estimation the query planner was written against, so the RocksIndexStore override becomes a real typed method mirroring getRange (reverse flip included) instead of a conditional prototype assignment, and the real-store estimate tests run unconditionally. The per-store capability probe and result-shape validation stay: LMDB-backed and custom index stores don't implement estimateCount. Co-Authored-By: Claude Opus 5 --- DESIGN.md | 19 +++-- package-lock.json | 83 ++++++++++--------- package.json | 2 +- resources/RocksIndexStore.ts | 30 ++++--- resources/search.ts | 3 +- .../resources/estimateRangeCondition.test.js | 5 +- 6 files changed, 78 insertions(+), 64 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 0455da38ab..aea339e0b2 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1275,20 +1275,21 @@ report standing, since it is shared with every other watcher of that file; closi `estimateCondition` estimates range comparators (`starts_with`/`prefix`, the `between` family, `lt`/`le`/`gt`/`ge`) via the store's `estimateCount({start, end, …}) → { count, confidence }` -(rocksdb-js ≥ #778) instead of flat table fractions, blended as +(rocksdb-js ≥ 2.8.0) instead of flat table fractions, blended as `round(confidence × count + (1 − confidence) × fraction-heuristic)` so a low-confidence estimate degrades to the historical behavior rather than replacing it. Invariants that are easy to break: -- **Capability is feature-detected per store** (`typeof store.estimateCount === 'function'`), and - the `RocksIndexStore` override is assigned conditionally for the same reason — do not define it - unconditionally or the probe lies on older rocksdb-js. The result shape is validated - (`Number.isFinite(count)`, `0 ≤ confidence ≤ 1`) and the native call is try/caught because a - caret bump can activate this path on an image rebuild without a code change. +- **Capability is feature-detected per store** (`typeof store.estimateCount === 'function'`) + because LMDB-backed and custom index stores do not implement it. The result shape is validated + (`Number.isFinite(count)`, `0 ≤ confidence ≤ 1`) and the native call is try/caught, so a store + that answers differently — or one closing concurrently — degrades the plan to the fraction + heuristic instead of NaN-poisoning condition ordering. - **The estimated range must be the executed range.** Construction mirrors `searchByIndex`'s comparator switch; bounds longer than `MAX_SEARCH_KEY_LENGTH` fall back entirely because - execution truncates + filters (wider range than the estimable one). `RocksIndexStore` widens - value-space bounds to `[value, MAXIMUM_KEY]` composite bounds — the base implementation's - byte-successor semantics would exclude the wrong entries on composite `[value, primaryKey]` keys. + execution truncates + filters (wider range than the estimable one). `RocksIndexStore.estimateCount` + widens value-space bounds to `[value, MAXIMUM_KEY]` composite bounds, mirroring its `getRange` + translation (reverse flip included) — the base implementation's byte-successor semantics would + exclude the wrong entries on composite `[value, primaryKey]` keys. - **Negated conditions estimate `Infinity` at the root** (`estimateConditionForTable`), following the filter-only convention (`contains`/`ends_with`): the negated flag always forces `needFullScan`, so `estimated_count` here is execution-cost ordering, not result cardinality — diff --git a/package-lock.json b/package-lock.json index b1f75a82ca..40ebb3642b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,7 @@ "@fastify/cors": "^11.2.0", "@fastify/static": "^9.1.3", "@harperfast/extended-iterable": "1.0.3", - "@harperfast/rocksdb-js": "2.7.1", + "@harperfast/rocksdb-js": "2.8.0", "@harperfast/skills": "^1.10.8", "@turf/area": "6.5.0", "@turf/boolean-contains": "6.5.0", @@ -2495,13 +2495,13 @@ } }, "node_modules/@harperfast/rocksdb-js": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js/-/rocksdb-js-2.7.1.tgz", - "integrity": "sha512-Fs+Ki/9ysu4w0oGl4NN+6LWlKlQtUpw0RUZlOewbAXStEzJ4087Dk1vHKM2YqnAqNYoZb0UOz2XeoM8YkelMww==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js/-/rocksdb-js-2.8.0.tgz", + "integrity": "sha512-czswCG+1KCRMYe6XQcsh5u28IQ6EbIx9eXPQd0kmjiLbtoMjiszYE92756bZJSbDykbR2OJLOazMZg7+qfFSJA==", "license": "Apache-2.0", "dependencies": { "@harperfast/extended-iterable": "1.0.3", - "msgpackr": "2.0.5", + "msgpackr": "2.0.6", "ordered-binary": "1.6.1" }, "bin": { @@ -2511,20 +2511,20 @@ "node": "^22.18.0 || >=24.0.0" }, "optionalDependencies": { - "@harperfast/rocksdb-js-darwin-arm64": "2.7.1", - "@harperfast/rocksdb-js-darwin-x64": "2.7.1", - "@harperfast/rocksdb-js-linux-arm64-glibc": "2.7.1", - "@harperfast/rocksdb-js-linux-arm64-musl": "2.7.1", - "@harperfast/rocksdb-js-linux-x64-glibc": "2.7.1", - "@harperfast/rocksdb-js-linux-x64-musl": "2.7.1", - "@harperfast/rocksdb-js-win32-arm64": "2.7.1", - "@harperfast/rocksdb-js-win32-x64": "2.7.1" + "@harperfast/rocksdb-js-darwin-arm64": "2.8.0", + "@harperfast/rocksdb-js-darwin-x64": "2.8.0", + "@harperfast/rocksdb-js-linux-arm64-glibc": "2.8.0", + "@harperfast/rocksdb-js-linux-arm64-musl": "2.8.0", + "@harperfast/rocksdb-js-linux-x64-glibc": "2.8.0", + "@harperfast/rocksdb-js-linux-x64-musl": "2.8.0", + "@harperfast/rocksdb-js-win32-arm64": "2.8.0", + "@harperfast/rocksdb-js-win32-x64": "2.8.0" } }, "node_modules/@harperfast/rocksdb-js-darwin-arm64": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-darwin-arm64/-/rocksdb-js-darwin-arm64-2.7.1.tgz", - "integrity": "sha512-H0aEOziU6WFaVUjoRvsWVUoIEAZ6ylLRYTa4z4R7SRVUt2pzpjiUMW1mHBTmIRAue/Fwsfm8SCM95WQj5goFKA==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-darwin-arm64/-/rocksdb-js-darwin-arm64-2.8.0.tgz", + "integrity": "sha512-v7cj3bGpRptZHXSbhVzYJ0k+jyMD0Z0kX0u8YNZH6Xq+ifZ6zBkBbyoSdvQ2waGLJXxBORylUxTgYMvTZWiQBA==", "cpu": [ "arm64" ], @@ -2538,9 +2538,9 @@ } }, "node_modules/@harperfast/rocksdb-js-darwin-x64": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-darwin-x64/-/rocksdb-js-darwin-x64-2.7.1.tgz", - "integrity": "sha512-YDPVGmfFyg9sCu3C+uulJa68o4B06pJ8MDWSgSqm+5uNXh4QNWuMSE1x2wLZYOt1hxE7xptHXzhMFFuSqIcvtg==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-darwin-x64/-/rocksdb-js-darwin-x64-2.8.0.tgz", + "integrity": "sha512-9mMPPvRhxjLorE7u1gN3n+InzT4XHXQkHYdNKE6qN4iwZy0vM8iHoQLwIiN9GpzUiueEqBEilAP/9mIOHhMgBw==", "cpu": [ "x64" ], @@ -2554,9 +2554,9 @@ } }, "node_modules/@harperfast/rocksdb-js-linux-arm64-glibc": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-arm64-glibc/-/rocksdb-js-linux-arm64-glibc-2.7.1.tgz", - "integrity": "sha512-8LoPDc4muFb4U09VMWC1BJZRA/CxYBer1eIRW6Td6EfB8lOiBgZlpQvmc44GilYgrH4/C1at8lMK+G24U2CDyA==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-arm64-glibc/-/rocksdb-js-linux-arm64-glibc-2.8.0.tgz", + "integrity": "sha512-H1nNimVbIB4/6Sxv5DrxmP1asHLbxTCHuZ2ZN/zudqPkp/bTTSs5RsczOH6JeKTEUxUsJDTXM4nGRcn6LwFaOg==", "cpu": [ "arm64" ], @@ -2573,9 +2573,9 @@ } }, "node_modules/@harperfast/rocksdb-js-linux-arm64-musl": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-arm64-musl/-/rocksdb-js-linux-arm64-musl-2.7.1.tgz", - "integrity": "sha512-vqeH0JM/FUbQfx1Q5n21aBCiLUukN/qMXKa5d3YUQt7hH1fFA4d9wQhnRqwV9VbIjxm6i3drYAy5HaFkU2TV1g==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-arm64-musl/-/rocksdb-js-linux-arm64-musl-2.8.0.tgz", + "integrity": "sha512-EqG4lkYptNviIcPPaQjK+X4ufjqzSImIvVxP89GHFHrbOJkGgEwRpiya0gWfGu3vJ1iGL1XL1nsU/kEjeDOugg==", "cpu": [ "arm64" ], @@ -2592,9 +2592,9 @@ } }, "node_modules/@harperfast/rocksdb-js-linux-x64-glibc": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-x64-glibc/-/rocksdb-js-linux-x64-glibc-2.7.1.tgz", - "integrity": "sha512-2pbbjq36Ln+ln31a3oDf2ic5IQNvrXNP9ND6LM7lMYUjuG/7tV3tWK79x2AyATym6iq0Kmz5Sp0b8tnUL2nQNg==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-x64-glibc/-/rocksdb-js-linux-x64-glibc-2.8.0.tgz", + "integrity": "sha512-MZPKZ6gF8kCy/KXTav4tc7T2uq+iOunxDOYq7nifZkCCy+0+C9U7g9CZx0NErORRQRP29x3z0zEKkCQ/go5c6Q==", "cpu": [ "x64" ], @@ -2611,9 +2611,9 @@ } }, "node_modules/@harperfast/rocksdb-js-linux-x64-musl": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-x64-musl/-/rocksdb-js-linux-x64-musl-2.7.1.tgz", - "integrity": "sha512-hrk3BvNjjrWV0j0XjDZ80r3XIQdLnP2nH/WNy+KFU484XEkdXONmj0zH0hsOJVItTv3JShEECWOk6BXlR3osaw==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-linux-x64-musl/-/rocksdb-js-linux-x64-musl-2.8.0.tgz", + "integrity": "sha512-PnMn1f/UxGOZKxlayR82tzsFCGoaMu0mBGKMmP/r2rbxzX5T89T0c+aVhuhpgNrFlWXTdawlA10Oh/YjfT/2qQ==", "cpu": [ "x64" ], @@ -2630,9 +2630,9 @@ } }, "node_modules/@harperfast/rocksdb-js-win32-arm64": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-win32-arm64/-/rocksdb-js-win32-arm64-2.7.1.tgz", - "integrity": "sha512-E4Y725WvIcJjfSidOJt/gee4ogKWaFxj8exX/D6Kw2+v5RxuVjMgyKyOIxXQ+nLd18XGXTKj6HtgYzWOJfbEGg==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-win32-arm64/-/rocksdb-js-win32-arm64-2.8.0.tgz", + "integrity": "sha512-4khqmGbIebiCwY3FVIgcUXEmMapgWP8f97aNrbThPDCIkuJscB3yoAG4A8OJo+Jln/TdjJZOgy8e2ir9F926dA==", "cpu": [ "arm64" ], @@ -2646,9 +2646,9 @@ } }, "node_modules/@harperfast/rocksdb-js-win32-x64": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-win32-x64/-/rocksdb-js-win32-x64-2.7.1.tgz", - "integrity": "sha512-7TcSJ3gXgknoccLe+Tc0nqAxOA1kt5FE30GFrrTs7MyZczdO1rioRiccqhgcRA4UvW8Xc29GwJPd9ehmK8yYHg==", + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@harperfast/rocksdb-js-win32-x64/-/rocksdb-js-win32-x64-2.8.0.tgz", + "integrity": "sha512-E5HpBAnRwotITq++534WCinMvYidkkPoaAD5uwBNHiQbNt6YYvNZllJAIh/bukXD+uO6DL6qUMmVkeBh3kWK5A==", "cpu": [ "x64" ], @@ -2661,6 +2661,15 @@ "node": "^22.18.0 || >=24.0.0" } }, + "node_modules/@harperfast/rocksdb-js/node_modules/msgpackr": { + "version": "2.0.6", + "resolved": "https://registry.npmjs.org/msgpackr/-/msgpackr-2.0.6.tgz", + "integrity": "sha512-plGul/tqjt9vqWFR9zyqyLZls6gb5KTLvnsRO2B9+TZ8tNiXiVI/CR8LiDWlAX4IUeVkQ9gsmzKA6voC2QOciw==", + "license": "MIT", + "optionalDependencies": { + "msgpackr-extract": "^3.0.4" + } + }, "node_modules/@harperfast/skills": { "version": "1.12.1", "resolved": "https://registry.npmjs.org/@harperfast/skills/-/skills-1.12.1.tgz", diff --git a/package.json b/package.json index f2e6dedab1..2fa605ef15 100644 --- a/package.json +++ b/package.json @@ -179,7 +179,7 @@ "@fastify/cors": "^11.2.0", "@fastify/static": "^9.1.3", "@harperfast/extended-iterable": "1.0.3", - "@harperfast/rocksdb-js": "2.7.1", + "@harperfast/rocksdb-js": "2.8.0", "@harperfast/skills": "^1.10.8", "@turf/area": "6.5.0", "@turf/boolean-contains": "6.5.0", diff --git a/resources/RocksIndexStore.ts b/resources/RocksIndexStore.ts index 3ac458b7af..e28f25456e 100644 --- a/resources/RocksIndexStore.ts +++ b/resources/RocksIndexStore.ts @@ -1,4 +1,6 @@ import { + type CountEstimate, + type CountEstimateOptions, DBI, type StoreIteratorOptions, type StorePutOptions, @@ -40,6 +42,22 @@ export class RocksIndexStore extends RocksDatabase { }); } + /** + * Estimate the entry count of a range of indexed values. Bounds must widen to + * `[value, MAXIMUM_KEY]` composite bounds exactly as `getRange` does, rather than the base + * implementation's encoded-byte successor of the bare indexed value. + */ + estimateCount(options?: CountEstimateOptions): CountEstimate { + let { start, end, exclusiveStart, inclusiveEnd, reverse } = options ?? {}; + if ((reverse ? !exclusiveStart : exclusiveStart) && start !== undefined) { + start = [start, MAXIMUM_KEY]; + } + if ((reverse ? !inclusiveEnd : inclusiveEnd) && end !== undefined) { + end = [end, MAXIMUM_KEY]; + } + return super.estimateCount({ ...options, start, end }); + } + /** * Translate a put with indexed value and primary key to an underlying put * @param indexedValue - ignored, only used by LMDB @@ -66,18 +84,6 @@ export class RocksIndexStore extends RocksDatabase { } } -// Bounds on indexed values must widen to [value, MAXIMUM_KEY] composite bounds (as in getRange), -// not the base implementation's encoded-byte successor. Assigned conditionally so -// `typeof store.estimateCount === 'function'` stays a capability probe on older rocksdb-js. -if (typeof (RocksDatabase.prototype as any).estimateCount === 'function') { - (RocksIndexStore.prototype as any).estimateCount = function estimateCount(options?: any) { - let { start, end, exclusiveStart, inclusiveEnd } = options ?? {}; - if (exclusiveStart && start !== undefined) start = [start, MAXIMUM_KEY]; - if (inclusiveEnd && end !== undefined) end = [end, MAXIMUM_KEY]; - return (RocksDatabase.prototype as any).estimateCount.call(this, { start, end }); - }; -} - /** * Add `getValuesCount` to the DBI prototype which is used by the `RocksDatabase` and `Transaction` * classes. diff --git a/resources/search.ts b/resources/search.ts index fbe32aede3..45b5564e42 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1122,6 +1122,7 @@ function estimateRangeCondition(table, condition, searchType, fraction) { const attributeName = condition[0] ?? condition.attribute; const isPrimaryKey = attributeName === table.primaryKey; const store = isPrimaryKey ? table.primaryStore : table.indices[attributeName]; + // LMDB-backed and custom index stores don't implement estimateCount if (typeof store?.estimateCount !== 'function') return undefined; let value = condition[1] ?? condition.value; if (value instanceof Date) value = value.getTime(); @@ -1188,7 +1189,7 @@ function estimateRangeCondition(table, condition, searchType, fraction) { // a concurrently closing/dropped store must degrade the plan, not fail the query return undefined; } - // The dependency pin can activate this path on an image rebuild; never trust the shape blind. + // LMDB and custom index stores may expose their own estimateCount; validate the contract. if (!Number.isFinite(count) || !(confidence >= 0 && confidence <= 1)) return undefined; const heuristic = fraction * estimatedEntryCount(table.primaryStore) + 1; return Math.max(1, Math.round(confidence * count + (1 - confidence) * heuristic)); diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index 2c78ab5a33..8e6dac38ec 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -171,10 +171,7 @@ describe('estimateCondition range estimates', () => { }); }); -const { RocksDatabase } = require('@harperfast/rocksdb-js'); -const supportsEstimateCount = typeof RocksDatabase.prototype.estimateCount === 'function'; - -(supportsEstimateCount ? describe : describe.skip)('estimateCondition range estimates (real stores)', () => { +describe('estimateCondition range estimates (real stores)', () => { const { setupTestDBPath } = require('../testUtils'); const { table } = require('#src/resources/databases'); const { setMainIsWorker } = require('#js/server/threads/manageThreads'); From 30d4060a2a943e7ef10251cc5169d4748d57dd91 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 25 Aug 2026 16:22:38 -0600 Subject: [PATCH 06/13] Gate the real-store estimate suite on the live store, not the Rocks prototype test:unit:lmdb re-runs test:unit:resources with HARPER_STORAGE_ENGINE=lmdb, where the index stores have no estimateCount at all. Keying the skip off RocksDatabase.prototype was already wrong and became always-true once 2.8.0 was pinned, so gate on the table's actual index store instead. Co-Authored-By: Claude Opus 5 --- resources/search.ts | 4 ++-- unitTests/resources/estimateRangeCondition.test.js | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/resources/search.ts b/resources/search.ts index 45b5564e42..e97e0acbcd 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1122,7 +1122,8 @@ function estimateRangeCondition(table, condition, searchType, fraction) { const attributeName = condition[0] ?? condition.attribute; const isPrimaryKey = attributeName === table.primaryKey; const store = isPrimaryKey ? table.primaryStore : table.indices[attributeName]; - // LMDB-backed and custom index stores don't implement estimateCount + // LMDB-backed and custom index stores may not implement estimateCount, or may answer with a + // different shape, so both the probe and the validation below are load-bearing if (typeof store?.estimateCount !== 'function') return undefined; let value = condition[1] ?? condition.value; if (value instanceof Date) value = value.getTime(); @@ -1189,7 +1190,6 @@ function estimateRangeCondition(table, condition, searchType, fraction) { // a concurrently closing/dropped store must degrade the plan, not fail the query return undefined; } - // LMDB and custom index stores may expose their own estimateCount; validate the contract. if (!Number.isFinite(count) || !(confidence >= 0 && confidence <= 1)) return undefined; const heuristic = fraction * estimatedEntryCount(table.primaryStore) + 1; return Math.max(1, Math.round(confidence * count + (1 - confidence) * heuristic)); diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index 8e6dac38ec..4786ab8e2c 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -191,6 +191,9 @@ describe('estimateCondition range estimates (real stores)', () => { { name: 'name', indexed: true }, ], }); + // HARPER_STORAGE_ENGINE=lmdb runs this same suite against index stores with no + // estimateCount, where every range falls back to the flat heuristic + if (typeof T.indices.score.estimateCount !== 'function') return this.skip(); let last; for (let i = 0; i < N; i++) { last = T.put({ id: i, score: i, name: `name-${String(i).padStart(6, '0')}` }); From cc21ae036977544f294569aab5d87f70949aa2bd Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 25 Aug 2026 17:32:56 -0600 Subject: [PATCH 07/13] Estimate lt/le over the range execution actually iterates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit searchByIndex bounds lt/le at `start: true`, which sorts above `null`, so an indexNulls index's `[null, primaryKey]` entries are outside the executed range. estimateRangeCondition left the lower bound open and counted them: on an index that is 99% nulls the estimate came back 21x high (19960 for a 200-row condition), so a genuinely selective condition looked like a full scan and lost the driving-condition ordering — worse than the flat heuristic it replaces. getRange and estimateCount now share one translateIndexBounds helper so the "estimate the range you execute" invariant is structural instead of two copies, and a spy test pins that both forward identical bounds, reverse branch included. Co-Authored-By: Claude Opus 5 --- DESIGN.md | 12 ++- resources/RocksIndexStore.ts | 43 +++++----- resources/search.ts | 9 +- .../resources/estimateRangeCondition.test.js | 83 ++++++++++++++++++- 4 files changed, 117 insertions(+), 30 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index aea339e0b2..7c337f515b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1286,10 +1286,14 @@ degrades to the historical behavior rather than replacing it. Invariants that ar heuristic instead of NaN-poisoning condition ordering. - **The estimated range must be the executed range.** Construction mirrors `searchByIndex`'s comparator switch; bounds longer than `MAX_SEARCH_KEY_LENGTH` fall back entirely because - execution truncates + filters (wider range than the estimable one). `RocksIndexStore.estimateCount` - widens value-space bounds to `[value, MAXIMUM_KEY]` composite bounds, mirroring its `getRange` - translation (reverse flip included) — the base implementation's byte-successor semantics would - exclude the wrong entries on composite `[value, primaryKey]` keys. + execution truncates + filters (wider range than the estimable one). Two ways this has already + been got wrong: `lt`/`le` need `searchByIndex`'s `start: true` lower bound, or the estimate + counts the `[null, primaryKey]` entries an `indexNulls` index holds and execution skips (`true` + sorts above `null`) — measured at 21× inflation on an index that is 99% nulls, which is worse + than the flat heuristic it replaces; and `RocksIndexStore` must widen value-space bounds to + `[value, MAXIMUM_KEY]` composite bounds, because the base implementation's byte-successor + semantics exclude the wrong entries on composite `[value, primaryKey]` keys. `getRange` and + `estimateCount` therefore share one `translateIndexBounds` helper rather than two copies. - **Negated conditions estimate `Infinity` at the root** (`estimateConditionForTable`), following the filter-only convention (`contains`/`ends_with`): the negated flag always forces `needFullScan`, so `estimated_count` here is execution-cost ordering, not result cardinality — diff --git a/resources/RocksIndexStore.ts b/resources/RocksIndexStore.ts index e28f25456e..5696c4d862 100644 --- a/resources/RocksIndexStore.ts +++ b/resources/RocksIndexStore.ts @@ -16,6 +16,25 @@ declare module '@harperfast/rocksdb-js' { } } +/** + * Widen value-space bounds to `[value, MAXIMUM_KEY]` composite bounds. The base implementation's + * encoded-byte successor of the bare indexed value would exclude the wrong entries on composite + * `[indexedValue, primaryKey]` keys. + */ +function translateIndexBounds< + T extends { start?: any; end?: any; exclusiveStart?: boolean; inclusiveEnd?: boolean; reverse?: boolean }, +>(options: T): T { + let { start, end } = options; + const { exclusiveStart, inclusiveEnd, reverse } = options; + if ((reverse ? !exclusiveStart : exclusiveStart) && start !== undefined) { + start = [start, MAXIMUM_KEY]; + } + if ((reverse ? !inclusiveEnd : inclusiveEnd) && end !== undefined) { + end = [end, MAXIMUM_KEY]; + } + return { ...options, start, end }; +} + /** * A specialized RocksDB-based index store that maintains indexed references to primary keys. * This store uses composite keys consisting of indexed values and primary keys, enabling @@ -29,33 +48,17 @@ export class RocksIndexStore extends RocksDatabase { * @param options */ getRange(options: StoreIteratorOptions): Iterable { - let { start, end, exclusiveStart, inclusiveEnd, reverse } = options; - if ((reverse ? !exclusiveStart : exclusiveStart) && start !== undefined) { - start = [start, MAXIMUM_KEY]; - } - if ((reverse ? !inclusiveEnd : inclusiveEnd) && end !== undefined) { - end = [end, MAXIMUM_KEY]; - } - const translatedOptions = { ...options, start, end }; - return super.getRange(translatedOptions).map(({ key }) => { + return super.getRange(translateIndexBounds(options)).map(({ key }) => { return { key: key[0], value: key.length > 2 ? key.slice(1) : key[1] }; }); } /** - * Estimate the entry count of a range of indexed values. Bounds must widen to - * `[value, MAXIMUM_KEY]` composite bounds exactly as `getRange` does, rather than the base - * implementation's encoded-byte successor of the bare indexed value. + * Estimate the entry count of a range of indexed values. Shares `getRange`'s bound translation + * so a planner estimate always covers exactly the range execution would iterate. */ estimateCount(options?: CountEstimateOptions): CountEstimate { - let { start, end, exclusiveStart, inclusiveEnd, reverse } = options ?? {}; - if ((reverse ? !exclusiveStart : exclusiveStart) && start !== undefined) { - start = [start, MAXIMUM_KEY]; - } - if ((reverse ? !inclusiveEnd : inclusiveEnd) && end !== undefined) { - end = [end, MAXIMUM_KEY]; - } - return super.estimateCount({ ...options, start, end }); + return super.estimateCount(translateIndexBounds(options ?? {})); } /** diff --git a/resources/search.ts b/resources/search.ts index e97e0acbcd..dbdc0c62c0 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1122,18 +1122,19 @@ function estimateRangeCondition(table, condition, searchType, fraction) { const attributeName = condition[0] ?? condition.attribute; const isPrimaryKey = attributeName === table.primaryKey; const store = isPrimaryKey ? table.primaryStore : table.indices[attributeName]; - // LMDB-backed and custom index stores may not implement estimateCount, or may answer with a - // different shape, so both the probe and the validation below are load-bearing + // LMDB-backed and custom index stores do not implement estimateCount if (typeof store?.estimateCount !== 'function') return undefined; let value = condition[1] ?? condition.value; if (value instanceof Date) value = value.getTime(); let range; switch (searchType) { case 'lt': - range = { end: value }; + // `start: true` mirrors searchByIndex: `true` sorts above `null`, so an indexNulls + // index's `[null, primaryKey]` entries are outside the executed range + range = { start: true, end: value }; break; case 'le': - range = { end: value, inclusiveEnd: true }; + range = { start: true, end: value, inclusiveEnd: true }; break; case 'gt': range = { start: value, exclusiveStart: true }; diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index 4786ab8e2c..fc0bf2400b 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -87,9 +87,15 @@ describe('estimateCondition range estimates', () => { assert.strictEqual(estimate(table, { attribute: 'attr', comparator: 'gt', value: 5 }), 40); assert.deepStrictEqual(table.indices.attr.calls[0], { start: 5, exclusiveStart: true }); + // `start: true` is not decoration: searchByIndex uses it to skip an indexNulls index's + // [null, primaryKey] entries, so an unbounded estimate would count rows execution never visits const table2 = makeTable({ indexEstimate: { count: 40, confidence: 1 } }); assert.strictEqual(estimate(table2, { attribute: 'attr', comparator: 'lt', value: 5 }), 40); - assert.deepStrictEqual(table2.indices.attr.calls[0], { end: 5 }); + assert.deepStrictEqual(table2.indices.attr.calls[0], { start: true, end: 5 }); + + const table3 = makeTable({ indexEstimate: { count: 40, confidence: 1 } }); + assert.strictEqual(estimate(table3, { attribute: 'attr', comparator: 'le', value: 5 }), 40); + assert.deepStrictEqual(table3.indices.attr.calls[0], { start: true, end: 5, inclusiveEnd: true }); }); it('estimates primary-key ranges against the primary store', () => { @@ -171,11 +177,64 @@ describe('estimateCondition range estimates', () => { }); }); +describe('RocksIndexStore.estimateCount composite bound translation', () => { + const { RocksIndexStore } = require('#src/resources/RocksIndexStore'); + const { RocksDatabase } = require('@harperfast/rocksdb-js'); + + // Both methods must translate bounds identically or the planner estimates a range execution + // never iterates. Spying on the base captures what each actually forwards. + function captureBounds(method, options) { + const original = RocksDatabase.prototype[method]; + let captured; + RocksDatabase.prototype[method] = function (received) { + captured = received; + return method === 'estimateCount' ? { count: 0, confidence: 1 } : []; + }; + try { + RocksIndexStore.prototype[method].call(Object.create(RocksIndexStore.prototype), options); + } finally { + RocksDatabase.prototype[method] = original; + } + return captured; + } + + const cases = [ + ['bare bounds pass through', { start: 5, end: 10 }], + ['exclusiveStart widens the lower bound', { start: 5, end: 10, exclusiveStart: true }], + ['inclusiveEnd widens the upper bound', { start: 5, end: 10, inclusiveEnd: true }], + ['reverse flips which bound widens', { start: 10, end: 5, reverse: true }], + ['reverse with explicit flags', { start: 10, end: 5, reverse: true, exclusiveStart: true, inclusiveEnd: true }], + ]; + + for (const [name, options] of cases) { + it(`${name} the same way getRange does`, () => { + const estimated = captureBounds('estimateCount', options); + const iterated = captureBounds('getRange', options); + assert.deepStrictEqual( + { start: estimated.start, end: estimated.end }, + { start: iterated.start, end: iterated.end } + ); + }); + } + + it('widens to [value, MAXIMUM_KEY] rather than the bare indexed value', () => { + const { start, end } = captureBounds('estimateCount', { + start: 5, + end: 10, + exclusiveStart: true, + inclusiveEnd: true, + }); + assert.deepStrictEqual(start, [5, MAXIMUM_KEY]); + assert.deepStrictEqual(end, [10, MAXIMUM_KEY]); + }); +}); + describe('estimateCondition range estimates (real stores)', () => { const { setupTestDBPath } = require('../testUtils'); const { table } = require('#src/resources/databases'); const { setMainIsWorker } = require('#js/server/threads/manageThreads'); const N = 20000; + const SPARSE_COUNT = 200; let T; before(async function () { @@ -189,6 +248,7 @@ describe('estimateCondition range estimates (real stores)', () => { { name: 'id', isPrimaryKey: true }, { name: 'score', type: 'Int', indexed: true }, { name: 'name', indexed: true }, + { name: 'sparse', type: 'Int', indexed: true }, ], }); // HARPER_STORAGE_ENGINE=lmdb runs this same suite against index stores with no @@ -196,12 +256,19 @@ describe('estimateCondition range estimates (real stores)', () => { if (typeof T.indices.score.estimateCount !== 'function') return this.skip(); let last; for (let i = 0; i < N; i++) { - last = T.put({ id: i, score: i, name: `name-${String(i).padStart(6, '0')}` }); + last = T.put({ + id: i, + score: i, + name: `name-${String(i).padStart(6, '0')}`, + // explicit nulls are indexed as [null, primaryKey]; a missing attribute is not + sparse: i < SPARSE_COUNT ? i : null, + }); } await last; await T.primaryStore.flush(); await T.indices.score.flush(); await T.indices.name.flush(); + await T.indices.sparse.flush(); }); it('scales between estimates with the real range width', () => { @@ -228,4 +295,16 @@ describe('estimateCondition range estimates (real stores)', () => { assert.ok(tail10 < tail50, `10% tail (${tail10}) should be < 50% tail (${tail50})`); assert.ok(tail10 <= 0.3 * N + 1, `10% tail (${tail10}) should not exceed the flat heuristic`); }); + + it('excludes null index entries from lt/le, as execution does', () => { + // `sparse` is set on 200 of 20000 rows, so the index holds 19800 [null, id] entries that + // searchByIndex's `start: true` skips. An unbounded lower bound counts them all and makes + // a 200-row condition look like a 20000-row one — worse than the heuristic it replaces. + const est = estimateCondition(T); + const belowAll = est({ attribute: 'sparse', comparator: 'lt', value: SPARSE_COUNT }); + assert.ok( + belowAll < N / 2, + `lt over all ${SPARSE_COUNT} non-null values (${belowAll}) must not count the ${N - SPARSE_COUNT} null entries` + ); + }); }); From 2b016c8a85444d1c774a89397087e6497a41c5f4 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 25 Aug 2026 17:39:35 -0600 Subject: [PATCH 08/13] Pin estimates against executed result counts, not just against each other MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real-store suite compared estimates to one another (narrow < wide), which passes even if every estimate is an order of magnitude off. Three cases now run the actual query and assert the estimate lands within 10x of what it returned, covering the primary-key path as well — that path passes `start: true` through to the primary store, and nothing pinned it. Co-Authored-By: Claude Opus 5 --- .../resources/estimateRangeCondition.test.js | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index fc0bf2400b..9b9a22c226 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -87,8 +87,7 @@ describe('estimateCondition range estimates', () => { assert.strictEqual(estimate(table, { attribute: 'attr', comparator: 'gt', value: 5 }), 40); assert.deepStrictEqual(table.indices.attr.calls[0], { start: 5, exclusiveStart: true }); - // `start: true` is not decoration: searchByIndex uses it to skip an indexNulls index's - // [null, primaryKey] entries, so an unbounded estimate would count rows execution never visits + // searchByIndex bounds lt/le at `start: true` to skip an indexNulls index's [null, id] entries const table2 = makeTable({ indexEstimate: { count: 40, confidence: 1 } }); assert.strictEqual(estimate(table2, { attribute: 'attr', comparator: 'lt', value: 5 }), 40); assert.deepStrictEqual(table2.indices.attr.calls[0], { start: true, end: 5 }); @@ -182,7 +181,7 @@ describe('RocksIndexStore.estimateCount composite bound translation', () => { const { RocksDatabase } = require('@harperfast/rocksdb-js'); // Both methods must translate bounds identically or the planner estimates a range execution - // never iterates. Spying on the base captures what each actually forwards. + // never iterates. function captureBounds(method, options) { const original = RocksDatabase.prototype[method]; let captured; @@ -296,6 +295,25 @@ describe('estimateCondition range estimates (real stores)', () => { assert.ok(tail10 <= 0.3 * N + 1, `10% tail (${tail10}) should not exceed the flat heuristic`); }); + // The estimate is only worth anything if it tracks what the query returns; the assertions above + // compare estimates to each other, this one compares them to reality. + for (const [name, condition] of [ + ['secondary index between', { attribute: 'score', comparator: 'between', value: [1000, 3000] }], + ['secondary index lt', { attribute: 'score', comparator: 'lt', value: 4000 }], + ['primary key lt', { attribute: 'id', comparator: 'lt', value: 1000 }], + ]) { + it(`estimates ${name} within an order of magnitude of the executed result count`, async () => { + const estimated = estimateCondition(T)(condition); + let actual = 0; + for await (const _ of T.search({ conditions: [condition], select: ['id'] })) actual++; + assert.ok(actual > 0, `precondition: ${name} must match rows, got ${actual}`); + assert.ok( + estimated > actual / 10 && estimated < actual * 10, + `${name}: estimated ${estimated} is not within 10x of the executed ${actual}` + ); + }); + } + it('excludes null index entries from lt/le, as execution does', () => { // `sparse` is set on 200 of 20000 rows, so the index holds 19800 [null, id] entries that // searchByIndex's `start: true` skips. An unbounded lower bound counts them all and makes From 29a6345940a678491699c365d5cd0347200637bf Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Wed, 26 Aug 2026 07:07:24 -0600 Subject: [PATCH 09/13] Align root msgpackr pin with rocksdb-js 2.8.0's dependency rocksdb-js 2.8.0 depends on msgpackr@2.0.6; the root package.json still pinned 2.0.5, so npm couldn't dedupe to a single msgpackr instance and the shrinkwrap-pin smoke check failed. Co-Authored-By: Claude Sonnet 5 --- package-lock.json | 17 ++++------------- package.json | 2 +- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index 40ebb3642b..db96d3044a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -61,7 +61,7 @@ "minimist": "1.2.8", "moment": "2.30.1", "mqtt-packet": "~9.0.1", - "msgpackr": "2.0.5", + "msgpackr": "2.0.6", "needle": "3.5.0", "node-forge": "^1.3.1", "node-stream-zip": "1.16.0", @@ -2661,15 +2661,6 @@ "node": "^22.18.0 || >=24.0.0" } }, - "node_modules/@harperfast/rocksdb-js/node_modules/msgpackr": { - "version": "2.0.6", - "resolved": "https://registry.npmjs.org/msgpackr/-/msgpackr-2.0.6.tgz", - "integrity": "sha512-plGul/tqjt9vqWFR9zyqyLZls6gb5KTLvnsRO2B9+TZ8tNiXiVI/CR8LiDWlAX4IUeVkQ9gsmzKA6voC2QOciw==", - "license": "MIT", - "optionalDependencies": { - "msgpackr-extract": "^3.0.4" - } - }, "node_modules/@harperfast/skills": { "version": "1.12.1", "resolved": "https://registry.npmjs.org/@harperfast/skills/-/skills-1.12.1.tgz", @@ -11566,9 +11557,9 @@ "license": "MIT" }, "node_modules/msgpackr": { - "version": "2.0.5", - "resolved": "https://registry.npmjs.org/msgpackr/-/msgpackr-2.0.5.tgz", - "integrity": "sha512-cef05H/dSYpLpqp3sj/qyZh5vhUYCalnaLO7j1yOmpsR0y/XwLVtK7r5gn+U/F7CTEfMowcGhlUQJDLcLf7jcA==", + "version": "2.0.6", + "resolved": "https://registry.npmjs.org/msgpackr/-/msgpackr-2.0.6.tgz", + "integrity": "sha512-plGul/tqjt9vqWFR9zyqyLZls6gb5KTLvnsRO2B9+TZ8tNiXiVI/CR8LiDWlAX4IUeVkQ9gsmzKA6voC2QOciw==", "license": "MIT", "optionalDependencies": { "msgpackr-extract": "^3.0.4" diff --git a/package.json b/package.json index 2fa605ef15..1f4e711481 100644 --- a/package.json +++ b/package.json @@ -223,7 +223,7 @@ "minimist": "1.2.8", "moment": "2.30.1", "mqtt-packet": "~9.0.1", - "msgpackr": "2.0.5", + "msgpackr": "2.0.6", "needle": "3.5.0", "node-forge": "^1.3.1", "node-stream-zip": "1.16.0", From c568812689c3676737cd391567f047551a733d28 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 27 Aug 2026 08:36:17 -0600 Subject: [PATCH 10/13] Fix DESIGN.md formatting after rebase conflict resolution Prettier wants blank lines around the two independently-appended sections merged during the rebase onto main. --- DESIGN.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/DESIGN.md b/DESIGN.md index 7c337f515b..ab8642afd8 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1213,6 +1213,7 @@ An `{}` in the config system is context-dependent, and conflating the contexts i Removal therefore prunes: `deleteNestedValue` removes ancestors the deletion emptied, only when it actually deleted an existing leaf, and reports what it pruned. The overlap case — a file-declared empty scope an env layer temporarily populated — is tracked in the state file's `emptyScopeOriginals` (separate from `originalValues` so a marker can never mask or be consumed as a real leaf original at the same path; older state files lacking the field are defaulted). Restore consumes a marker only for a path the prune actually removed, so a scalar overwrite or an absent-leaf no-op can never resurrect a scope over live env-layer content. Note there are two coexisting mechanisms for "file `{}` is user content": `restoreBaseEmptyObjects` on the stateless compose path and the marker pair on the stateful removal path — if you touch one, check the other. Two durable limitations of the marker mechanism, both with user config-file content as the blast radius: markers can only be recorded at populate time, so a scope an env layer populated _before_ `emptyScopeOriginals` existed (any pre-upgrade boot) has no marker and prunes away on its first post-upgrade vacate; and a corrupt config-state file resets to fresh state — dropping `originalValues` and `emptyScopeOriginals` for every tracked path — after which the next removal prunes those scopes for good; `saveConfigState` writes via temp+rename precisely so a torn write cannot be the trigger, leaving genuine corruption (disk faults, hand edits) as the remaining path. + ## Every path handed to a native file watch must be canonicalized (`utility/watchPath.ts`) libuv's Windows fs-event callback rebuilds each event's absolute path, expands it with @@ -1247,6 +1248,7 @@ absolute paths built from `cwd`, so its bases must be derived from the same spel paths are relative to `cwd` and reads stay on the configured `component.directory`. And a watcher that degrades to polling stays there for its lifetime, so a caller with no polling story of its own (`resources/blob.ts`) needs one — there it polls `readMore` on the existing no-progress deadline. + ## No descriptor on the root config may outlive a turn (`config/configUtils.ts`, `config/RootConfigWatcher.ts`, `components/OptionsWatcher.ts`) `atomicWriteFile` replaces `harper-config.yaml` by rename-over and retries `EPERM`/`EACCES` with a From a824835d36edab3ff1b67dc152d0bfc13c65a6ff Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 27 Aug 2026 10:54:04 -0600 Subject: [PATCH 11/13] Short-circuit negated range conditions before the native estimate call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit condition.negated forced estimated_count to Infinity at the end of estimateConditionForTable, but every branch above it (including the native estimateCount FFI call for range comparators) still ran first and had its result discarded. Hoist the negated check to skip that work entirely — same outcome, no wasted native call. Found by the pre-push independent review (codex, graded leg) on this rebase. Co-Authored-By: Claude Sonnet 5 --- resources/search.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/resources/search.ts b/resources/search.ts index dbdc0c62c0..58fa7f482b 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1225,7 +1225,14 @@ export function estimateCondition(table) { // skip if it is cached let searchType = condition.comparator || condition.search_type; searchType = ALTERNATE_COMPARATOR_NAMES[searchType] || searchType; - if (searchType === SEARCH_TYPES.EQUALS || !searchType) { + if (condition.negated) { + // a negated condition always executes as a full scan (searchByIndex forces + // needFullScan), so follow the filter-only convention used by contains/ends_with: + // estimate Infinity so its positive-range estimate can never win the driving-condition + // ordering. Short-circuit here rather than computing (and discarding) a positive-range + // estimate that would cross the native FFI boundary for nothing. + condition.estimated_count = Infinity; + } else if (searchType === SEARCH_TYPES.EQUALS || !searchType) { const attribute_name = condition[0] ?? condition.attribute; if (attribute_name == null || attribute_name === table.primaryKey) condition.estimated_count = 1; else if (Array.isArray(attribute_name) && attribute_name.length > 1) { @@ -1301,11 +1308,6 @@ export function estimateCondition(table) { estimateRangeCondition(table, condition, searchType, OPEN_RANGE_ESTIMATE) ?? OPEN_RANGE_ESTIMATE * estimatedEntryCount(table.primaryStore) + 1; } - // a negated condition always executes as a full scan (searchByIndex forces - // needFullScan), so follow the filter-only convention used by contains/ends_with: - // estimate Infinity so its positive-range estimate can never win the - // driving-condition ordering - if (condition.negated) condition.estimated_count = Infinity; // we give a condition significantly more weight/preference if we will be ordering by it if (typeof condition.descending === 'boolean') condition.estimated_count /= 2; } From 30c51bb0f80be7d21c33842976492edb9360548c Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 27 Aug 2026 11:08:04 -0600 Subject: [PATCH 12/13] Reject negative counts in the estimateCount result-shape validation estimateRangeCondition checked Number.isFinite(count) and the confidence bound but not count's sign, so a store returning a negative count (a malformed/buggy native response) would pass validation and blend a negative number into the estimate. Found by the pre-push independent review (codex, graded leg) on this rebase. Co-Authored-By: Claude Sonnet 5 --- resources/search.ts | 2 +- unitTests/resources/estimateRangeCondition.test.js | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/resources/search.ts b/resources/search.ts index 58fa7f482b..5b014ca09c 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1191,7 +1191,7 @@ function estimateRangeCondition(table, condition, searchType, fraction) { // a concurrently closing/dropped store must degrade the plan, not fail the query return undefined; } - if (!Number.isFinite(count) || !(confidence >= 0 && confidence <= 1)) return undefined; + if (!Number.isFinite(count) || count < 0 || !(confidence >= 0 && confidence <= 1)) return undefined; const heuristic = fraction * estimatedEntryCount(table.primaryStore) + 1; return Math.max(1, Math.round(confidence * count + (1 - confidence) * heuristic)); } diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index 9b9a22c226..e42cab5b74 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -148,7 +148,14 @@ describe('estimateCondition range estimates', () => { }); it('falls back when the estimate shape is unexpected', () => { - for (const bad of [42, { count: NaN, confidence: 1 }, { count: 10 }, { count: 10, confidence: 2 }, null]) { + for (const bad of [ + 42, + { count: NaN, confidence: 1 }, + { count: -1, confidence: 1 }, + { count: 10 }, + { count: 10, confidence: 2 }, + null, + ]) { const table = makeTable({ indexEstimate: bad }); const estimated = estimate(table, { attribute: 'attr', comparator: 'between', value: [5, 10] }); assert.strictEqual(estimated, 0.1 * ENTRY_COUNT + 1, `shape ${JSON.stringify(bad)} must fall back`); From 44f7fff7fc80097d3447ed7b58f7208a1afbe497 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 27 Aug 2026 11:41:05 -0600 Subject: [PATCH 13/13] Reject non-numeric confidence in the estimateCount result-shape validation The malformed-shape guard rejected NaN/negative count but let a non-numeric-but-comparable confidence (e.g. NaN via >= coercion quirks, or an out-of-range non-finite value) slip through the >=/<= comparison. Mirror the count check: require Number.isFinite before bounds-checking. Co-Authored-By: Claude Sonnet 5 --- resources/search.ts | 3 ++- unitTests/resources/estimateRangeCondition.test.js | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/resources/search.ts b/resources/search.ts index 5b014ca09c..52576db747 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -1191,7 +1191,8 @@ function estimateRangeCondition(table, condition, searchType, fraction) { // a concurrently closing/dropped store must degrade the plan, not fail the query return undefined; } - if (!Number.isFinite(count) || count < 0 || !(confidence >= 0 && confidence <= 1)) return undefined; + if (!Number.isFinite(count) || count < 0 || !Number.isFinite(confidence) || confidence < 0 || confidence > 1) + return undefined; const heuristic = fraction * estimatedEntryCount(table.primaryStore) + 1; return Math.max(1, Math.round(confidence * count + (1 - confidence) * heuristic)); } diff --git a/unitTests/resources/estimateRangeCondition.test.js b/unitTests/resources/estimateRangeCondition.test.js index e42cab5b74..54f453ebe5 100644 --- a/unitTests/resources/estimateRangeCondition.test.js +++ b/unitTests/resources/estimateRangeCondition.test.js @@ -154,6 +154,8 @@ describe('estimateCondition range estimates', () => { { count: -1, confidence: 1 }, { count: 10 }, { count: 10, confidence: 2 }, + { count: 10, confidence: NaN }, + { count: 10, confidence: -0.1 }, null, ]) { const table = makeTable({ indexEstimate: bad });