git-hulk commented on code in PR #2508:
URL: https://github.com/apache/kvrocks/pull/2508#discussion_r1736256826
##########
src/search/indexer.cc:
##########
@@ -226,13 +226,19 @@ Status IndexUpdater::UpdateTagIndex(engine::Context &ctx,
std::string_view key,
for (const auto &tag : tags_to_delete) {
auto index_key = search_key.ConstructTagFieldData(tag, key);
- batch->Delete(cf_handle, index_key);
+ auto rocker_status = batch->Delete(cf_handle, index_key);
+ if (!rocker_status.ok()) {
+ return {Status::NotOK, rocker_status.ToString()};
+ }
}
for (const auto &tag : tags_to_add) {
auto index_key = search_key.ConstructTagFieldData(tag, key);
- batch->Put(cf_handle, index_key, Slice());
+ auto rocker_status = batch->Put(cf_handle, index_key, Slice());
Review Comment:
Same as above.
##########
src/search/indexer.cc:
##########
@@ -226,13 +226,19 @@ Status IndexUpdater::UpdateTagIndex(engine::Context &ctx,
std::string_view key,
for (const auto &tag : tags_to_delete) {
auto index_key = search_key.ConstructTagFieldData(tag, key);
- batch->Delete(cf_handle, index_key);
+ auto rocker_status = batch->Delete(cf_handle, index_key);
Review Comment:
Why not name it to `s` to keep consistent with others.
##########
src/types/redis_zset.cc:
##########
@@ -83,14 +84,17 @@ rocksdb::Status ZSet::Add(engine::Context &ctx, const Slice
&user_key, ZAddFlags
old_score_bytes.append(it->member);
std::string old_score_key =
InternalKey(ns_key, old_score_bytes, metadata.version,
storage_->IsSlotIdEncoded()).Encode();
- batch->Delete(score_cf_handle_, old_score_key);
+ s = batch->Delete(score_cf_handle_, old_score_key);
+ if (!s.ok() && !s.IsNotFound()) return s;
Review Comment:
IsNotFound should never happen in batch->Delete and batch->Put?
##########
src/types/redis_zset.cc:
##########
@@ -101,18 +105,21 @@ rocksdb::Status ZSet::Add(engine::Context &ctx, const
Slice &user_key, ZAddFlags
}
std::string score_bytes;
PutDouble(&score_bytes, it->score);
- batch->Put(member_key, score_bytes);
+ s = batch->Put(member_key, score_bytes);
+ if (!s.ok() && !s.IsNotFound()) return s;
Review Comment:
same as above
##########
src/types/redis_stream.cc:
##########
@@ -1619,11 +1672,12 @@ uint64_t Stream::trim(engine::Context &ctx, const
std::string &ns_key, const Str
metadata->recorded_first_entry_id.Clear();
}
- if (ret > 0) {
+ if (delete_cnt > 0) {
metadata->max_deleted_entry_id = entryIDFromInternalKey(last_deleted);
}
- return ret;
+ return rocksdb::Status::OK();
+ ;
Review Comment:
```suggestion
return rocksdb::Status::OK();
```
##########
src/types/redis_stream.cc:
##########
@@ -1619,11 +1672,12 @@ uint64_t Stream::trim(engine::Context &ctx, const
std::string &ns_key, const Str
metadata->recorded_first_entry_id.Clear();
}
- if (ret > 0) {
+ if (delete_cnt > 0) {
metadata->max_deleted_entry_id = entryIDFromInternalKey(last_deleted);
}
- return ret;
+ return rocksdb::Status::OK();
+ ;
Review Comment:
Can use `StatusOr` instead of introducing a new input reference.
--
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]
