acelyc111 commented on code in PR #1091:
URL: https://github.com/apache/incubator-pegasus/pull/1091#discussion_r937403751
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -55,7 +55,35 @@
pegasus_client_impl::pegasus_scanner_impl::pegasus_scanner_impl(::dsn::apps::rrd
_rpc_started(false),
_validate_partition_hash(validate_partition_hash),
_full_scan(full_scan)
+
Review Comment:
remove the blank line
##########
idl/rrdb.thrift:
##########
@@ -279,11 +279,15 @@ struct get_scanner_request
11:optional bool validate_partition_hash;
12:optional bool return_expire_ts;
13:optional bool full_scan; // true means client want to build 'full scan'
context with the server side, false otherwise
+ 14:optional filter_type value_filter_type;
+ 15:optional dsn.blob value_filter_pattern;
Review Comment:
Are the two parameters related to this patch?
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -162,13 +194,15 @@ void
pegasus_client_impl::pegasus_scanner_impl::_async_next_internal()
}
// valid data got
- std::string hash_key, sort_key;
- pegasus_restore_key(_kvs[_p].key, hash_key, sort_key);
- std::string value(_kvs[_p].value.data(), _kvs[_p].value.length());
- uint32_t expire_ts_seconds = _kvs[_p].__isset.expire_ts_seconds
- ?
static_cast<uint32_t>(_kvs[_p].expire_ts_seconds)
- : 0;
-
+ std::string hash_key, sort_key, value = "";
Review Comment:
```suggestion
std::string hash_key, sort_key, value;
```
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -162,13 +194,15 @@ void
pegasus_client_impl::pegasus_scanner_impl::_async_next_internal()
}
// valid data got
- std::string hash_key, sort_key;
- pegasus_restore_key(_kvs[_p].key, hash_key, sort_key);
- std::string value(_kvs[_p].value.data(), _kvs[_p].value.length());
- uint32_t expire_ts_seconds = _kvs[_p].__isset.expire_ts_seconds
- ?
static_cast<uint32_t>(_kvs[_p].expire_ts_seconds)
- : 0;
-
+ std::string hash_key, sort_key, value = "";
+ uint32_t expire_ts_seconds = 0;
+ if (_kv_count == -1) {
Review Comment:
it suggest to add some comment to explain what dose `_kv_count == -1` mean
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -1389,6 +1419,11 @@ void pegasus_server_impl::on_scan(scan_rpc rpc)
it->Next();
}
+ if (request.only_return_count) {
Review Comment:
you can use the flag in pegasus_scan_context, right?
##########
src/client_lib/pegasus_scanner_impl.cpp:
##########
@@ -55,7 +55,35 @@
pegasus_client_impl::pegasus_scanner_impl::pegasus_scanner_impl(::dsn::apps::rrd
_rpc_started(false),
_validate_partition_hash(validate_partition_hash),
_full_scan(full_scan)
+
+{
+}
+
+int pegasus_client_impl::pegasus_scanner_impl::next(std::string &hashkey,
Review Comment:
why not reuse the exist `pegasus_client_impl::pegasus_scanner_impl::next` ?
##########
idl/rrdb.thrift:
##########
@@ -279,11 +279,15 @@ struct get_scanner_request
11:optional bool validate_partition_hash;
12:optional bool return_expire_ts;
13:optional bool full_scan; // true means client want to build 'full scan'
context with the server side, false otherwise
+ 14:optional filter_type value_filter_type;
+ 15:optional dsn.blob value_filter_pattern;
+ 16:optional bool only_return_count = false;
}
struct scan_request
{
1:i64 context_id;
+ 2:optional bool only_return_count;
Review Comment:
the `get_scanner_request ` has a `only_return_count` field, why add another
one to `scan_request`?
--
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]
