Exposing min/max value interface on MultiValuedFastField Reader #1094
Conversation
| @@ -46,6 +46,24 @@ impl<Item: FastValue> MultiValuedFastFieldReader<Item> { | |||
| self.vals_reader.get_range_u64(range.start, &mut vals[..]); | |||
| } | |||
|
|
|||
| /// Returns the minimum value for this fast field. | |||
| /// | |||
| /// The max value does not take in account of possible | |||
There was a problem hiding this comment.
this comment seems like a copy-pasta
There was a problem hiding this comment.
Yes the comment looked like a warning that need to be clear to users from the internal reader. That's why I felt it's necessary to also have it on this methods.
| /// deleted document, and should be considered as an upper bound | ||
| /// of the actual maximum value. | ||
| pub fn min_value(&self) -> Item { | ||
| self.vals_reader.max_value() |
There was a problem hiding this comment.
out-of-context, is it deliberate to have min_value() return vals_reader.max_value() and vice versa?
There was a problem hiding this comment.
Thanks for catching that. @evanxg852000 can you fix?
There was a problem hiding this comment.
good catch indeed, By the way, I was worried by the fact they were no test on that.
In that case we should check in tantivy if a field is actually multivalued on serialization and use single value if applicable to reduce index size and read performance. |
|
Let's move discussion to this new pr |
Issue: as pointed out by @fmassot We want to avoid issues related to schema evolution in Quickwit. so currently fast fields are implicitly declared as multivalued.
We need the max/min value interface to be available for
MultiValuedFastFieldReaderas it is forFastFieldReaderCurrently, we need it to allow us to perform the timestamp filtering optimization. That is when a split time range is already within the user query bound, we should not attach filtering.