Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

fixed cleos get_kv_table_rows bugs #9595

Merged
merged 2 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 96 additions & 73 deletions plugins/chain_plugin/chain_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1987,7 +1987,7 @@ void read_only::convert_key(const string& index_type, const string& encode_type,
string bytes_data = boost::algorithm::unhex(index_value);
auto bytes_datasize = bytes_data.size();
if( bin.size() < bytes_datasize ) {
bin.resize(index_value.size());
bin.resize(bytes_datasize);
}
memcpy(bin.data(), bytes_data.data(), bytes_datasize);
return;
Expand Down Expand Up @@ -2127,7 +2127,6 @@ read_only::get_table_rows_result read_only::get_kv_table_rows( const read_only::
const abi_def abi = eosio::chain_apis::get_abi(db, p.code);
name database_id = chain::kvram_id;

// Enable the code block once rocksdb_nodeos_integratin is merged
const chain::kv_database_config &limits = kv_config;
auto &kv_database = const_cast<chain::controller&>(db).kv_db();
// To do: provide kv_resource_manmager to create_kv_context
Expand All @@ -2136,7 +2135,8 @@ read_only::get_table_rows_result read_only::get_kv_table_rows( const read_only::
return get_kv_table_rows_context(p, *kv_context, abi);
}

read_only::get_table_rows_result read_only::get_kv_table_rows_context( const read_only::get_kv_table_rows_params& p, kv_context &kv_context, const abi_def &abi )const {
read_only::get_table_rows_result read_only::get_kv_table_rows_context( const read_only::get_kv_table_rows_params& pp, kv_context &kv_context, const abi_def &abi )const {
read_only::get_kv_table_rows_params p(pp);
string tbl_name = p.table.to_string();

// Check valid table name
Expand Down Expand Up @@ -2172,80 +2172,59 @@ read_only::get_table_rows_result read_only::get_kv_table_rows_context( const rea
auto end_time = cur_time + fc::microseconds(1000 * 10);

auto wait_time = end_time - cur_time;
bool unbounded = false;
///////////////////////////////////////////////////////////
// point query
///////////////////////////////////////////////////////////
if( point_query ) {
const string &index_value = *p.index_value;
vector<char> iv;
convert_key(index_type, p.encode_type, index_value, iv);
vector<char> key;
key.resize(prefix.size() + iv.size());
memcpy(key.data(), prefix.data(), prefix.size());
memcpy(key.data() + prefix.size(), iv.data(), iv.size());

fc::variant row_var;
auto success = kv_context.kv_get(p.code.to_uint64_t(), key.data(), key.size(), value_size);
if( success ) {
vector<char> value;
value.resize(value_size);
auto return_size = kv_context.kv_get_data(offset, value.data(), value_size);
EOS_ASSERT(return_size == value_size, chain::contract_table_query_exception, "point query value size mismatch: ${s1} ${s2}", ("s1", return_size)("s2", value_size));

if( !is_primary_idx) {
success = kv_context.kv_get(p.code.to_uint64_t(), value.data(), value.size(), value_size);
if (success) {
value.resize(value_size);
return_size = kv_context.kv_get_data(offset, value.data(), value_size);
EOS_ASSERT(return_size == value_size, chain::contract_table_query_exception, "point query value size mismatch: ${s1} ${s2}", ("s1", return_size)("s2", value_size));
}
}

fc::variant row_var;
if( p.json ) {
try {
row_var = abis.binary_to_variant( p.table.to_string(), value, abi_serializer::create_yield_function( wait_time ), shorten_abi_errors );
} catch ( fc::exception &e ) {
row_var = fc::variant(value);
}
} else {
row_var = fc::variant(value);
}
result.rows.emplace_back( std::move(row_var) );
}

return result;
p.lower_bound = p.index_value;
unbounded = true;
}

///////////////////////////////////////////////////////////
// ranged query
///////////////////////////////////////////////////////////
bool has_lower_bound = p.lower_bound.has_value() && !p.lower_bound.value().empty();
bool has_upper_bound = p.upper_bound.has_value() && !p.upper_bound.value().empty();
EOS_ASSERT(has_lower_bound && has_upper_bound, chain::contract_table_query_exception, "Unknown range: ${t} ${i}", ("t", p.table)("i", p.index_name));
unbounded = !has_lower_bound || !has_upper_bound;
// reverse mode has upper_bound value, non-reverse and point query has lower bound value
EOS_ASSERT((p.reverse && has_upper_bound) || (!p.reverse && has_lower_bound) || (has_lower_bound && point_query), chain::contract_table_query_exception, "Unknown/Invalid range: ${t} ${i}", ("t", p.table)("i", p.index_name));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can change (!p.reverse && has_lower_bound) || (has_lower_bound && point_query) to (!p.reverse || point_query) && has_lower_bound) to match the comment and be consistent with (p.reverse && has_upper_bound)


vector<char> lb_key;
const string &lower_bound = *p.lower_bound;
vector<char> lv;
convert_key(index_type, p.encode_type, lower_bound, lv);
lb_key.resize(prefix.size() + lv.size());
memcpy(lb_key.data(), prefix.data(), prefix.size());
memcpy(lb_key.data() + prefix.size(), lv.data(), lv.size());

if( has_lower_bound ) {
convert_key(index_type, p.encode_type, lower_bound, lv);
lb_key.resize(prefix.size() + lv.size());
memcpy(lb_key.data(), prefix.data(), prefix.size());
memcpy(lb_key.data() + prefix.size(), lv.data(), lv.size());
}

vector<char> ub_key;
const string &upper_bound = *p.upper_bound;
vector<char> uv;
convert_key(index_type, p.encode_type, upper_bound, uv);
ub_key.resize(prefix.size() + uv.size());
memcpy(ub_key.data(), prefix.data(), prefix.size());
memcpy(ub_key.data() + prefix.size(), uv.data(), uv.size());
if( has_upper_bound ) {
convert_key(index_type, p.encode_type, upper_bound, uv);
ub_key.resize(prefix.size() + uv.size());
memcpy(ub_key.data(), prefix.data(), prefix.size());
memcpy(ub_key.data() + prefix.size(), uv.data(), uv.size());
}

// Iterate the range
vector<char> row_key;
vector<char> row_value;

auto lb_itr = kv_context.kv_it_create(p.code.to_uint64_t(), prefix.data(), prefix.size());
auto ub_itr = kv_context.kv_it_create(p.code.to_uint64_t(), prefix.data(), prefix.size());
std::unique_ptr<kv_iterator> lb_itr;
std::unique_ptr<kv_iterator> ub_itr;

if( has_lower_bound ) {
lb_itr = kv_context.kv_it_create(p.code.to_uint64_t(), prefix.data(), prefix.size());
}
if( has_upper_bound ) {
ub_itr = kv_context.kv_it_create(p.code.to_uint64_t(), prefix.data(), prefix.size());
}

uint32_t lb_key_size = 0;
uint32_t lb_value_size = 0;
Expand All @@ -2255,19 +2234,33 @@ read_only::get_table_rows_result read_only::get_kv_table_rows_context( const rea
uint32_t ub_key_actual_size = 0;

// Find lower bound iterator
auto status_lb = lb_itr->kv_it_lower_bound(lb_key.data(), lb_key.size(), &lb_key_size, &lb_value_size);
EOS_ASSERT(status_lb != chain::kv_it_stat::iterator_erased, chain::contract_table_query_exception, "Invalid lower bound iterator in ${t} ${i}", ("t", p.table)("i", p.index_name));
lb_key.resize(lb_key_size);
status_lb = lb_itr->kv_it_key(0, lb_key.data(), lb_key_size, lb_key_actual_size);
auto status_lb = chain::kv_it_stat::iterator_end;
if( has_lower_bound ) {
status_lb = lb_itr->kv_it_lower_bound(lb_key.data(), lb_key.size(), &lb_key_size, &lb_value_size);
EOS_ASSERT(status_lb != chain::kv_it_stat::iterator_erased, chain::contract_table_query_exception, "Invalid lower bound iterator in ${t} ${i}", ("t", p.table)("i", p.index_name));

if( !point_query ) {
lb_key.resize(lb_key_size);
status_lb = lb_itr->kv_it_key(0, lb_key.data(), lb_key_size, lb_key_actual_size);
}
}

// Find upper bound iterator
auto status_ub = ub_itr->kv_it_lower_bound(ub_key.data(), ub_key.size(), &ub_key_size, &ub_value_size);
EOS_ASSERT(status_ub != chain::kv_it_stat::iterator_erased, chain::contract_table_query_exception, "Invalid upper bound iterator in ${t} ${i}", ("t", p.table)("i", p.index_name));
ub_key.resize(ub_key_size);
status_ub = ub_itr->kv_it_key(0, ub_key.data(), ub_key_size, ub_key_actual_size);
auto status_ub = chain::kv_it_stat::iterator_end;
if( has_upper_bound ) {
auto exact_match = kv_context.kv_get(p.code.to_uint64_t(), ub_key.data(), ub_key.size(), value_size);
status_ub = ub_itr->kv_it_lower_bound(ub_key.data(), ub_key.size(), &ub_key_size, &ub_value_size);
if( p.reverse && !point_query && !exact_match ) {
status_ub = ub_itr->kv_it_prev(&ub_key_size, &ub_value_size);
}

EOS_ASSERT(status_ub != chain::kv_it_stat::iterator_erased, chain::contract_table_query_exception, "Invalid upper bound iterator in ${t} ${i}", ("t", p.table)("i", p.index_name));
ub_key.resize(ub_key_size);
status_ub = ub_itr->kv_it_key(0, ub_key.data(), ub_key_size, ub_key_actual_size);
}

kv_it_stat status;
// iterate the range
//=============================== iterate the range ============================
auto walk_table_row_range = [&]( auto &itr, auto &end_itr, bool reverse ) {
uint32_t actual_size = 0;

Expand All @@ -2279,25 +2272,42 @@ read_only::get_table_rows_result read_only::get_kv_table_rows_context( const rea
key_size = lb_key_size;
value_size = lb_value_size;
}
auto cmp = itr->kv_it_key_compare(end_key.data(), end_key.size());
if( reverse ) cmp *= -1;

auto cur_status = itr->kv_it_status();
int32_t cmp = 0;
if( unbounded ) {
cmp = (cur_status == chain::kv_it_stat::iterator_ok ) ? -1 : 1;
} else {
cmp = itr->kv_it_key_compare(end_key.data(), end_key.size());
if( reverse ) {
cmp *= -1;
}
}

unsigned int count = 0;
for( count = 0; cur_time <= end_time && count < p.limit && cmp < 0; cur_time = fc::time_point::now() ) {
row_key.resize(key_size);
status = itr->kv_it_key(0, row_key.data(), key_size, value_size);
if( point_query ) {
if( row_key.size() != lb_key_size || row_key != lb_key) {
cmp = 1;
continue;
}
}

row_value.clear();
row_value.resize(value_size);
status = itr->kv_it_value(offset, row_value.data(), value_size, actual_size);
EOS_ASSERT(status_lb == chain::kv_it_stat::iterator_ok, chain::contract_table_query_exception, "Invalid key in ${t} ${i}", ("t", p.table)("i", p.index_name));
status = itr->kv_it_value(offset, row_value.data(), key_size, value_size);
EOS_ASSERT(status == chain::kv_it_stat::iterator_ok, chain::contract_table_query_exception, "Invalid key in ${t} ${i}", ("t", p.table)("i", p.index_name));

if (!is_primary_idx) {
std::swap(row_key, row_value);
auto success = kv_context.kv_get(p.code.to_uint64_t(), row_key.data(), row_key.size(), value_size);
auto success = kv_context.kv_get(p.code.to_uint64_t(), row_value.data(), row_value.size(), value_size);
if (success) {
row_value.resize(value_size);
actual_size = kv_context.kv_get_data(offset, row_value.data(), value_size);
EOS_ASSERT(value_size == actual_size, chain::contract_table_query_exception, "range query value size mismatch: ${s1} ${s2}", ("s1", value_size)("s2", actual_size));
} else {
EOS_ASSERT(false, chain::contract_table_query_exception, "range query failed to get data: ${t} ${i}", ("t", p.table)("i", p.index_name));
EOS_ASSERT(value_size == actual_size, chain::contract_table_query_exception, "range query value size mismatch: ${s1} ${s2}", ("s1", value_size)("s2", actual_size));
}
}

Expand Down Expand Up @@ -2325,16 +2335,29 @@ read_only::get_table_rows_result read_only::get_kv_table_rows_context( const rea
}
EOS_ASSERT(status != chain::kv_it_stat::iterator_erased, chain::contract_table_query_exception, "Invalid lower bound iterator in ${t} ${i}", ("t", p.table)("i", p.index_name));

cmp = itr->kv_it_key_compare(end_key.data(), end_key.size());
if( reverse ) cmp *= -1;
if (unbounded) {
cur_status = itr->kv_it_status();
cmp = (cur_status == chain::kv_it_stat::iterator_ok) ? -1 : 1;
if( point_query && cmp != 0) {
cmp = 1;
}
} else {
cmp = itr->kv_it_key_compare(end_key.data(), end_key.size());
if (reverse) {
cmp *= -1;
}
}

if( count == p.limit && cmp < 0 ) {
if (count == p.limit && cmp < 0)
{
result.more = true;
row_key.resize(key_size);
auto status = itr->kv_it_key(0, row_key.data(), key_size, value_size);
EOS_ASSERT(status != chain::kv_it_stat::iterator_erased && value_size >= prefix_size(), chain::contract_table_query_exception, "Invalid lower bound iterator in ${t} ${i}", ("t", p.table)("i", p.index_name));
result.next_key = string(&row_key.data()[prefix_size()], row_key.size() - prefix_size());
}
auto next_key = string(&row_key.data()[prefix_size()], row_key.size() - prefix_size());

boost::algorithm::hex(next_key.begin(), next_key.end(), std::back_inserter(result.next_key));
}
} // end of for
};

Expand Down
3 changes: 2 additions & 1 deletion programs/cleos/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2881,12 +2881,13 @@ int main( int argc, char** argv ) {
getKvTable->add_option( "table", table, localized("The name of the kv_table as specified by the contract abi") )->required();
getKvTable->add_option( "index_name", index_name, localized("The name of the kv_table index as specified by the contract abi") )->required();
getKvTable->add_option( "-l,--limit", limit, localized("The maximum number of rows to return") );
getKvTable->add_option( "-k,--key", index_value, localized("Index value") );
getKvTable->add_option("-i,--index", index_value, localized("Index value"));
getKvTable->add_option( "-L,--lower", lower, localized("JSON representation of lower bound value of key, defaults to first") );
getKvTable->add_option( "-U,--upper", upper, localized("JSON representation of upper bound value of key, defaults to last") );
getKvTable->add_option( "--encode-type", encode_type,
localized("The encoding type of index_value, lower bound, upper bound"
" 'bytes' for hexdecimal encoded bytes"
" 'string' for string value"
" 'dec' for decimal encoding of (uint[64|32|16|8], int[64|32|16|8], float64)"
" 'hex' for hexdecimal encoding of (uint[64|32|16|8], int[64|32|16|8], sha256, ripemd160" ));
getKvTable->add_flag("-b,--binary", binary, localized("Return the value as BINARY rather than using abi to interpret as JSON"));
Expand Down
Loading