salvatore-campagna commented on PR #16629: URL: https://github.com/apache/lucene/pull/16629#issuecomment-5538915580
Thanks @hossman for the repro and the float/double test, nice catch. +1 to reverting. Reading back through it, the real issue is that `NumericFieldStats.getStats()` sources the min/max from points while `SortedNumericDocValuesRangeQuery` executes against doc values, and the index never guarantees those two structures encode (or even hold) the same values. So for negative floats and doubles the stats read from points can land in a different number space than the query bounds, depending on how the doc values were encoded. On keeping `NumericFieldStats` with a decoder param (@romseygeek): a decoder does fix how we interpret point values (the current `decodeLong` hardcodes one encoding, which is its own latent bug), but I don't think it addresses this particular failure, since we would still be feeding stats read from points into a doc values query. For the doc values range path the `DocValuesSkipper` already gives correct stats in the same space. Would it make sense to keep the points stats and the skipper stats as separate methods, so a caller can only ever get stats from the structure it is actually querying? My understanding is that the important things here are: * for a query using doc values, derive stats from doc values (the skipper) * for a query using points, derive stats from points That said, since each path already has its own stats source, I would lean toward removing it rather than keeping even separate methods. The whole point of `NumericFieldStats` was to produce stats no matter whether they come from points or the skipper, and that is exactly what is unsafe here, because points and doc values are not guaranteed to be in the same space. Removing it does not lose any capability, it just removes the footgun. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
