Skip to content

fix: unify sketch query semantics - #273

Open
Phoenix500526 wants to merge 1 commit into
apache:mainfrom
Phoenix500526:issue/266-query-semantics
Open

fix: unify sketch query semantics#273
Phoenix500526 wants to merge 1 commit into
apache:mainfrom
Phoenix500526:issue/266-query-semantics

Conversation

@Phoenix500526

Copy link
Copy Markdown
Contributor

Distinguish valid empty sketches from invalid query arguments across T-Digest, KLL, and REQ without changing deserialization errors.

CLOSES #266

@tisonkun

tisonkun commented Sep 4, 2026

Copy link
Copy Markdown
Member

Distinguish valid empty sketches from invalid query arguments across T-Digest, KLL, and REQ without changing deserialization errors.

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.

@Phoenix500526

Copy link
Copy Markdown
Contributor Author

Distinguish valid empty sketches from invalid query arguments across T-Digest, KLL, and REQ without changing deserialization errors.

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
@Phoenix500526
Phoenix500526 force-pushed the issue/266-query-semantics branch from 2abf36e to 7283c3f Compare September 4, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Define consistent fallible semantics for sketch query APIs

2 participants