LindaSummer commented on code in PR #2849:
URL: https://github.com/apache/kvrocks/pull/2849#discussion_r2080867476
##########
src/types/redis_tdigest.cc:
##########
@@ -225,6 +225,10 @@ rocksdb::Status TDigest::Quantile(engine::Context& ctx,
const Slice& digest_name
if (auto status = dumpCentroids(ctx, ns_key, metadata, ¢roids);
!status.ok()) {
return status;
}
+ if (centroids.empty()) {
Review Comment:
> So just to be clear, after we load the metadata
[here](https://github.com/apache/kvrocks/blob/6719b607141fd2b07f5d5a967971fc1d6de0f87a/src/types/redis_tdigest.cc#L191C19-L191C27)
i add a check to do something like this.
>
>
>
> ```cpp
>
> if(metadata.merged_nodes == 0 && metadata.unmerged_nodes == 0){
>
> //tdigest does not contain any data. so set the flag
>
> }
>
> ```
>
> Correct me if im wrong.
>
>
>
> > And it would be better to add cpp unit test for this behavior.
>
>
>
> I'll try working on this as well.
Hi @SharonIV0x86 ,
We could directly use `total_observations` to check the tdigest's elements. 😊
Best Regards,
Edward
--
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]
