apache / apache/datasketches-rust

Define consistent fallible semantics for sketch query APIs

Open
#266 3 comments 0 reactions 0 assignees View on GitHub
good first issue help wanted
Dominant language
Rust
Stars
123
Forks
45
Avg merge
9h 3m
Merged PRs (30d)
52

Description

## 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, 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;
```

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;
```

- `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.

Contributor guide

Open the contributing guide

Research direction

Start with the T-Digest, KLL, and REQ query APIs and the crate's ErrorKind and deserialization boundaries; compare their current behavior with the referenced C++ tdigest_impl.hpp, kll_sketch_impl.hpp, and Java query classes. Done means a documented crate-wide policy is agreed, the affected APIs follow it consistently, and tests cover empty sketches, invalid arguments, malformed serialized data, and internal invariants.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend-api-design, data
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.