[MOD-11174] Port NewInvIndIterator_NumericFull to Rust by gdesmott · Pull Request #6796 · RediSearch/RediSearch

gdesmott · 2025-09-08T07:35:47Z

Rust port of NewInvIndIterator_NumericFull. Does not include validation yet, this will be part of a follow up PR.

TODO:

tests and bench from rqe_iterators_bencher are currently crashing but does work when linking on the debug C lib rather than the release one ?! Fixed with fix bugs regarding misalignment and missing symbols #7000
benchmark results, blocked by the above.

Benchmark results

Rust implementation is slightly faster as the C one.

Iterator - InvertedIndex - NumericFull - Read Dense/C    time:   [11.226 ms 11.238 ms 11.254 ms]
Iterator - InvertedIndex - NumericFull - Read Dense/Rust time:   [7.5867 ms 7.6308 ms 7.6764 ms]

Iterator - InvertedIndex - NumericFull - Read Sparse/C    time:   [11.459 ms 11.481 ms 11.507 ms]
Iterator - InvertedIndex - NumericFull - Read Sparse/Rust time:   [9.0627 ms 9.0705 ms 9.0792 ms]

Iterator - InvertedIndex - NumericFull - SkipTo Dense/C    time:   [11.123 ms 11.143 ms 11.165 ms]
Iterator - InvertedIndex - NumericFull - SkipTo Dense/Rust time:   [7.3433 ms 7.3705 ms 7.3999 ms]

Iterator - InvertedIndex - NumericFull - SkipTo Sparse/C    time:   [13.109 ms 13.125 ms 13.145 ms]
Iterator - InvertedIndex - NumericFull - SkipTo Sparse/Rust time:   [11.015 ms 11.026 ms 11.040 ms]

Mark if applicable

This PR introduces API changes
This PR introduces serialization changes

Note

Ports the NumericFull inverted index iterator to Rust, introduces numeric reader/decoder traits, and adds tests and benchmarks with necessary FFI/build updates.

Iterators (Rust):
- Add rqe_iterators::inverted_index with FullIterator and NumericFull providing full-scan and skip_to over inverted indexes.
- Define RQEIteratorError via thiserror; extend SkipToOutcome and iterator API.
Inverted Index (core):
- Add marker traits NumericDecoder and NumericReader; implement for numeric::Numeric, IndexReaderCore, and filter readers.
- Constrain FilterNumericReader and FilterGeoReader to wrap NumericReader and propagate NumericReader impls.
Tests:
- New iterator tests for NumericFull read/skip behavior.
- Update core tests to use NumericReader where applicable and mark iterator mocks as NumericReader.
Benchmarks:
- Add rqe_iterators_bencher support for NumericFull (dense/sparse read and skip-to), including C FFI wrappers, stubs, and build script/header updates.
Dependencies/Build:
- Add thiserror to rqe_iterators; add triemap_ffi and varint_ffi to bencher; update build to link buffer and generate bindings for inverted index headers.
Misc:
- Minor comment style fix in src/tag_index.h.

^{Written by Cursor Bugbot for commit 9210057. This will update automatically on new commits. Configure here.}

src/redisearch_rs/rqe_iterators/src/inverted_index_it.rs

src/redisearch_rs/rqe_iterators_bencher/src/ffi.rs

src/redisearch_rs/rqe_iterators_bencher/src/lib.rs

src/redisearch_rs/rqe_iterators/tests/inverted_index.rs

src/redisearch_rs/rqe_iterators/src/lib.rs

src/redisearch_rs/rqe_iterators/src/inverted_index_it.rs

src/redisearch_rs/rqe_iterators/src/inverted_index.rs

src/redisearch_rs/rqe_iterators/tests/inverted_index.rs

Will allow us to use '?' when reading from the II which is returning a std::io::Error

src/redisearch_rs/rqe_iterators_bencher/src/lib.rs

src/tag_index.h

src/redisearch_rs/inverted_index/src/tests.rs

src/redisearch_rs/rqe_iterators/src/inverted_index.rs

Rust re-implementation of NewInvIndIterator_NumericFull(). The tests logic have been copied from the existing C++ tests. Validation is not implemented yet.

I had to edit a comment block in tag_index.h to workaround a nasty bug in bindgen, see rust-lang/rust-bindgen#1313 Also add simple wrappers so it can easily be used in the bencher.

Will be used to share code with the term iterator tests.

cursor

Bug: Trait Bound Mismatch in FilterNumericReader

The generic IndexReader implementation for FilterNumericReader uses the bound IR: IndexReader<'index>, but the constructor FilterNumericReader::new requires IR: NumericReader<'index>. This is a trait bound mismatch. The impl IndexReader block should use IR: NumericReader<'index> to match the constructor's requirements and ensure type safety, since FilterNumericReader is specifically designed to work with numeric readers only.

src/redisearch_rs/inverted_index/src/lib.rs#L1503-L1575

RediSearch/src/redisearch_rs/inverted_index/src/lib.rs

Lines 1503 to 1575 in dbd036b

    
                       if self.filter.value_in_range(value) { 
        
                           return Ok(true); 
        
                       } 
        
                   } 
        
               } 
        
               /// Seek to the record with the given document ID in the inner reader that matches the numeric filter. 
        
               /// 
        
               /// # Safety 
        
               /// 
        
               /// 1. `result.is_numeric()` must be true when this function is called. 
        
               #[inline(always)] 
        
               fn seek_record( 
        
                   &mut self, 
        
                   doc_id: t_docId, 
        
                   result: &mut RSIndexResult<'index>, 
        
               ) -> std::io::Result<bool> { 
        
                   let success = self.inner.seek_record(doc_id, result)?; 
        
                   if !success { 
        
                       return Ok(false); 
        
                   } 
        
                   // SAFETY: the caller must ensure the result is numeric 
        
                   let value = unsafe { result.as_numeric_unchecked() }; 
        
                   if self.filter.value_in_range(value) { 
        
                       Ok(true) 
        
                   } else { 
        
                       self.next_record(result) 
        
                   } 
        
               } 
        
               fn skip_to(&mut self, doc_id: t_docId) -> bool { 
        
                   self.inner.skip_to(doc_id) 
        
               } 
        
               fn reset(&mut self) { 
        
                   self.inner.reset(); 
        
               } 
        
               fn unique_docs(&self) -> usize { 
        
                   self.inner.unique_docs() 
        
               } 
        
               fn has_duplicates(&self) -> bool { 
        
                   self.inner.has_duplicates() 
        
               } 
        
               fn flags(&self) -> ffi::IndexFlags { 
        
                   self.inner.flags() 
        
               } 
        
               fn needs_revalidation(&self) -> bool { 
        
                   self.inner.needs_revalidation() 
        
               } 
        
           } 
        
           impl<'filter, 'index, E: DecodedBy<Decoder = D>, D: Decoder> 
        
               FilterNumericReader<'filter, IndexReaderCore<'index, E, D>> 
        
           { 
        
               /// Check if the underlying index has been modified since the last time this reader read from it. 
        
               /// If it has, then the reader should be reset before reading from it again. 
        
               pub fn needs_revalidation(&self) -> bool { 
        
                   self.inner.needs_revalidation() 
        
               } 
        
               /// Check if this reader is reading from the given index 
        
               pub fn is_index(&self, index: &InvertedIndex<E>) -> bool { 
        
                   self.inner.is_index(index) 
        
               }

Will be used to restrict the numeric II iterator to readers producing numeric results. Also allow us to restrict the type of reader wrapper by FilterNumericReader and FilterGeoReader.

Allow the numeric iterator to be used with a numeric filter wrapper. Will also be used to implement the term iterator.

github-actions bot added the size:L label Sep 8, 2025

gdesmott force-pushed the gd_11174_numeric_full_it branch 8 times, most recently from 47e98a7 to 6eb70a0 Compare September 9, 2025 11:11

gdesmott requested a review from chesedo September 9, 2025 13:56

chesedo reviewed Sep 9, 2025

View reviewed changes

src/redisearch_rs/rqe_iterators/src/inverted_index_it.rs Outdated Show resolved Hide resolved

chesedo reviewed Sep 9, 2025

View reviewed changes

src/redisearch_rs/rqe_iterators/src/inverted_index_it.rs Outdated Show resolved Hide resolved

gdesmott force-pushed the gd_11174_numeric_full_it branch 2 times, most recently from 0bdd24b to 7c649f0 Compare September 10, 2025 08:42

gdesmott requested a review from chesedo September 10, 2025 08:43

gdesmott force-pushed the gd_11174_numeric_full_it branch 5 times, most recently from 7c4e1d1 to 2cf02bb Compare September 11, 2025 09:50

gdesmott mentioned this pull request Sep 11, 2025

[MOD-11178] Port NewInvIndIterator_TermFull to Rust #6814

Merged

2 tasks

gdesmott force-pushed the gd_11174_numeric_full_it branch 2 times, most recently from e293e76 to 548fc17 Compare October 6, 2025 13:03

GlenDC force-pushed the gd_11174_numeric_full_it branch from 548fc17 to d25ff5e Compare October 7, 2025 08:35

GlenDC mentioned this pull request Oct 7, 2025

fix bugs regarding misalignment and missing symbols #7000

Merged

2 tasks

gdesmott requested a review from GlenDC October 7, 2025 12:02

GlenDC suggested changes Oct 7, 2025

View reviewed changes

gdesmott force-pushed the gd_11174_numeric_full_it branch from d25ff5e to 7a2b27d Compare October 8, 2025 07:16

gdesmott requested a review from GlenDC October 8, 2025 07:17

GlenDC previously approved these changes Oct 9, 2025

View reviewed changes

gdesmott force-pushed the gd_11174_numeric_full_it branch from 30a44e3 to c101b05 Compare October 27, 2025 08:32

gdesmott requested a review from chesedo October 27, 2025 08:33

cursor bot reviewed Oct 27, 2025

View reviewed changes

src/redisearch_rs/rqe_iterators/tests/inverted_index.rs Show resolved Hide resolved

gdesmott force-pushed the gd_11174_numeric_full_it branch from c101b05 to 2f70504 Compare October 27, 2025 09:35

iterator: use thiserror

779a5c6

Will allow us to use '?' when reading from the II which is returning a std::io::Error

gdesmott force-pushed the gd_11174_numeric_full_it branch 3 times, most recently from f7bd0b9 to b68fee0 Compare October 27, 2025 12:27

chesedo previously approved these changes Oct 28, 2025

View reviewed changes

GuyAv46 reviewed Oct 28, 2025

View reviewed changes

add NumericFull iterator

4abbc5f

Rust re-implementation of NewInvIndIterator_NumericFull(). The tests logic have been copied from the existing C++ tests. Validation is not implemented yet.

gdesmott dismissed chesedo’s stale review via 86e1206 October 29, 2025 08:46

gdesmott force-pushed the gd_11174_numeric_full_it branch from b68fee0 to 86e1206 Compare October 29, 2025 08:46

gdesmott requested a review from GuyAv46 October 29, 2025 09:23

gdesmott added 3 commits October 29, 2025 10:36

generate bindings for the II C iterator

a845538

I had to edit a comment block in tag_index.h to workaround a nasty bug in bindgen, see rust-lang/rust-bindgen#1313 Also add simple wrappers so it can easily be used in the bencher.

bench II numeric full iterator

553c56c

make iterator test more generic

bdf3022

Will be used to share code with the term iterator tests.

gdesmott force-pushed the gd_11174_numeric_full_it branch from 86e1206 to dbd036b Compare October 29, 2025 09:36

cursor bot reviewed Oct 29, 2025

View reviewed changes

GuyAv46 previously approved these changes Oct 29, 2025

View reviewed changes

gdesmott added 2 commits October 29, 2025 10:44

add NumericDecoder and NumericReader marker traits

3c68f7d

Will be used to restrict the numeric II iterator to readers producing numeric results. Also allow us to restrict the type of reader wrapper by FilterNumericReader and FilterGeoReader.

add FullIterator abstraction

9210057

Allow the numeric iterator to be used with a numeric filter wrapper. Will also be used to implement the term iterator.

gdesmott dismissed GuyAv46’s stale review via 9210057 October 29, 2025 09:44

gdesmott force-pushed the gd_11174_numeric_full_it branch from dbd036b to 9210057 Compare October 29, 2025 09:44

gdesmott enabled auto-merge October 29, 2025 09:46

GuyAv46 approved these changes Oct 29, 2025

View reviewed changes

gdesmott added this pull request to the merge queue Oct 29, 2025

Merged via the queue into master with commit 71de36c Oct 29, 2025
19 checks passed

gdesmott deleted the gd_11174_numeric_full_it branch October 29, 2025 11:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MOD-11174] Port NewInvIndIterator_NumericFull to Rust#6796

[MOD-11174] Port NewInvIndIterator_NumericFull to Rust#6796
gdesmott merged 7 commits intomasterfrom
gd_11174_numeric_full_it

gdesmott commented Sep 8, 2025 •

edited by cursor bot

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


	if self.filter.value_in_range(value) {
	return Ok(true);
	}
	}
	}

	/// Seek to the record with the given document ID in the inner reader that matches the numeric filter.
	///
	/// # Safety
	///
	/// 1. `result.is_numeric()` must be true when this function is called.
	#[inline(always)]
	fn seek_record(
	&mut self,
	doc_id: t_docId,
	result: &mut RSIndexResult<'index>,
	) -> std::io::Result<bool> {
	let success = self.inner.seek_record(doc_id, result)?;

	if !success {
	return Ok(false);
	}

	// SAFETY: the caller must ensure the result is numeric
	let value = unsafe { result.as_numeric_unchecked() };

	if self.filter.value_in_range(value) {
	Ok(true)
	} else {
	self.next_record(result)
	}
	}

	fn skip_to(&mut self, doc_id: t_docId) -> bool {
	self.inner.skip_to(doc_id)
	}

	fn reset(&mut self) {
	self.inner.reset();
	}

	fn unique_docs(&self) -> usize {
	self.inner.unique_docs()
	}

	fn has_duplicates(&self) -> bool {
	self.inner.has_duplicates()
	}

	fn flags(&self) -> ffi::IndexFlags {
	self.inner.flags()
	}

	fn needs_revalidation(&self) -> bool {
	self.inner.needs_revalidation()
	}
	}

	impl<'filter, 'index, E: DecodedBy<Decoder = D>, D: Decoder>
	FilterNumericReader<'filter, IndexReaderCore<'index, E, D>>
	{
	/// Check if the underlying index has been modified since the last time this reader read from it.
	/// If it has, then the reader should be reset before reading from it again.
	pub fn needs_revalidation(&self) -> bool {
	self.inner.needs_revalidation()
	}

	/// Check if this reader is reading from the given index
	pub fn is_index(&self, index: &InvertedIndex<E>) -> bool {
	self.inner.is_index(index)
	}

Conversation

gdesmott commented Sep 8, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark results

Mark if applicable

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Bug: Trait Bound Mismatch in FilterNumericReader

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gdesmott commented Sep 8, 2025 •

edited by cursor bot

Loading