Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #1151 #1152

Merged
merged 5 commits into from
Sep 10, 2021
Merged

fix #1151 #1152

merged 5 commits into from
Sep 10, 2021

Conversation

PSeitz
Copy link
Contributor

@PSeitz PSeitz commented Sep 8, 2021

Fixes a off by one error in the stats for the index fast field in the multi value fast field.
When retrieving the data range for a docid, get(doc)..get(docid+1) is requested. On creation
the num_vals statistic was set to doc instead of docid + 1. In the multivaluelinearinterpol fast
field the last value was therefore not serialized (and would return 0 instead in most cases).
So the last document get(lastdoc)..get(lastdoc + 1) would return the invalid range value..0.

This PR adds a proptest to cover this scenario. A combination of a large number values, since multilinear
interpolation is only active for more than 5_000 values, and a merge is required.

Fixes a off by one error in the stats for the index fast field in the multi value fast field.
When retrieving the data range for a docid, `get(doc)..get(docid+1)` is requested. On creation
the num_vals statistic was set to doc instead of docid + 1. In the multivaluelinearinterpol fast
field the last value was therefore not serialized (and would return 0 instead in most cases).
So the last document get(lastdoc)..get(lastdoc + 1) would return the invalid range `value..0`.

This PR adds a proptest to cover this scenario. A combination of a large number values, since multilinear
interpolation is only active for more than 5_000 values, and a merge is required.
@codecov
Copy link

codecov bot commented Sep 8, 2021

Codecov Report

Merging #1152 (896cdd6) into main (319609e) will increase coverage by 0.02%.
The diff coverage is 96.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1152      +/-   ##
==========================================
+ Coverage   93.74%   93.77%   +0.02%     
==========================================
  Files         203      203              
  Lines       33768    33654     -114     
==========================================
- Hits        31656    31559      -97     
+ Misses       2112     2095      -17     
Impacted Files Coverage Δ
src/fastfield/multivalued/mod.rs 94.36% <95.52%> (+1.54%) ⬆️
src/indexer/index_writer.rs 97.23% <100.00%> (+0.02%) ⬆️
src/indexer/merger.rs 98.75% <100.00%> (+0.37%) ⬆️
src/directory/watch_event_router.rs 96.18% <0.00%> (+0.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 319609e...896cdd6. Read the comment docs.

src/indexer/merger.rs Outdated Show resolved Hide resolved
src/indexer/merger.rs Show resolved Hide resolved
src/indexer/merger.rs Outdated Show resolved Hide resolved
src/indexer/merger.rs Outdated Show resolved Hide resolved
src/indexer/merger.rs Outdated Show resolved Hide resolved
@fulmicoton fulmicoton merged commit 3bc177e into main Sep 10, 2021
@fulmicoton fulmicoton deleted the uberflow branch September 10, 2021 14:00
This was referenced Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants