fix: unify sketch query semantics - #273
Conversation
Could you give a brief on the design? Otherwise I have to reverse engineering the code to understand the intention. Even if with AI assistant the intention can drift. |
The query APIs currently handle empty sketches and invalid arguments inconsistently: KLL and REQ report an empty sketch as InvalidArgument, while T-Digest returns None for an empty sketch but panics on invalid query arguments. This PR adopts one contract across the three implementations. A fallible query returns Ok(Some(value)) for a valid query on a non-empty sketch, Ok(None) for a valid query on an empty sketch, and Err(InvalidArgument) for an invalid rank, NaN value, or invalid split points. Validation runs before the empty-state check, so an invalid query against an empty sketch still returns an error. Queries without an invalid-argument path, such as rank on KLL and REQ, return Option directly. Deserialization continues to return InvalidData; the serialization format and deserialization behavior are unchanged. The implementation applies this contract to mutable and immutable T-Digest queries and to both Sketch and SortedView for KLL and REQ. The rest of the diff updates callers, examples, benchmarks, and existing tests to the new return types, with regression coverage for empty queries, invalid arguments, and their precedence. |
Distinguish valid empty sketches from invalid query arguments across T-Digest, KLL, and REQ without changing deserialization errors. CLOSES apache#266
2abf36e to
7283c3f
Compare
Distinguish valid empty sketches from invalid query arguments across T-Digest, KLL, and REQ without changing deserialization errors.
CLOSES #266