Size tables for query planning with the O(1) RocksDB key estimate instead of a full key-space scan (backport #2163 to v5.1)

[kriszyp]

estimatedEntryCount() sized every table for query planning by calling RocksDatabase.getKeysCount() — a synchronous native iteration of the entire key space. On a 28.6M-row analytics table that is ~11s of blocked main thread per call, and its 10-second memo never took effect there: now is sampled before the call, so now + 10000 is already in the past by the time it returns. The planner reaches it once per additional condition, so an N-condition query paid N−1 full scans — a 10-condition operation measured ~100s of main-thread stall with the Operations API unresponsive throughout, while the data plane kept answering in 1–3ms.

It now reads the rocksdb.estimate-num-keys property, which is O(1) and returned the identical count (28,668,898) on the affected table.

This is the estimatedEntryCount portion of #2163, which merged to main on 2026-08-31; the helper and intersectionEstimate are taken byte-identical from it. No dependency bump is needed — getEstimatedKeyCount() has been present since rocksdb-js 2.4.x, so v5.1's ~2.4.1 pin already carries it. The statistical range-estimate portion of #2163, which does require rocksdb-js 2.8.0, is deliberately excluded.

The two guards, which are the addition beyond #2163

estimate-num-keys reports 0 for a populated table once its accumulated tombstones reach its non-deletions. The exact count it replaces could not do that — a 0 there meant a genuinely empty store. Measured on rocksdb-js 2.4.1: a store holding 200 live records reports an estimate of 0 after 5000 tombstones for never-written keys, while getKeysCount() still returns 200.

Zero is still the truthful count, so the count is left alone and the two arithmetic sites that cannot take it are guarded instead:

site	with a 0 estimate	guard	what it fixes
AND-group branch divides by it	Infinity	|| 1, the idiom already on the sibling relationship divisor two branches below	plan selection
ne null branch subtracts the index null count from it	negative (e.g. -500)	Math.max(…, 0)	a negative cardinality handed to a caller

The divisor is the one that changes planning. The adaptive filter/index switch derives thresholdRemainingMisses as estimated_count >> 4, and Infinity >> 4 is 0 rather than NaN, so the isNaN check passes it through and the first non-matching record flips the request onto searchByIndex plus a full Set build for the next 10 seconds — the inverse of the plan this change exists to protect. Guarded, the same group estimates 200 and the threshold is a usable 12.

The subtraction does not change planning: -32 and 0 both trip the switch on the first miss. It is guarded because a negative cardinality is not a value to hand to a caller — on main, Table.ts reports estimated_count verbatim as an estimated pagination total, so an empty page could answer Content-Range: */-500. The clamp is at 0, not 1: a table whose rows are all null has a true ne null count of zero, and flooring at 1 would trade a negative total for a phantom row. The same two guards go to main separately, since #2163 left them there.

For the record, my first justification for this second guard was that it protected the adaptive switch. That was wrong — review caught it — and the guard is kept on the narrower, real ground above.

For the human reviewer

1. Estimate accuracy, in both directions — the decision worth your attention. estimate-num-keys counts entries across memtable and SSTs without reconciling overwrites, so it reads high on rewrite-heavy stores and collapses toward 0 once tombstones reach non-deletions. This repo already measured it: DESIGN.md records 37,775 against an exact 4,001 on a real HNSW index store, and concludes it "reads exact on a fresh store with simple puts, so it validates clean in isolation and only misleads on a real index" — which is why HNSW node counting uses a monotonic id counter instead. That matters here because the relationship estimate calls this helper on table.indices[…], which is a RocksIndexStore extends RocksDatabase. The consequence is mis-sized range heuristics and condition ordering that can put a wide range ahead of a narrow indexed equals: plan quality, not correctness, and never wrong rows. It is Use storage-level statistical range estimates in the query planner #2163's trade, already merged on main, and this PR carries it unchanged rather than re-litigating it — but if you want the backport scoped to the primary store only, this is the place to say so. Reverting is one line and reinstates the ~11s-per-condition scan.
2. The four multiplier call sites are deliberately left unguarded. A 0 estimate makes starts_with/between/sort/open-range estimate 1 — finite and positive, so the >> 4 guard still functions; only the divisor and the subtraction produce values it cannot handle. Guarding them would need a representative count, not a floor, which is the hard problem above.
3. Guard at the arithmetic, not in the helper. The first draft floored inside estimatedEntryCount. That is wrong on main, where Table.ts returns the value verbatim as the estimated pagination total for a condition-free query — an empty table would have answered Content-Range: */1 with zero rows, and the clamp below it only ever raises a total. v5.1 has no such consumer, but the branches should not disagree about what the accessor means. A unit test pins the count as 0.
4. Two different idioms for the same invariant (|| 1 on the divisor, Math.max(…, 0) on the subtraction) reads inconsistent. It is deliberate: || 1 cannot catch a negative, and the two sites want different floors — 1 for a divisor, 0 for a cardinality.
5. Residual, deliberately not fixed here. estimated_count >> 4 is not a safe way to derive a threshold from an unbounded estimate. It wraps negative above 2³¹ (1e12 >> 4 === -45461248), and its isNaN guard is dead code either way — NaN >> 4 is 0, so the check can never fire and a non-numeric estimate silently becomes threshold 0. Every pathology in this PR passes through that one expression. Review also surfaced a separate pre-existing defect in the same accumulator, which this PR does not touch: a zero-match condition followed by a full-scan comparator makes it NaN (0 * Infinity), and isFinite(NaN) being false then discards every estimate accumulated before it. The || 1 divisor is not involved — it is NaN for any divisor. Fixing that, or clamping an intersection to min(|A|,|B|) as set semantics require, is a planner-wide behavior change and wants its own PR on main.
6. The ne null subtraction now mixes precisions. Its subtrahend, getValuesCount(null), is an exact index range count, while the minuend is now approximate — before this change both came from the same exact count. On a table after a bulk delete/reinsert cycle the estimate can fall below the live null count, so ne null clamps to 0 and the planner ranks it as the most selective condition. Worth knowing, though it is not made worse by the clamp: a negative sorted even more selective than 0. Same root as item 1.

Verification

Unit tests

unitTests/resources/estimatedEntryCount.test.js — 6 tests. All 6 pass under RocksDB; under HARPER_STORAGE_ENGINE=lmdb 5 pass and the RocksDB-specific one is pending. That test gates on store instanceof RocksDatabase rather than on typeof store.getEstimatedKeyCount, so a renamed or dropped binding method fails the guard loudly instead of silently skipping the one test that covers it. It asserts the returned estimate against an [N/2, 2N] band rather than comparing two live probes, so a background compaction between them cannot redden CI.

Fails on base

Re-verified at the final head with rm -rf dist before each build, because tsc incremental will otherwise report a pass against stale output. The base is origin/v5.1 with only the export keyword added, so the export is not what fails and behavior is the only variable:

assertion	on base
does not iterate the key space	estimatedEntryCount must not iterate the whole key space
intersectionEstimate stays finite	Infinity !== 35
ne null estimate stays non-negative	-500 !== 0
AND-group estimate stays finite	Infinity !== 200

4 of 6 fail on base. The other two pass there and should: the memo test is a regression guard, and the "count is reported as 0" test guards against the wrong fix rather than proving this one.

Dependency claim

A review leg raised this as a blocker on the grounds that it could not be checked offline, so to be explicit about what was actually run: @harperfast/rocksdb-js@2.4.1 was installed from the registry into a scratch project and exercised, not inferred from the version range or from typings. RocksDatabase.prototype.getEstimatedKeyCount is a function; its body (getDBIntProperty('rocksdb.estimate-num-keys') ?? 0) is identical to 2.8.0's; a live store of 5,000 records returned {estimated: 5000, exact: 5000}; and the tombstone case that motivates the guards was measured on that same 2.4.1 build — 200 live rows plus 5,000 tombstones for never-written keys gives {estimated: 0, exact: 200}. CI settles it independently, since it installs under the branch's own range and the new test calls the method directly.

That probe also rules out a BigInt return (which would throw inside Math.max): its result was serialized with JSON.stringify, which throws on a BigInt and did not.

Suites

test:unit:resources — 1001 passing, 3 failing, all in sourceApplyConflictRetry.test.js. Those 3 reproduce identically on unmodified origin/v5.1: their premises assume the branch's pinned ~2.4.1 and this worktree resolves 2.8.0 through a shared node_modules. test:unit:main — 2987 passing, 18 failing, all in globalIsolation.test.js and resolvePreload.test.js, which reproduce on the pristine main checkout too (macOS resolves the temp dir through /private/var). CI is the authoritative run for both.

Not covered

The tests prove no Infinity or negative escapes into a plan estimate. They do not prove the resulting plan is the better one on a churned table, and — per the DESIGN.md note above — the 2,000-row fixture is a fresh store with simple puts, which is precisely the profile the estimate reads accurately on. Neither the rewrite-inflated nor the tombstone-collapsed real-store case is covered; both need an end-to-end query against a churned table and are additive to this change.

Complexity: medium

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=7 @ 52e8c09}

_{Human-Review-Need: 4 (decisions: zero-floor-placement, estimate-vs-exact-for-ne-null, export-helper-for-testing, engine-gated-skip) @ 52e8c09}

[gemini-code-assist]

Code Review

This pull request optimizes the key-count estimation for RocksDB by switching from the expensive getKeysCount() to the O(1) getEstimatedKeyCount(). It also addresses potential division-by-zero and negative estimate issues in query estimation, and adds a comprehensive test suite to cover these scenarios. The reviewer suggests adding a defensive null/undefined check to the newly exported estimatedEntryCount function to prevent runtime errors.

[kriszyp]

CI note: Integration Tests 1/6 (Node.js v26) and Integration Tests 6/6 (Node.js v26) are red here, and they are red on the base rather than because of this change.

The most recent v5.1 branch run — 33649894873, from the merge of #2386 earlier today, which does not contain this commit — fails on exactly those two jobs and no others. Every other job on this PR passes, including all three Unit Test matrix entries, which are the ones that exercise the new estimatedEntryCount suite.

Those unit jobs also settle the dependency question independently of my local checks: CI installs under this branch's own ~2.4.1 range, and the new test calls getEstimatedKeyCount() directly, so it would have gone red immediately if the method were not present in 2.4.x.

Not re-running the two red jobs, and not treating them as this PR's to fix — flagging so the base failure is not mistaken for a regression here.

— Claude Opus 5

[claude]

Reviewed; no blockers found.