LindaSummer commented on code in PR #3268:
URL: https://github.com/apache/kvrocks/pull/3268#discussion_r2554118673
##########
src/types/redis_tdigest.h:
##########
@@ -33,6 +33,115 @@
#include "tdigest.h"
namespace redis {
+
+// TODO: It should be replaced by a iteration of the rocksdb iterator
+template <bool Reverse>
+class DummyCentroids {
+ public:
+ DummyCentroids(const TDigestMetadata& meta_data, const
std::vector<Centroid>& centroids)
+ : meta_data_(meta_data), centroids_(centroids) {}
+ class Iterator {
+ public:
+ using IterType = std::conditional_t<Reverse,
std::vector<Centroid>::const_reverse_iterator,
+ std::vector<Centroid>::const_iterator>;
+
+ Iterator(IterType iter, const std::vector<Centroid>& centroids) :
iter_(iter), centroids_(centroids) {}
+ std::unique_ptr<Iterator> Clone() const {
+ if constexpr (Reverse) {
+ if (iter_ != centroids_.crend()) {
Review Comment:
Maybe we could add a private method like below.
```cpp
class Ietrator {
...
private:
template <typename Container>
decltype(auto) get_cbegin_iter(Container centroids) const {
if constexpr (Reverse) {
return centroids.rcbegin();
} else {
return centroids.cbegin() ;
}
}
template <typename Container>
decltype(auto) get_cend_iter(Container centroids) const {
if constexpr (Reverse) {
return centroids.rcend();
} else {
return centroids.cend() ;
}
}
```
Then we could unify the `centroids.cebgin()` and `centroids.cend()`.
This could help us reduce the `constexpr if` in every iterator getter.
##########
src/types/redis_tdigest.h:
##########
@@ -132,4 +242,39 @@ class TDigest : public SubKeyScanner {
Centroid* centroid) const;
};
+template <bool Reverse>
+rocksdb::Status TDigest::Rank(engine::Context& ctx, const Slice& digest_name,
const std::vector<double>& inputs,
+ std::vector<int>& result) {
+ auto ns_key = AppendNamespacePrefix(digest_name);
+ TDigestMetadata metadata;
+ {
+ LockGuard guard(storage_->GetLockManager(), ns_key);
+
+ if (auto status = getMetaDataByNsKey(ctx, ns_key, &metadata);
!status.ok()) {
+ return status;
+ }
+
+ if (metadata.total_observations == 0) {
+ result.resize(inputs.size(), -2);
+ return rocksdb::Status::OK();
+ }
+
+ if (auto status = mergeNodes(ctx, ns_key, &metadata); !status.ok()) {
+ return status;
+ }
+ }
+
+ std::vector<Centroid> centroids;
+ if (auto status = dumpCentroids(ctx, ns_key, metadata, ¢roids);
!status.ok()) {
+ return status;
+ }
+
+ auto dump_centroids = DummyCentroids<Reverse>(metadata, centroids);
+ auto status = TDigestRank<Reverse>(dump_centroids, inputs, result);
+ if (!status) {
Review Comment:
We could follow sonarqube and use the init-statement.
```suggestion
if (auto status = TDigestRank<Reverse>(dump_centroids, inputs, result);
!status) {
```
##########
src/types/tdigest.h:
##########
@@ -266,13 +266,4 @@ inline Status TDigestRankImpl(TD&& td, const
std::vector<double>& inputs, std::v
}
}
return Status::OK();
-}
-
-template <typename TD>
-inline Status TDigestRank(TD&& td, const std::vector<double>& inputs, bool
reverse, std::vector<int>& result) {
- if (reverse) {
- return TDigestRankImpl<TD, true>(std::forward<TD>(td), inputs, result);
- } else {
- return TDigestRankImpl<TD, false>(std::forward<TD>(td), inputs, result);
- }
-}
+}
Review Comment:
Add an endline for this file.
--
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]
