adriangb opened a new pull request, #23737:
URL: https://github.com/apache/datafusion/pull/23737

   ## Which issue does this PR close?
   
   No existing issue; this is a small additive API enhancement. Happy to file a 
tracking issue if preferred.
   
   <!-- - Closes #. -->
   
   ## Rationale for this change
   
   `TDigest` already exposes `to_scalar_state()` / `from_scalar_state()` so 
that external systems can persist and restore digest state. However, that 
contract requires the caller to pack and unpack the state through a 
`ScalarValue::List` of `ScalarValue::Float64` (which `from_scalar_state` 
immediately downcasts right back to an `&[f64]`).
   
   A caller that wants to serialize a digest into its own format (this arose 
from a production system persisting digests in its own serialization format) 
has to construct and then deconstruct `ScalarValue` lists purely to move data 
across that boundary. There is no way to read the `sum` field or the raw 
centroids, and no way to rebuild a digest from primitives.
   
   This PR exposes the identical external-state contract without the Arrow 
round-trip.
   
   ## What changes are included in this PR?
   
   Three additive, public API additions to `TDigest` (no behavior changes):
   
   - `pub fn from_parts(max_size: usize, sum: f64, count: f64, max: f64, min: 
f64, centroids: Vec<Centroid>) -> Self` — the non-Arrow counterpart to 
`from_scalar_state`. It mirrors `from_scalar_state`'s semantics exactly: 
`centroids` are trusted to be sorted by mean and are not re-sorted or validated 
(documented as a precondition, with a `debug_assert!`), `sum`/`count` are 
stored as given, and it replicates the same `max >= min` assertion for finite 
bounds.
   - `pub fn centroids(&self) -> &[Centroid]` — read access to the centroids 
(the missing half of the state; the individual `Centroid` `mean()`/`weight()` 
accessors already exist).
   - `pub fn sum(&self) -> f64` — read access to the `sum` field 
(`count`/`max`/`min`/`max_size` accessors already existed; `sum` did not).
   
   ## Are these changes tested?
   
   Yes, in the existing `tdigest` test module:
   
   - `test_from_parts_roundtrip` — for digests built from real value streams 
(empty, single value, and many values forcing compression), a digest rebuilt 
via `from_parts` from the public accessors produces a `to_scalar_state()` equal 
to the original's, and `estimate_quantile(q)` that is bitwise-equal across a 
quantile grid.
   - `test_from_parts_equals_original` — for non-empty digests, 
`from_parts(...)` equals the original under the derived `PartialEq`.
   - `test_accessors_agree_with_scalar_state` — `sum()` and `centroids()` agree 
with the values packed into `to_scalar_state()`.
   
   ## Are there any user-facing changes?
   
   Yes: new public API on `TDigest` (`from_parts`, `centroids`, `sum`). The 
change is purely additive: no existing API is modified or removed, and there 
are no behavior changes to existing methods.


-- 
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]

Reply via email to