* test: add failing test for values_per_hash drift on duplicate geo point removal
Reproduces the bug where `remove_point` only calls `decrement_hash_value_counts`
once per unique geohash, while `add_many_geo_points` increments it once per value.
When a point has duplicate geo coordinates (same geohash), the counters drift
upward permanently after removal.
Ref: https://github.com/qdrant/qdrant/pull/9033#discussion_r3241154045
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: decrement values_per_hash once per value on geo point removal
`add_many_geo_points` increments `values_per_hash` once per value, but
`remove_point` deduplicated geohashes with a HashSet + `continue` that
also skipped the per-value decrement. A point with duplicate geo
coordinates (same geohash) therefore left the counters drifted upward
permanently after removal.
Move `decrement_hash_value_counts` above the dedup guard so it runs once
per value, matching the increment side. `points_map` and
`points_per_hash` track points, not values, so they stay deduplicated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: generall <andrey@vasnetsov.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: read only full text
* fix: linter
* refactor: follow new file structure
* chore: remove storage enums
* fix: linter
* fix: FullTextReadIndex methods
* chore: remove duplicated impls
* fix: format
* refactor: unify FullTextIndex telemetry via trait default method
Add a `get_telemetry_data` default method to `FullTextIndexRead` built
from the existing telemetry methods, so the `ReadOnlyFieldIndex` match
arm is consistent with every other variant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: move full-text lifecycle methods off FullTextIndexRead trait
`populate`, `clear_cache`, `files` and `immutable_files` are lifecycle
concerns, not part of the read surface. Move them from the
`FullTextIndexRead` trait to inherent methods alongside the rest of each
index's lifecycle code (open / wipe / flusher). Also split the
`mutable_text_index` tests into their own `tests.rs`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: restructure full_text_index module layout
Move the `FullTextIndex` enum into `full_text_index/mod.rs` and split its
impls following the inner-module convention: lifecycle (constructors,
builders, ValueIndexer, PayloadFieldIndex) into `lifecycle.rs`, read-path
impls and shared free functions into `read_ops.rs`. Extract the
`FullTextIndexRead` trait into its own `full_text_index_read.rs`.
Drops the inherent `FullTextIndex::get_telemetry_data`, which duplicated
the `FullTextIndexRead::get_telemetry_data` default method.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: read-only counterpart of MutableFullTextIndex
Extract the shared in-memory state of `MutableFullTextIndex` into
`MutableFullTextIndexInner` (inverted index, config, tokenizer), which
implements `FullTextIndexRead`. `MutableFullTextIndex` now wraps it
alongside a writable `Gridstore`.
Add `ReadOnlyAppendableFullTextIndex<S>`, wrapping the same inner state
with a `GridstoreReader` over generic `UniversalRead`. Mirrors the
`MutableMapIndex` / `ReadOnlyAppendableMapIndex` split; loading and
lifecycle land in a follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: turn ReadOnlyFullTextIndex into a two-variant enum
Rename the `read_only_text_index` module to `read_only` and reshape
`ReadOnlyFullTextIndex` from a thin `MmapFullTextIndex` wrapper into an
`Appendable` / `Immutable` enum, mirroring `ReadOnlyMapIndex`. Its
`FullTextIndexRead` impl is now a dispatcher forwarding to the active
variant.
Drop the no-op inherent `populate` (mutable, immutable) and
`immutable_files` (mutable) methods that tripped `clippy::unused_self`
and `clippy::unnecessary_wraps` after the earlier trait-to-inherent
move; `FullTextIndex::populate` inlines the in-RAM no-op directly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: generall <andrey@vasnetsov.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: split numeric index variants into dedicated modules
Move MutableNumericIndex, ImmutableNumericIndex, and MmapNumericIndex
into their own directories, each split into mod.rs (struct definitions),
lifecycle.rs (open/build/wipe/mutations), and read_ops.rs (accessors),
mirroring the map_index layout.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: drop single-variant Storage enums in mutable/immutable numeric index
Replace `Storage<T>` wrappers around the only backing store with the
store types directly: `Gridstore<Vec<T>>` for `MutableNumericIndex` and
`Box<MmapNumericIndex<T>>` for `ImmutableNumericIndex`. Collapses the
trivial single-arm matches into direct method calls.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: introduce NumericIndexRead trait
Mirror `MapIndexRead` from the map_index refactor: define a
`NumericIndexRead<T>` trait in `numeric_index/read_ops.rs` and implement
it on each of the three storage variants
(`MutableNumericIndex`, `ImmutableNumericIndex`, `MmapNumericIndex`).
Trait signatures are unified across variants — in-memory variants accept
and ignore the `hw_counter` argument that the mmap-backed variant uses
for IO tracking, and `total_unique_values_count`, `values_range`, and
`orderable_values_range` return `OperationResult` everywhere so the
dispatcher in `NumericIndexInner` can call them generically.
Variant-specific helpers that don't fit the shared shape stay as
inherent methods: `MutableNumericIndex::map()`,
`ImmutableNumericIndex::values_range_size()`, and
`MmapNumericIndex::{values_range_size, is_on_disk}`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: add ReadOnlyAppendableNumericIndex
Counterpart to `MutableNumericIndex`, mirroring `ReadOnlyAppendableMapIndex`
from the map_index refactor. It reuses the shared `InMemoryNumericIndex`
in-memory state but is backed by a `GridstoreReader` over generic
`UniversalRead` instead of a writable `Gridstore`, and implements
`NumericIndexRead` by forwarding to the in-memory index — no mutation
surface.
Loading / lifecycle (constructor, files, populate, clear_cache) will
follow in a separate change; the storage field is held only to pin the
on-disk layout for now.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: rename MmapNumericIndex to UniversalNumericIndex, expose UniversalRead param
Mirror `UniversalMapIndex`: the type is now generic over `S: UniversalRead`
with a `MmapFile` default, so the index can be served from any
`UniversalRead` backend (io_uring, disk-cache wrappers, …) rather than
the hard-coded `MmapFile`.
The `NumericIndexRead` impl and read-side helpers are generic over `S`;
`build` / `open` and the other lifecycle methods stay `MmapFile`-only
since they construct mmap-backed storage from a path. The
`NumericIndexInner::Mmap` enum variant keeps its name and uses the
default `S = MmapFile`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: split numeric_index/storage/mod.rs into lifecycle and read_ops
`storage/mod.rs` now holds only the `NumericIndexInner` enum and module
wiring. The variant-dispatch impls are split into sibling modules
matching the layout of the individual storage variants:
- `lifecycle.rs`: construction, persistence, file listing, cache
control, and `remove_point`.
- `read_ops.rs`: read-path forwarding — value lookups, telemetry, RAM
accounting, `is_on_disk`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: implement NumericIndexRead for NumericIndexInner
The enum-level read dispatch was a set of inherent methods scattered
across storage/read_ops.rs and storage/statistics.rs with signatures
that drifted from the variant trait (`values_count` returned `usize`,
`max_values_per_point` vs `get_max_values_per_point`, a hand-rolled
`get_telemetry_data`). Make `NumericIndexInner` implement
`NumericIndexRead` directly so it shares one interface with the three
storage variants.
- All 12 trait methods are forwarded via match dispatch in
storage/read_ops.rs; `values_range` / `orderable_values_range` box
the per-variant iterators.
- `get_histogram`, `get_points_count`, `total_unique_values_count` move
out of statistics.rs into the trait impl; `values_is_empty` and
`get_telemetry_data` now come from the trait defaults.
- `point_ids_by_value` and `is_on_disk` stay as enum-only inherent
helpers (not part of the shared trait).
- Callers updated: `NumericIndex::values_count` unwraps the now
`Option`-returning trait method; `filter` boxes `point_ids_by_value`;
`field_index.rs` and `numeric_field_index.rs` import the trait.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: add ReadOnlyNumericIndexInner
Read-only counterpart to `NumericIndexInner`, mirroring `ReadOnlyMapIndex`
from the map_index refactor. Lives under
`numeric_index/storage/read_only` and selects across the two read-only
storage backends:
- `Appendable(ReadOnlyAppendableNumericIndex<T, S>)` — loaded into RAM
from the appendable Gridstore format.
- `Immutable(UniversalNumericIndex<T, S>)` — served directly from the
immutable stored format.
Implements `NumericIndexRead` by forwarding each method to the active
variant; `values_is_empty` / `get_telemetry_data` come from the trait
defaults.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: rename numeric_index read_ops to numeric_index_read, split mod.rs
Two changes:
- Rename `numeric_index/read_ops.rs` (the `NumericIndexRead` trait
definition) to `numeric_index_read.rs`, freeing the `read_ops` name.
- Split the leftover content of `numeric_index/mod.rs` into sibling
modules, matching the per-variant layout:
- `lifecycle.rs`: the `Encodable` key-format trait + impls and the
`HISTOGRAM_*` construction constants.
- `read_ops.rs`: the `StreamRange` trait and the `Range` →
index-key-bounds conversion.
`mod.rs` now only wires modules and re-exports. `Encodable` and
`StreamRange` keep their public paths via re-export; `tests.rs` gains
explicit imports for the symbols it previously picked up through the
`mod.rs` glob.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: dissolve numeric_index/index.rs into mod, lifecycle, read_ops
`index.rs` only held the `NumericIndex` wrapper and its two impl blocks;
spread them to match the per-module layout used elsewhere:
- `NumericIndex` struct + `NumericIndexIntoInnerValue` trait → `mod.rs`
(type definitions live with the module wiring).
- The inherent `impl NumericIndex` (open / build / cache control /
storage introspection) → `lifecycle.rs`, alongside the `HISTOGRAM_*`
seed constants.
- The `PayloadFieldIndexRead` impl → `read_ops.rs`.
Also move the `Encodable` key-format trait out of `lifecycle.rs` into
its own `encodable.rs`. `mod.rs` keeps re-exporting `Encodable` so its
public path is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: add ReadOnlyNumericIndex with NumericIndexRead + PayloadFieldIndexRead
Read-only counterpart to `NumericIndex`, wrapping `ReadOnlyNumericIndexInner`
plus the payload value type parameter `P`. Implements both `NumericIndexRead`
and `PayloadFieldIndexRead` by forwarding to the inner storage-variant enum.
To support `PayloadFieldIndexRead` without duplicating the query logic, the
cardinality/filter/payload-block/condition-checker code is extracted into a
new `query` module of generic free functions over `NumericIndexRead<T>`.
`ReadOnlyNumericIndexInner` implements `PayloadFieldIndexRead` by plugging
into those helpers; `ReadOnlyNumericIndex` delegates to its inner.
The writable `NumericIndexInner` path is left untouched — its existing
variant-specialized `estimate_points` heuristic stays in `storage`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: make query.rs the single source of truth for numeric index queries
The generic `query` helpers and the per-`NumericIndexInner` impls in
`storage/{trait_impls,statistics}.rs` had duplicated cardinality /
filter / payload-block / condition-checker logic. Collapse them onto
the shared `query` helpers:
- `storage/trait_impls.rs`: `PayloadFieldIndexRead for NumericIndexInner`
now forwards each method to `query::*` instead of carrying its own copy.
- `storage/statistics.rs`: deleted — `range_cardinality` and
`estimate_points` were duplicates of the `query` versions.
- `estimate_points` needs a range size; add `values_range_size` to the
`NumericIndexRead` trait with a default that counts `values_range`,
overridden by the `Immutable` / `Mmap` variants with their `O(log n)`
boundary search. The `MutableNumericIndex::map()` accessor (its only
caller was the old `estimate_points`) is removed.
- `values_range_size` takes `hw_counter` and threads it into
`values_range` rather than fabricating a disposable counter.
`tests.rs` calls `query::range_cardinality` directly now that the
inherent method is gone.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: integrate read-only numeric index into ReadOnlyFieldIndex
Wire the four numeric variants (`IntIndex`, `DatetimeIndex`,
`FloatIndex`, `UuidIndex`) into `ReadOnlyFieldIndex`, mirroring
`FieldIndex`:
- `PayloadFieldIndexRead` / `FieldIndexRead` dispatch covers the new
variants — telemetry, value counts, value retrievers, `as_numeric`.
- `ReadOnlyNumericFieldIndex` is the read-only counterpart of
`NumericFieldIndex` (Int/Float order-by erasure over
`ReadOnlyNumericIndexInner`); `as_numeric` returns it for the
Int/Datetime/Float variants (UUIDs aren't numerically order-by-able,
matching `FieldIndex`).
- `ReadOnlyNumericIndex` gains per-`(T, P)` `value_retriever` methods
(in `read_only/value_retriever.rs`) and an `inner()` accessor.
`StreamRange` is now backed by a shared generic `query::stream_range`
helper over `NumericIndexRead`, implemented for both `NumericIndexInner`
and `ReadOnlyNumericIndexInner` — replacing the bespoke `EitherVariant`
dispatch in `storage/trait_impls.rs`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: collapse ReadOnlyNumericIndex value retrievers onto one generic method
The four per-`(T, P)` `value_retriever` methods were identical except
for the per-value `T -> Value` conversion. Extract that conversion into
a `NumericValueToJson` trait (one tiny impl per `(T, P)`) and keep a
single generic `value_retriever` that builds the retriever closure once.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fmt
* refactor: dedup NumericFieldIndex / ReadOnlyNumericFieldIndex
The two enums were structurally identical — same `StreamRange`,
`get_ordering_values`, and `NumericFieldIndexRead` bodies — differing
only in the backing storage type. Collapse them onto one generic
`NumericFieldIndexView<'a, I, F>` with a single set of impls (over
`I: NumericIndexRead<i64> + StreamRange<i64>` and the `f64` counterpart).
`NumericFieldIndex` and `ReadOnlyNumericFieldIndex` are now type aliases
of the generic view, so every existing call site is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: read only geo index
* fix: linter
* fix: drop open temp
* fix: read_ops
* fix: address review fixes and use same file structure like map index
* fix: review comments
* fix: linter
* rename mmap -> universal
* [AI] Introduce `ReadOnlyMapIndex` and implement `MapIndexRead` for it
* add missing file
* [AI] Implement read-only PayloadFieldIndexRead / FieldIndexRead (#9026)
Wires `ReadOnlyFieldIndex` into the read-only field-index surface that
`ReadOnlyMapIndex` and `ReadOnlyNullIndex` already provide, sharing the
non-trivial bodies with the existing `MapIndex` impls.
- Extracted per-`N` `PayloadFieldIndexRead` bodies (filter,
estimate_cardinality, for_each_payload_block, condition_checker) in
`payload_index_impl/{str,int,uuid}.rs` into free functions over
`T: MapIndexRead<N>`. Both `MapIndex<N>` and `ReadOnlyMapIndex<N, S>`
now impl `PayloadFieldIndexRead` via thin delegation to those bodies.
- Lifted `value_retriever` bodies in `value_indexer_impl.rs` the same
way; `ReadOnlyMapIndex<N, S>` gets the per-K closure for free.
- Added `telemetry_index_type` (required) + `get_telemetry_data`
(default) to `MapIndexRead`, mirroring the `NullIndexRead` pattern;
every map-index variant now reports telemetry without an inherent
method on the enum.
- Promoted `MapIndexRead` + its module to `pub` so
`field_index_base/read_only/` can name them.
- Implemented `FacetIndex for ReadOnlyMapIndex<N, S>` and extended
`FacetIndexEnum` with `S: UniversalRead = MmapFile` plus the three
read-only variants. `FieldIndex::as_facet_index` pins
`S = MmapFile` via turbofish to keep inference happy.
- `ReadOnlyFieldIndex::as_numeric` returns `None` for now; no concrete
read-only numeric type exists yet (the doc on `NumericFieldIndexRead`
already notes this is intended for a future `ReadOnlySegment`).
The geo_index module was a single 2134-line file. Split it into focused
files for better readability and maintainability:
- mod.rs: GeoMapIndex enum definition and core methods (~290 lines)
- builders.rs: GeoMapIndexMmapBuilder and GeoMapIndexGridstoreBuilder
- payload_index.rs: ValueIndexer, PayloadFieldIndex, PayloadFieldIndexRead impls
- tests.rs: all test code (~1340 lines)
No logic changes; all 40 existing tests pass.
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(map_index): extract MutableMapIndexInner; add ReadOnlyAppendableMapIndex
Extract the four in-memory fields (`map`, `point_to_values`, `indexed_points`,
`values_count`) of `MutableMapIndex<N>` into a shared inner struct
`MutableMapIndexInner<N>` under `mutable_map_index/inner.rs`. The
`MapIndexRead<N>` impl moves wholesale onto the inner. `MutableMapIndex<N>`
becomes a thin wrapper `{ inner, storage: Gridstore<...> }`; its
`MapIndexRead<N>` impl forwards to the inner. The Gridstore load loop in
`open_gridstore` collapses to `MutableMapIndexInner::empty()` +
`inner.ingest(idx, value)` per row, sharing the per-value ingestion path with
the future read-only loader.
Add a new sibling `mutable_map_index/read_only/` module containing
`ReadOnlyAppendableMapIndex<N>` (inner + `GridstoreReader<Vec<...>, MmapFile>`),
with its `MapIndexRead<N>` impl forwarding to the same inner. No constructor
yet — loading and lifecycle for the read-only variant will land in a
follow-up. Field `storage` is held to pin the on-disk type and tagged
`#[allow(dead_code)]` until then.
No behavioural change. The 17 existing map_index tests pass unchanged.
* expose storage generic
* refactor(map_index): split mod.rs into read_ops, lifecycle, tests
mod.rs is now ~40 lines containing only the MapIndex enum, type
aliases, and module declarations.
- read_ops.rs: read-only inherent methods (get_values, get_iterator,
for_each_*, except_cardinality, except_set, telemetry, ram_usage,
mutability/storage type)
- lifecycle.rs: open/builder/flush/wipe/remove_point/files/populate/
clear_cache
- tests.rs: all #[cfg(test)] tests
Also folded payload_index_impl_{int,str,uuid}.rs into a dedicated
payload_index_impl/ submodule.
* refactor(map_index): split storage submodules into dedicated dirs (#9016)
* refactor(map_index): split storage submodules into dedicated module dirs
Turn each of the three storage implementation files into a directory
module split into read_ops + lifecycle, mirroring the parent module
layout introduced in #9015.
- mutable_map_index/{mod,lifecycle,read_ops}.rs
- immutable_map_index/{mod,lifecycle,read_ops}.rs
- mmap_map_index/{mod,lifecycle,read_ops}.rs
Each mod.rs holds only the struct definitions, internal Storage type,
and config/constants. read_ops.rs holds the read-only query methods;
lifecycle.rs holds open/build/flush/wipe/files/remove_point and
internal mutation helpers.
Pure refactor — no behavior change.
* refactor(map_index): introduce MapIndexRead trait
Define a unified read-only trait `MapIndexRead<N>` describing the
methods every storage variant exposes (check_values_any, get_values,
get_iterator, for_each_*, storage_type, ram_usage_bytes, etc.).
Each storage variant's read_ops.rs now contains a trait impl instead
of inherent methods. Signatures are unified across variants:
- `hw_counter` is accepted by every method that needs it for the mmap
variant; mutable / immutable accept and ignore it.
- `check_values_any` returns `bool` (mmap absorbs IO errors internally
with the existing FIXME, matching the parent's prior `.unwrap_or`).
- `for_each_count_per_value` takes `deferred_internal_id` uniformly;
the immutable variant `debug_assert!`s it is `None`.
`for_points_values` keeps its variant-specific callback signatures
and stays as an inherent method — it's only used by FacetIndex with
explicit pattern matching.
Pure refactor — no behavior change.
* refactor(mutable_map_index): drop single-variant Storage enum
The `Storage<T>` enum had only one variant (`Gridstore`), so every
`match &self.storage { Storage::Gridstore(s) => ... }` was just
unwrapping the same path. Replace the field with `Gridstore<Vec<...>>`
directly and inline every match.
The ~2150-line map_index/mod.rs was difficult to navigate. Split it into
focused files while preserving all logic and public API:
- key.rs: MapIndexKey trait and impls for str, IntPayloadType, UuidIntType
- builders.rs: MapIndexBuilder, MapIndexMmapBuilder, MapIndexGridstoreBuilder
- payload_index_impl_str.rs: PayloadFieldIndex/Read for MapIndex<str>
- payload_index_impl_int.rs: PayloadFieldIndex/Read for MapIndex<IntPayloadType>
- payload_index_impl_uuid.rs: PayloadFieldIndex/Read for MapIndex<UuidIntType>
- facet_index_impl.rs: FacetIndex impl for MapIndex<N>
- value_indexer_impl.rs: ValueIndexer impls and value_retriever methods
mod.rs retains the MapIndex enum, its core inherent methods, type aliases,
constants, and tests.
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(field_index): make FieldIndexRead a supertrait of PayloadFieldIndexRead
Removes the `get_payload_field_index_read() -> &dyn PayloadFieldIndexRead`
bridge from `FieldIndexRead` and the five default impls that forwarded
through it. The overlapping read methods now come from the supertrait
directly, eliminating one layer of dynamic dispatch on the hot read path:
`FieldIndex::filter` → variant match → concrete typed-index method.
`FieldIndex` gains a direct `impl PayloadFieldIndexRead` block with
per-method match arms, mirroring the existing dispatch shape used by
`get_telemetry_data`, `values_count`, etc.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: apply nightly rustfmt
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(numeric_index): split mod.rs into focused submodules (#8997)
* refactor(numeric_index): split mod.rs into focused submodules
`numeric_index/mod.rs` was 1308 lines mixing the storage-dispatch enum,
the public `NumericIndex<T, P>` wrapper, three builders, per-(T, P)
`ValueIndexer` impls, and the `Encodable`/`StreamRange` traits — hard
to navigate.
Split into:
- `mod.rs` keeps the shared traits (`Encodable`, `StreamRange`),
`Range<T>::as_index_key_bounds`, and re-exports.
- `wrapper.rs` — `NumericIndex<T, P>` + inherent impl + `NumericIndexIntoInnerValue` trait.
- `builders.rs` — `NumericIndexBuilder`, `NumericIndexMmapBuilder`, `NumericIndexGridstoreBuilder`.
- `value_indexer.rs` — `ValueIndexer` and per-(T, P) `value_retriever` inherent impls.
- `storage/` — the `NumericIndexInner` dispatch enum:
- `storage/mod.rs` — enum + simple match-and-forward (constructors, lifecycle, telemetry, per-point access).
- `storage/statistics.rs` — histogram-driven cardinality and point-count helpers.
- `storage/trait_impls.rs` — `PayloadFieldIndex`, `PayloadFieldIndexRead`, `StreamRange` impls.
Pure code reorganization — no behavior change. A few inherent methods
on `NumericIndexInner` had to widen from private to `pub(super)` /
`pub(in crate::index::field_index::numeric_index)` to remain reachable
across the new module boundaries (and from `tests.rs`); the new
constructors (`NumericIndexMmapBuilder::new`,
`NumericIndexGridstoreBuilder::new`) replace direct field
construction across files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(numeric_index): rename wrapper.rs -> numeric_index.rs, move point_ids_by_value out of statistics
- Renamed `wrapper.rs` to `numeric_index.rs`, matching the central
`NumericIndex` type and the surrounding module name.
- Moved `point_ids_by_value` from `storage/statistics.rs` to
`storage/mod.rs` next to `get_values`. It is an exact value->points
lookup primitive, not a cardinality estimate; the statistics module
is left to the histogram-driven helpers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(clippy): rename numeric_index submodule to index to fix module_inception lint
Clippy's module_inception rule disallows a module with the same name as
its containing module. Rename numeric_index.rs -> index.rs and update
the three internal references.
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(field_index): push PayloadFieldIndexRead through NumericIndex and move special_check_condition per-variant (#8998)
Two related cleanups that share a theme — drop enum-level read dispatch
in favor of per-variant trait impls:
1. `NumericIndex<T, P>` now implements `PayloadFieldIndexRead` directly
(forwarding to its inner storage enum). The four numeric arms in
`FieldIndex`'s `impl PayloadFieldIndexRead` drop their `.inner()`
calls, so all eleven variants now use a uniform `idx.<method>(...)`
form.
2. `special_check_condition` moves from `FieldIndexRead` to
`PayloadFieldIndexRead` with a default `Ok(None)` body.
`FullTextIndex` (the only variant with non-trivial logic) overrides
it. `NumericIndex<T, P>` forwards through to the inner enum, and
the `FieldIndex` enum's match dispatch moves out of `FieldIndexRead`
into `PayloadFieldIndexRead` for consistency with the other five
trait methods. Removed the now-redundant declaration from
`FieldIndexRead` (inherited via the supertrait).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* Callback-based InvertedIndex::for_each_vocab_with_postings_len
* Integrate UniversalHashMap into full-text index
* Integrate UniversalHashMap into immutable_map_index
The read view's `field_indexes` was concretely typed as
`&IndexesMap = &HashMap<PayloadKeyType, Vec<FieldIndex>>`. Replace
the FieldIndex with a generic parameter F: FieldIndexRead, so the
view advertises a dependency on the trait surface rather than the
concrete enum. `StructPayloadIndex::with_view` instantiates
`F = FieldIndex`; any future consumer that implements
FieldIndexRead (e.g. a read-only segment with a narrower
field-index representation) can plug in directly.
Changes:
1. StructPayloadIndexReadView gains a fourth generic parameter F
bound by FieldIndexRead. `field_indexes` is now
`&HashMap<PayloadKeyType, Vec<F>>`. All five impl blocks across
read_view/ propagate the F generic.
2. `check_field_condition` / `select_nested_indexes` /
`check_payload` in payload_storage/query_checker.rs become
generic over `FI: FieldIndexRead` (and `R: AsRef<Vec<FI>>`).
The bodies only call `special_check_condition` which is on
FieldIndexRead, so the generalization is free.
3. `variable_retriever` in read_view/value_retriever/helpers.rs
gains an F generic; the body already only uses FieldIndexRead
methods after the previous commits in this PR.
4. `with_view` and `SegmentReadViewFor` instantiate F = FieldIndex.
No external consumer breaks.
5. Tests construct the view with an inferred F.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The read view's `field_indexes` was concretely typed as
`&IndexesMap = &HashMap<PayloadKeyType, Vec<FieldIndex>>`. Replace
the FieldIndex with a generic parameter F: FieldIndexRead, so the
view advertises a dependency on the trait surface rather than the
concrete enum. `StructPayloadIndex::with_view` instantiates
`F = FieldIndex`; any future consumer that implements
FieldIndexRead (e.g. a read-only segment with a narrower
field-index representation) can plug in directly.
Changes:
1. StructPayloadIndexReadView gains a fourth generic parameter F
bound by FieldIndexRead. `field_indexes` is now
`&HashMap<PayloadKeyType, Vec<F>>`. All five impl blocks across
read_view/ propagate the F generic.
2. `check_field_condition` / `select_nested_indexes` /
`check_payload` in payload_storage/query_checker.rs become
generic over `FI: FieldIndexRead` (and `R: AsRef<Vec<FI>>`).
The bodies only call `special_check_condition` which is on
FieldIndexRead, so the generalization is free.
3. `variable_retriever` in read_view/value_retriever/helpers.rs
gains an F generic; the body already only uses FieldIndexRead
methods after the previous commits in this PR.
4. `with_view` and `SegmentReadViewFor` instantiate F = FieldIndex.
No external consumer breaks.
5. Tests construct the view with an inferred F.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): add condition_checker trait method (foundation)
First PR in the condition-checker-migration sub-plan
(docs/plans/field-index-read-trait/condition-checker-migration/).
Purely additive: every existing typed index keeps the default `None`
impl, so the legacy match in `field_condition_index` handles every
condition. Behaviour unchanged.
Changes:
1. PayloadFieldIndexRead::condition_checker — new method with default
`None` body. Lets each typed index advertise the conditions it can
serve.
2. FieldIndexRead::get_payload_field_index_read — new required method
returning `&dyn PayloadFieldIndexRead`. Distinct from the existing
inherent `get_payload_field_index` (which returns `&dyn PayloadFieldIndex`,
exposing write-side methods) — the read path now hands back a
handle that can't accidentally call wipe / flusher / files.
3. Five pure-delegation methods on FieldIndexRead get default impls
using the new getter (count_indexed_points, filter,
estimate_cardinality, for_each_payload_block, condition_checker).
`impl FieldIndexRead for FieldIndex` drops their explicit bodies.
4. field_condition_index in read_view/condition_converter/helpers.rs
tries `index.condition_checker(...)` first, then falls through to
the existing legacy match. The match handles everything until
per-variant overrides land in the follow-up PRs.
Why `&dyn` rather than `impl PayloadFieldIndexRead + '_`: RPITIT
requires a single concrete return type, but the per-variant arms
produce `&NumericIndexInner<T>`, `&MapIndex<K>`, `&GeoMapIndex`, etc.
— different types. A unifying borrowed-enum (à la FacetIndexEnum)
would be more code than the trait-object indirection saves.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): migrate GeoMapIndex into condition_checker (#8979)
* refactor(field_index): migrate GeoMapIndex condition checking into condition_checker
Sub-plan PR 1 of the condition-checker-migration. GeoMapIndex now
serves geo conditions (geo_radius, geo_bounding_box, geo_polygon)
directly through PayloadFieldIndexRead::condition_checker. The
corresponding match arms and three helper functions in the read
view's helpers.rs are gone — the polymorphic dispatch in
field_condition_index handles them.
Changes:
1. impl PayloadFieldIndexRead for GeoMapIndex gains a condition_checker
override. Bodies copied from get_geo_radius_checkers /
get_geo_bounding_box_checkers / get_geo_polygon_checkers; the
`&'a self` capture replaces the &FieldIndex match arm.
2. The override destructures FieldCondition explicitly (no `..`,
every field named with either a binding or `_:`). This forces a
compile error if a new field is added to FieldCondition, so each
index variant must explicitly decide whether to handle it. Same
pattern is mandated for the remaining variant migrations — see
docs/plans/field-index-read-trait/condition-checker-migration/00-overview.md.
3. read_view/condition_converter/helpers.rs drops the three Geo
FieldCondition arms from field_condition_index's legacy match
and deletes the three helper functions. The catch-all is relaxed
to `geo_radius: _ | geo_bounding_box: _ | geo_polygon: _` since
the trait method handles all geo cases now.
No behavioural change: non-Geo variants continue to return None for
geo conditions (the trait default returns None), and the read view's
catch-all still falls back to the payload check.
Stacked on #8978 (condition-checker-trait foundation).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): migrate NullIndex into condition_checker (#8980)
* refactor(field_index): migrate NullIndex condition checking into condition_checker
Sub-plan PR 2 of the condition-checker-migration. NullIndex now
serves is_empty / is_null conditions directly through
PayloadFieldIndexRead::condition_checker. The dispatch in the read
view's condition_converter/mod.rs collapses to mirror the
Condition::Field arm: try every index for the field, fall back to
the payload check on unwrap_or_else.
Changes:
1. impl PayloadFieldIndexRead for NullIndex gains a condition_checker
override. Explicit FieldCondition destructure (no `..`) — binds
is_empty and is_null, every other field uses `_:` to document
intentional non-handling.
2. read_view/condition_converter/helpers.rs drops the is_empty /
is_null arms from field_condition_index's legacy match and deletes
five orphaned helpers: get_is_empty_checker, get_is_null_checker,
get_is_empty_indexes, get_null_index_is_empty_checker,
get_fallback_is_empty_checker.
3. read_view/condition_converter/mod.rs rewrites Condition::IsEmpty
and Condition::IsNull to use field_condition_index with a
constructed FieldCondition, same shape as Condition::Field.
Behavioural note (decision locked during review): The legacy
values_is_empty fast-path for "field has indexes but no NullIndex"
is dropped. NullIndex was introduced in v1.13.5 (#6088); we're on
v1.18.0, so any collection touched in the past ~5 minor releases has
a NullIndex. Segments untouched since pre-v1.13.5 fall through to the
payload check on is_empty queries — no longer accelerated by another
index's values_is_empty. A re-index recovers the original perf.
Stacked on #8979 (GeoMapIndex) → #8978 (foundation) → dev.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): migrate NumericIndex into condition_checker (#8981)
* refactor(field_index): migrate NumericIndex condition checking into condition_checker
Sub-plan PR 3 of the condition-checker-migration. NumericIndexInner<T>
now serves Range conditions directly through
PayloadFieldIndexRead::condition_checker. The generic impl handles all
four FieldIndex variants (IntIndex, FloatIndex, DatetimeIndex,
UuidIndex) with one trait override.
Changes:
1. impl PayloadFieldIndexRead for NumericIndexInner<T> gains a
condition_checker override. Explicit FieldCondition destructure
(no `..`) binds `range`. Body converts the RangeInterface bounds
into T via T::from_f64 / T::from_u128 — same conversion `filter`
already uses.
2. read_view/condition_converter/helpers.rs drops the range arm from
field_condition_index's legacy match and deletes
get_range_checkers / get_float_range_checkers /
get_datetime_range_checkers (3 helpers, ~75 lines). Six unused
imports go with them.
Behavioural note: The legacy helpers were stricter than `filter` —
only IntIndex/FloatIndex served Float ranges, only DatetimeIndex
served DateTime ranges; UuidIndex returned None. `filter` already
handles every numeric variant uniformly via T::from_f64 / T::from_u128,
so there was a pre-existing inconsistency between filter (primary-
clause iteration) and condition_checker (post-filter validation).
This PR makes condition_checker match filter — every NumericIndex
variant can now serve any RangeInterface. The schema layer normally
prevents cross-type range queries from reaching here.
Stacked on #8980 (NullIndex) → #8979 (GeoMapIndex) → #8978
(foundation) → dev.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): migrate MapIndex into condition_checker (#8982)
* refactor(field_index): migrate MapIndex condition checking into condition_checker
Sub-plan PR 4 of the condition-checker-migration. MapIndex<K> now
serves Match::Value / Match::Any / Match::Except conditions directly
through PayloadFieldIndexRead::condition_checker, one override per K
(MapIndex<str>, MapIndex<UuidIntType>, MapIndex<IntPayloadType>).
Changes:
1. Three impl blocks gain condition_checker overrides — one per K.
Each uses an explicit FieldCondition destructure (no `..`) and an
exhaustive `Match` variant match (no `_ => None`); a new variant
in FieldCondition, Match, ValueVariants, or AnyVariants forces a
compile error in every relevant place. INDEXSET_ITER_THRESHOLD
small/large branching preserved.
2. read_view/condition_converter/match_converter.rs sheds the Map
arms. get_match_value_checker keeps (Bool, BoolIndex) plus the
explicit 30-tuple None list. get_match_any_checker becomes an
explicit 22-tuple None match (no Bool/FullText/Numeric/Geo/Null
variant ever served Any). get_match_except_checker reduces to the
unconditional values_count > 0 fallback — when no typed index
handles Except (via condition_checker), match any point with at
least one value, since the value can't be in a type-mismatched
list. Unused IndexSet/Uuid/INDEXSET_ITER_THRESHOLD imports drop.
Stacked on #8981 (NumericIndex) → #8980 (NullIndex) → #8979
(GeoMapIndex) → #8978 (foundation) → dev.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): migrate BoolIndex into condition_checker (#8983)
* refactor(field_index): migrate BoolIndex condition checking into condition_checker
Sub-plan PR 5 of the condition-checker-migration. BoolIndex now
serves `Match::Value(Bool)` directly through
PayloadFieldIndexRead::condition_checker.
Changes:
1. impl PayloadFieldIndexRead for BoolIndex gains a condition_checker
override. Explicit FieldCondition destructure (no `..`) plus an
exhaustive `Match` variant match (no `_ => None`). Match::Any /
Match::Except over booleans return None — matches legacy
match_converter behaviour.
2. read_view/condition_converter/match_converter.rs's
get_match_value_checker drops the `(Bool, BoolIndex)` arm; the
function now returns None unconditionally for every (value, index)
pair, with the explicit 33-tuple list preserved so adding a new
ValueVariants or FieldIndex variant forces a compile error.
Stacked on #8982 (MapIndex) → #8981 → #8980 → #8979 → #8978 → dev.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): migrate FullTextIndex into condition_checker (#8984)
* refactor(field_index): migrate FullTextIndex condition checking into condition_checker
Sub-plan PR 6 of the condition-checker-migration. FullTextIndex now
serves Match::Text / Match::TextAny / Match::Phrase directly through
PayloadFieldIndexRead::condition_checker.
Changes:
1. impl PayloadFieldIndexRead for FullTextIndex gains a
condition_checker override. Explicit FieldCondition destructure
(no `..`) plus exhaustive Match variant matching (no `_ => None`).
Match::Value / Match::Any / Match::Except all return None
explicitly. Body copies parse_text_query / parse_phrase_query /
parse_text_any_query dispatch and the check_match closure from the
legacy helper, preserving the FIXME-marked error swallowing.
2. read_view/condition_converter/match_converter.rs loses
get_match_text_checker and the TextQueryType enum (both
unused after this PR). get_match_checkers' Text/TextAny/Phrase
arms collapse to a single explicit `None` arm.
Stacked on #8983 (BoolIndex) → #8982 (MapIndex) → #8981 → #8980 →
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): remove legacy condition_checker dispatch helpers (#8985)
* refactor(field_index): remove legacy condition_checker dispatch helpers
Final PR (7) of the condition-checker-migration sub-plan. With every
typed FieldIndex variant now serving its conditions via
PayloadFieldIndexRead::condition_checker (PRs 1-6), the wrapper
`field_condition_index` and the legacy match dispatch are no longer
needed.
Changes:
1. read_view/condition_converter/mod.rs's three call sites
(Condition::Field, Condition::IsEmpty, Condition::IsNull) call
`index.condition_checker(...)` directly instead of going through
the `field_condition_index` wrapper.
2. helpers.rs (49 lines) and match_converter.rs (119 lines) deleted —
`field_condition_index`, `get_match_checkers`,
`get_match_value_checker`, `get_match_any_checker`,
`get_match_except_checker` are all gone.
3. The directory `condition_converter/` collapses to a flat file
`condition_converter.rs` (mod.rs renamed via git).
Behavioural note: the legacy `Match::Except` `values_count > 0`
fallback for type-mismatched Except queries is no longer applied —
those now fall through to the payload check via
`check_field_condition`. Schema-level type checking prevents
cross-type Match::Except queries from reaching here in practice, so
the fallback was only useful for degenerate inputs; payload check is
semantically equivalent.
Audit grep on `lib/segment/src/`:
$ grep -rn "field_condition_index\|get_match_checkers\|...\|TextQueryType"
(no matches)
All legacy helper machinery is gone.
Stacked on #8984 (FullTextIndex) → #8983 → #8982 → #8981 → #8980 →
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): migrate indexed_variable_retriever into FieldIndexRead (#8986)
* refactor(field_index): migrate indexed_variable_retriever into FieldIndexRead
Follow-up to the condition-checker-migration sub-plan. The read view's
`value_retriever/helpers.rs` previously destructured `FieldIndex`
variants inside `indexed_variable_retriever` to build per-point value
extractors. This PR turns that into a `value_retriever` trait method
on `FieldIndexRead`, so the helper now depends only on the trait —
no variant destructuring outside `FieldIndex`'s own impl.
Changes:
1. FieldIndexRead gains `value_retriever(&self, hw_counter)`. Lives
on the enum-level trait (rather than `PayloadFieldIndexRead`)
because the value→Value conversion depends on the payload type
`U` in `NumericIndex<T, U>`, and only `FieldIndex` knows the
`(T, U)` pairing.
2. impl FieldIndexRead for FieldIndex implements value_retriever
with an 11-arm variant match. Bodies copied verbatim from the old
`indexed_variable_retriever`. FullTextIndex and NullIndex return
None (caller falls back to payload).
3. read_view/value_retriever/helpers.rs:
- `variable_retriever` now calls `index.value_retriever(hw_counter)`
directly via trait dispatch
- `indexed_variable_retriever` deleted
- Unused imports dropped (Number, DateTimePayloadType,
UuidPayloadType, MultiValue)
Helpers.rs shrinks from 189 lines to 73 lines (the test module
stays). Total: +134 / -122 across 3 files, but the structural change
is moving the dispatch into the trait.
Stacked on #8985 (condition-checker-cleanup) → #8984 → … → #8978 → dev.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): push value_retriever conversion logic into typed indexes
`FieldIndex::value_retriever` previously inlined the per-variant
value→JSON `Value` conversion (Number, String, DateTime, UUID, Bool,
serde_json::to_value for geo) as a 100-line variant match. Move each
variant's conversion into an inherent `value_retriever` method on the
corresponding typed index; `FieldIndex` is left with a thin
11-arm dispatch that just delegates.
Changes:
1. NumericIndex<T, U> gains 4 per-(T, U) inherent value_retriever
impls (Int/Datetime/Float/Uuid). The conversion is U-specific
(timestamp formatting, UUID string, Number::from_f64 etc.), so
each (T, U) pair has its own impl block.
2. MapIndex<K> gains 3 per-K inherent value_retriever impls
(Keyword/Int/Uuid).
3. GeoMapIndex and BoolIndex each gain a single inherent
value_retriever.
4. FieldIndex::value_retriever collapses to a 11-arm match that
delegates to the typed index's inherent method. FullTextIndex
and NullIndex still return None at the FieldIndex level — they
don't expose value retrieval and the caller falls back to payload.
5. field_index.rs sheds the Number, MultiValue and Box::new
noise — about 100 lines smaller.
Same shape as how `wipe`, `add_point`, `get_telemetry_data` already
delegate from FieldIndex to typed indexes. Adding a new FieldIndex
variant now forces compile errors in (a) the dispatch in FieldIndex
and (b) the new typed index's inherent impl — both correct places.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(field_index): push BoolIndex/NullIndex condition_checker into storage variants
The `BoolIndex` and `NullIndex` wrappers were special-cased among
typed indexes: their `PayloadFieldIndexRead::condition_checker`
impls did the variant-specific work directly on `self` (matching
`Match::Value(Bool)`, `is_empty`, `is_null`) and called
`self.check_values_any` / `self.values_is_empty` / `self.values_is_null`,
each of which dispatched `match self { Mmap(...) | Immutable(...) }`
per point. The inner storage variants (`Immutable/MutableBoolIndex`,
`Immutable/MutableNullIndex`) implemented `PayloadFieldIndexRead`
solely for `filter` / `count_indexed_points` / `estimate_cardinality`
/ `for_each_payload_block` — their `condition_checker` was unreachable
behind the trait's default `None` body.
Removing the trait default surfaced this asymmetry. Rather than stub
the inner-type `condition_checker` as dead `None`, push the real
logic down to where the storage variants live — same shape as
`filter` already uses:
inner: explicit FieldCondition destructure + exhaustive Match
arms; per-point closure captures `&'a self` of the inner
type directly.
outer: 2-arm `match self { ... => inner.condition_checker(...) }`.
Result: per-point closure body no longer hits a `match self { ... }`
on the wrapper. The wrapper dispatch happens once during setup when
the wrapper picks which inner closure to return. Matches the pattern
the other 5 trait methods (filter/count/estimate/for_each_block) on
these wrappers already use.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* CI fix
* upd docstring
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When an `AddPeer` / `RemovePeer` / `UpdatePeerMetadata` /
`UpdateClusterMetadata` operation is proposed locally and then committed
remotely, the resulting log entry can be delivered back to us as part of
a raft snapshot rather than as a regular log entry. In that case the
operation never flows through `apply_conf_change_entry` /
`apply_normal_entry`, so the awaiter registered by
`propose_consensus_op_with_await` was never woken and timed out after
10s — manifesting as flaky failures of `test_peer_snapshot_bootstrap`
("Failed to add peer: ... Waiting for consensus operation commit failed").
Resolve awaiters in `apply_snapshot` whenever their effect is directly
observable in the new persistent state.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The helpers in query_optimization/condition_converter{,/match_converter}.rs
and the bulk of query_optimization/rescore_formula/value_retriever.rs were
only ever called from struct_payload_index/read_view/. Move them next to
their callers.
The trait split work (#8975, #8976) showed why these helpers can't migrate
to a uniform FieldIndexRead bound: every branch in field_condition_index,
match_converter, get_range_*, get_geo_*, indexed_variable_retriever
destructures FieldIndex variants and reaches into variant-specific
predicates on the underlying typed indexes (MapIndex::check_values_any,
FullTextIndex::parse_*_query, NumericIndex::get_values, …). The variant
match is the whole point of these helpers; co-locating them with the read
view is the right shape.
New layout:
- read_view/condition_converter/{mod.rs, helpers.rs, match_converter.rs}
(was a flat read_view/condition_converter.rs + scattered query_optimization
files)
- read_view/value_retriever/{mod.rs, helpers.rs}
(was a flat read_view/value_retriever.rs + query_optimization/rescore_formula/
value_retriever.rs)
VariableRetrieverFn type alias stays in query_optimization/rescore_formula/
value_retriever.rs (now a 6-line file): formula_scorer.rs in the same module
uses it as a struct field, so the type belongs there. Only the helper
functions and their tests move.
Visibility tightened where natural: cross-file entry points are
pub(super), purely-local helpers are private, public-API surface is
unchanged.
No behavioural change. cargo test -p segment: 666 unit + 120 integration
tests pass.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: implement FieldIndexRead for FieldIndex
Move the 10 read-only inherent methods on FieldIndex (special_check_condition,
count_indexed_points, filter, estimate_cardinality, for_each_payload_block,
get_telemetry_data, values_count, values_is_empty, as_numeric, as_facet_index)
into impl FieldIndexRead for FieldIndex. Bodies are unchanged.
Write/lifecycle methods (add_point, remove_point, wipe, flusher, files,
immutable_files, ram_usage_bytes, is_on_disk, populate, clear_cache,
get_full_index_type) stay inherent on FieldIndex.
Call sites that hold &FieldIndex now resolve through the trait — added
`use ... FieldIndexRead;` imports where the compiler asked
(read_view/{payload_index_read,filtering}.rs, query_optimization/
condition_converter.rs and match_converter.rs, payload_storage/
query_checker.rs, two integration tests, full_text_index tests module).
StructPayloadIndex::get_facet_index keeps its concrete
OperationResult<FacetIndexEnum<'_>> return: the trait method as_facet_index
returns Option<impl FacetIndex + '_> (RPITIT) which can't be downcast back
to FacetIndexEnum, so the function inlines the variant match. Same shape as
before, just expressed locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: remove unused StructPayloadIndex::get_facet_index
Confirmed dead code — the function had no callers anywhere in the workspace.
It was preserved across recent refactors (most recently #8967) but was never
referenced. Removing it also drops the now-unused JsonPath and FacetIndexEnum
imports in struct_payload_index/mod.rs.
This obsoletes the inline variant match introduced in the previous commit:
that match existed solely to keep get_facet_index returning the concrete
FacetIndexEnum<'_> when as_facet_index moved to the FieldIndexRead trait
with an opaque return. With get_facet_index gone, no workaround is needed.
The OperationError::MissingMapIndexForFacet variant stays — it is still used
by segment::read_view::facet and lib/collection/src/operations/types.rs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror the PayloadFieldIndex / PayloadFieldIndexRead split (#8966) one layer
up at the FieldIndex enum level. FieldIndexRead names the uniform read-only
surface (filter, estimate_cardinality, count_indexed_points,
for_each_payload_block, get_telemetry_data, values_count, values_is_empty,
special_check_condition, as_numeric, as_facet_index) — the methods that
StructPayloadIndexReadView and its helpers reach through IndexesMap.
Trait declaration only. No impl, no callers in this PR — this is purely
additive. Follow-up PRs will impl it on FieldIndex, then migrate
non-destructuring helpers to a FieldIndexRead bound.
as_numeric and as_facet_index use return-position impl Trait
(impl NumericFieldIndexRead + '_, impl FacetIndex + '_), matching how
PayloadIndexRead exposes the same shapes. The trait is therefore not
object-safe; consumers will use a generic bound rather than
&dyn FieldIndexRead.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(index): drop PayloadIndex: PayloadIndexRead super-trait
`PayloadIndex` now only declares the mutating surface; reads live on
the sibling `PayloadIndexRead` trait. The previous super-trait
relationship forced any type that implemented `PayloadIndex` to also
implement `PayloadIndexRead`, blocking a future `PayloadIndexRead`-only
view that doesn't (and shouldn't) own the writable index machinery.
No behavioural change. Audit before committing showed no generic
bound site on `PayloadIndex` exists in the workspace, and every
caller that uses read methods already imports `PayloadIndexRead`
explicitly (the trait was already used as a generic bound on
`SegmentReadView`'s `TPayloadIndex` parameter and on
`iter_filtered_points`). The full workspace builds clean and all
segment / storage / collection tests pass without any consumer
update.
Doc comment on `PayloadIndex` updated to point readers at
`PayloadIndexRead` for the read surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(index): introduce StructPayloadIndexReadView<P, I, V> (#8970)
Move the read surface of `StructPayloadIndex` onto a new borrowed view
struct generic over `<P: PayloadStorageRead, I: IdTrackerRead, V: VectorStorageRead>`.
The view holds exactly the fields that `PayloadIndexRead` requires --
no more, no less:
pub struct StructPayloadIndexReadView<'a, P, I, V> {
payload: &'a Arc<AtomicRefCell<P>>,
id_tracker: &'a I,
vector_storages: &'a HashMap<VectorNameBuf, Arc<AtomicRefCell<V>>>,
field_indexes: &'a IndexesMap,
config: &'a PayloadConfig,
visited_pool: &'a VisitedPool,
}
`StructPayloadIndex` now exposes a `with_view(|v| ...)` accessor that
borrows `id_tracker` once at the top and constructs the view for the
closure scope. All read-method bodies move onto the view, which is the
sole `PayloadIndexRead` implementor for this index.
Why three generics
==================
- `P: PayloadStorageRead` -- already generic via PR #8968.
- `I: IdTrackerRead` -- direct method calls; held as `&I` (not
`&Arc<AtomicRefCell<I>>`) because the cell is collapsed at the
`with_view` boundary, saving a per-method `borrow()` atomic op.
`dyn IdTrackerRead` does not satisfy `I: IdTrackerRead` bounds in
Rust without an explicit blanket impl, so generic is the only
consistent option here.
- `V: VectorStorageRead` -- the only access site is
`available_vector_count()` for the `HasVector` cardinality branch
(`condition_cardinality` in `read_view/filtering.rs`).
Why `payload` keeps the `Arc`
=============================
`PayloadProvider<P>::new(...)` (introduced in PR #8968) takes
`Arc<AtomicRefCell<P>>` so that the returned `FormulaScorer<'q>` /
`Box<dyn FilterContext + 'a>` can outlive the caller frame. The view
therefore holds `&'a Arc<AtomicRefCell<P>>` (asymmetric vs the bare
`&I` for `id_tracker`). Switching to a borrow-based provider would
require reworking `formula_scorer` / `filter_context` to callback
style; deferred to a follow-up if needed.
What does NOT move
==================
- `build_field_indexes` and `clear_index_for_point` stay on
`StructPayloadIndex`. `build_field_indexes` is read-shaped but only
has write-side callers, and pulls in the `selector` machinery which
uses `path` + `storage_type`. Keeping it on the writable struct
means `path` and `is_appendable` do not need to leak into the view.
- The `selector` / `selector_with_type` helpers stay on the writable
struct for the same reason.
- The free helpers in `query_optimization/condition_converter.rs`
(range / geo / null / is-empty checkers) stay where they are; their
visibility is bumped from `fn` to `pub(in crate::index)` so the view
can still call them.
Module layout
=============
lib/segment/src/index/struct_payload_index/
mod.rs # owning struct + with_view
build.rs # write-side build coordination
payload_index.rs # impl PayloadIndex (mutating only)
tests.rs
read_view/
mod.rs # view struct + module wiring
payload_index_read.rs # impl PayloadIndexRead for view
filtering.rs # struct_filtered_context, condition_cardinality, query_field, estimate_field_condition
condition_converter.rs # impl block from query_optimization/
optimizer.rs # impl block from query_optimization/
value_retriever.rs # impl block from query_optimization/
tests.rs # smoke test that builds the view directly
Consumer migration
==================
- `Segment::with_view` nests the new `StructPayloadIndex::with_view`
inside it; `SegmentReadViewFor<'s>` uses the view as its
`TPayloadIndex` parameter.
- HNSW (`hnsw.rs`), sparse (`sparse_vector_index.rs`), plain
(`plain_vector_index.rs`) call sites wrap their read-method calls in
`payload_index.borrow().with_view(|v| ...)`.
- `Segment::get_indexed_fields`, `update_all_field_indices`, and
`SegmentBuilder::build` switch to `with_view` for `indexed_fields()`
/ `get_payload_sequential()`.
- Integration tests and benches similarly migrate.
- `set_payload` (still on `PayloadIndex` write impl) inlines its
former `self.get_payload(...)` call as `self.payload.borrow().get(...)`
to avoid going through `with_view` from a `&mut self` write path.
Smoke test (`read_view/tests.rs`) constructs the view directly over
`InMemoryPayloadStorage` + `InMemoryIdTracker` + an empty vector-storage
map, and exercises `indexed_fields()`, `query_points()`, and
`available_point_count()` -- proving the view is genuinely decoupled
from `StructPayloadIndex`. This is the abstraction PR 4 will use to
wire a read-only segment.
Verified
========
- `cargo build --workspace --tests --benches` -- green
- `cargo test -p segment --lib` -- 666 passed (665 + 1 new smoke test), 0 failed
- `cargo test -p segment --tests` -- 120 integration tests pass
- `cargo test -p storage --lib` -- 44 passed
- `cargo test -p collection --lib` -- 197 passed
- `cargo clippy -p segment --tests --benches` -- clean
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `payload_ref(point_offset, hw_counter) -> OwnedPayloadRef<'_>` to
the `PayloadStorageRead` trait so payload retrieval no longer needs
to know whether the underlying storage is in-memory (zero-copy
borrow) or mmap/on-disk (materialised clone). In-memory storage
returns `OwnedPayloadRef::Ref(...)` to preserve the existing
zero-copy hot path; on-disk storages return `OwnedPayloadRef::Owned(...)`.
`PayloadProvider` is now generic over `P: PayloadStorageRead`
(`Arc<AtomicRefCell<P>>`) instead of being hard-coded to
`PayloadStorageEnum`. The previous `match`-on-enum dispatch in
`PayloadProvider::with_payload` is gone; dispatch happens through
`PayloadStorageRead::payload_ref`. Manual `Clone` impl avoids
imposing a `P: Clone` bound (cloning the provider is just an `Arc`
bump). The `empty_payload` fallback field is removed -- `payload_ref`
always returns a usable ref.
`<P: PayloadStorageRead + 'a>` is threaded through the consuming
signatures in `query_optimization/`: `optimize_filter`,
`convert_conditions`, `optimize_should/min_should/must/must_not`,
`condition_converter`, `variable_retriever`,
`payload_variable_retriever`. The `+ 'a` bound is required because
the returned `OptimizedFilter<'a>` / `ConditionCheckerFn<'a>` /
`VariableRetrieverFn<'a>` contain boxed closures that capture the
provider. The cascade stops at the box: `StructFilterContext<'a>`,
`FormulaScorer<'a>`, and the rest of the search machinery are
unaffected.
Construction sites (`StructPayloadIndex::formula_scorer`,
`struct_filtered_context`, `retrievers_map`) need no source-level
change -- type inference picks `P = PayloadStorageEnum` from the
`Arc<AtomicRefCell<PayloadStorageEnum>>` they pass in.
The panic-on-corruption policy that lived in the previous Mmap
branch is preserved verbatim and now applies uniformly across
storage backends.
This unblocks building a `PayloadStorageRead`-generic read-only
view of `StructPayloadIndex` (follow-up work).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Break up the 778-line field_index_base.rs into focused submodules
under field_index_base/ (payload_field_index, value_indexer,
field_index, builder) and relocate NumericFieldIndex /
NumericFieldIndexRead into the numeric_index module where they
naturally belong.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* bytemuck::Pod already implies 'static
* no supertraits
* regions gaps with TypedStorage
* StoredBitSlice with TypedStorage
* bytemuck for PostingsHeader
* bytemuck for TrackerHeader
* bytemuck for MmapRange
* bytemuck for stored_point_to_values::Header
* derive bytemuck::Pod on `OptionalPointer`, get rid of legacy transmute
* remove one layer of Option in `get_raw`
* keep `UniversalRead<u8>` bound, do bytemuck conversion at call site (#8953)
Drop the `+ UniversalRead<OptionalPointer>` / `+ UniversalWrite<OptionalPointer>`
trait bounds added in the previous two commits. Reads and writes of
`OptionalPointer` and `TrackerHeader` now go through `bytemuck::from_bytes` /
`bytemuck::bytes_of` on the existing `u8` byte slices instead.
This keeps the unsafe-transmute removal but avoids the turbofish noise
(`UniversalWrite::<u8>::flusher(&self.storage)`, `<S as UniversalRead<u8>>::open`,
…) that two `T`s on `S` forced everywhere.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* extra refactoring after #8955
* explicit zeroed instantiation
---------
Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Move element type from UniversalRead trait to method generics
Lifts the `T` parameter off `trait UniversalRead<T>` (and the matching
`UniversalWrite<T>`) and onto the read/write methods themselves. The
`ReadPipeline` associated type becomes a GAT over `T`. With per-method
generics, callers that need to read several element types from one storage
just write `S: UniversalRead` instead of stacking
`UniversalRead<u8> + UniversalRead<Counts> + ...`.
Removes the workarounds the old shape required:
- `TypedStorage<S, T>` newtype (sole purpose was disambiguating multi-bounds)
- `UniversalReadFamily` HKT shim
- `StoredGeoMapIndexStorage` four-bound trait alias
- `CachedSlice<T>` is now non-generic; `T` moves to `get_range`/`len`
No runtime behavior change: alignment in `IoUringRuntime` and `CachedSlice`
is preserved because `T` is still known at each call site.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Restore TypedStorage as a typed-access fail-safe
Reintroduces `TypedStorage<S, T>` as a transparent wrapper around
`UniversalRead`/`UniversalWrite` storage that fixes the element type to
`T`. With per-method generics on the underlying traits, callers can
otherwise read or write any `T` from the same handle; this wrapper
binds it at the type level so accidental cross-type access fails to
compile.
The wrapper exposes inherent typed methods (`read::<P>`, `read_iter`,
`write`, `len`, …) that delegate to the inner storage with `T` fixed.
It does not implement `UniversalRead`/`UniversalWrite` itself — those
are intentionally avoided to prevent the typed binding from being
bypassed via the generic trait methods.
Restores the wrapping at the previous call sites: `StoredStruct`'s
inner storage, `ImmutableIdTracker`'s version mmap, the geo and
numeric index storages, the chunked-vectors chunks, and the
immutable dense vector storage.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Silence clippy::len_without_is_empty on TypedStorage
`TypedStorage::len` returns `Result<u64>` (a fallible byte length from
the underlying storage), so an `is_empty` companion would also be
fallible and offer nothing over `len()? == 0`. Suppress the lint at
the impl block, matching how the underlying `UniversalRead::len` is
already exempted.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fmt
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [ai] Add integration test for triggering inconsistent resharding state
* [ai] Also add test for resharding down
* Update test
* Resolve resharding idempotency through setting replica states
* Remove resharding down test
* Reformat
* [ai] Remove resharding abort order, abort before setting replica state
* [ai] Resolve test flakiness
* Collapse matches into helper function
* Check preconditions before aborting resharding
* Fix test flakiness by not waiting for a dead node
* Abort resharding before aborting transfer for idempotency
* Update comment
* Release shard holder lock on transfer/reshard abort to prevent deadlock
* Remove now unused shard holder parameter
* Split handle_replica_changes to eliminate need for juggling locking
* In resharding tests, import all utils to enable every_test cleanup
Resolves flakiness I've been seeing in
test_set_replica_dead_clears_resharding_state test
* Fix phrase matching crossing string-array element boundaries
When indexing a string array like ["quick", "brown"], the index
concatenated tokens from all elements into a single flat document.
A phrase query for "quick brown" found the tokens consecutively,
producing false matches.
Insert a sentinel token ("\x00") between array elements during
indexing. It is registered as a normal vocab token that occupies a
position in the document, so phrase windows cannot span it. No
tokenizer ever produces this string, so it never appears in a query.
Closes#8937
Co-authored-by: Cursor <cursoragent@cursor.com>
* Strip null bytes from tokenizer output
Ensure no tokenizer can produce tokens containing '\0', which is
reserved as the array-boundary sentinel for phrase matching.
Covers all tokenizer paths:
- process_token_cow (used by Word, Whitespace, Prefix, Multilingual)
- Japanese tokenizer (has its own inline processing)
Co-authored-by: Cursor <cursoragent@cursor.com>
* Revert "Strip null bytes from tokenizer output"
This reverts commit fe6ea7a955.
* filter sentinel token from query
* Preallocate vector
Co-authored-by: Tim Visée <tim+github@visee.me>
---------
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: generall <andrey@vasnetsov.com>
Co-authored-by: Tim Visée <tim+github@visee.me>
