SharonIV0x86 commented on code in PR #2849:
URL: https://github.com/apache/kvrocks/pull/2849#discussion_r2079721466


##########
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, &centroids); 
!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 && and 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.



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

Reply via email to