tisonkun opened a new issue, #266: URL: https://github.com/apache/datasketches-rust/issues/266
## Context The Rust implementations currently use different error models for distribution queries: - T-Digest returns `Option` when querying an empty sketch and panics for invalid ranks, NaN rank inputs, or invalid split points. - KLL and REQ return `Result`, but currently classify an empty sketch as `InvalidArgument` alongside malformed query arguments. - Deserializers separately report malformed serialized state as `InvalidData`. An empty sketch is a valid object state: it can be created, serialized, merged, and updated. However, quantile, rank, CDF, and PMF values are undefined until the sketch has observed data. That condition is distinct from an invalid query argument and from corrupted serialized data. The C++ and Java implementations preserve this distinction with exception categories: - DataSketches C++ T-Digest and KLL throw `std::runtime_error` for queries on empty sketches and `std::invalid_argument` for invalid query parameters. - DataSketches Java T-Digest throws `SketchesStateException` for queries on empty sketches and `SketchesArgumentException` for invalid query parameters. - DataSketches Java KLL is a notable inconsistency: it reports empty-query failures as `SketchesArgumentException`. References: - C++ T-Digest query behavior: https://github.com/apache/datasketches-cpp/blob/master/tdigest/include/tdigest_impl.hpp - C++ KLL query behavior: https://github.com/apache/datasketches-cpp/blob/master/kll/include/kll_sketch_impl.hpp - Java T-Digest query behavior: https://github.com/apache/datasketches-java/blob/main/src/main/java/org/apache/datasketches/tdigest/TDigestDouble.java - Java KLL query behavior: https://github.com/apache/datasketches-java/blob/main/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java ## Problem The crate does not yet have a documented policy for these independent conditions: 1. The sketch is valid but contains no data. 2. The caller supplies an invalid rank, NaN value, or invalid split points. 3. Serialized input is malformed. 4. An internal invariant fails, indicating an implementation bug. As a result, similar query APIs differ in return types, panic behavior, and error classification. Adding more batch query APIs will make that inconsistency more visible. ## Design questions We should decide which Rust model applies consistently to T-Digest, KLL, and REQ queries. ### A. Keep absence separate from caller errors ```rust fn quantile(&self, rank: f64) -> Result<Option<f64>, Error>; ``` - `Ok(Some(value))`: query succeeded. - `Ok(None)`: the sketch is empty. - `Err(InvalidArgument)`: the rank is invalid. This is precise but introduces nested handling for every query. ### B. Model an undefined query as a distinct recoverable error ```rust fn quantile(&self, rank: f64) -> Result<f64, Error>; ``` For example, extend `ErrorKind` with a distinct category such as `EmptySketch` or `UndefinedOperation`: - `Ok(value)`: query succeeded. - `Err(EmptySketch)`: the sketch is valid but the requested statistic is undefined. - `Err(InvalidArgument)`: the query parameter is invalid. - `Err(InvalidData)`: serialized input is malformed. This is the closest Rust analogue to the C++ and Java T-Digest distinction while keeping one return layer. ### C. Treat invalid query parameters as programming errors ```rust fn quantile(&self, rank: f64) -> Option<f64>; ``` - `Some(value)`: query succeeded. - `None`: the sketch is empty. - Panic: the query parameter violates the documented precondition. This matches the current T-Digest API and is compact, but it makes runtime-provided query parameters non-recoverable without prevalidation. ### D. Make invalid inputs unrepresentable Validated `NormalizedRank` and split-point wrapper types could leave queries returning `Option`, but the additional public types and conversion steps may be disproportionate to the problem. The discussion should also decide: - Whether `min_value` and `max_value` should remain `Option`, following ordinary Rust collection accessors, even if statistical query methods return `Result`. - Whether query arguments are validated before checking emptiness, so invalid input has deterministic behavior independent of sketch state. - Whether internal invariant failures remain assertions or `unreachable!` rather than becoming public recoverable errors. - Whether the policy applies only to quantile sketches initially or to all sketch families. ## Desired outcome Agree on and document a crate-wide error policy for sketch queries, then apply it consistently to the affected APIs with tests covering empty sketches, invalid arguments, malformed serialized data, and internal invariants at the appropriate boundaries. -- 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]
