Address peer review comments. Better index position name handling. Remove...

Address peer review comments. Better index position name handling. Remove unneeded Scope template params.

plugins/chain_plugin/chain_plugin.cpp

@@ -768,6 +768,51 @@ read_only::get_info_results read_only::get_info(const read_only::get_info_params
    };
 }
 
+uint64_t read_only::get_table_index_name(const read_only::get_table_rows_params& p, bool& primary) {
+   using boost::algorithm::starts_with;
+   // see multi_index packing of index name
+   const uint64_t table = p.table;
+   uint64_t index = table & 0xFFFFFFFFFFFFFFF0ULL;
+   EOS_ASSERT( index == table, chain::contract_table_query_exception, "Unsupported table name: ${n}", ("n", p.table) );
+
+   primary = false;
+   uint64_t pos = 0;
+   if (p.index_position.empty() || p.index_position == "first" || p.index_position == "primary" || p.index_position == "one") {
+      primary = true;
+   } else if (starts_with(p.index_position, "sec") || p.index_position == "two") { // second, secondary
+   } else if (starts_with(p.index_position , "ter") || starts_with(p.index_position, "th")) { // tertiary, ternary, third, three
+      pos = 1;
+   } else if (starts_with(p.index_position, "fou")) { // four, fourth
+      pos = 2;
+   } else if (starts_with(p.index_position, "fi")) { // five, fifth
+      pos = 3;
+   } else if (starts_with(p.index_position, "six")) { // six, sixth
+      pos = 4;
+   } else if (starts_with(p.index_position, "sev")) { // seven, seventh
+      pos = 5;
+   } else if (starts_with(p.index_position, "eig")) { // eight, eighth
+      pos = 6;
+   } else if (starts_with(p.index_position, "nin")) { // nine, ninth
+      pos = 7;
+   } else if (starts_with(p.index_position, "ten")) { // ten, tenth
+      pos = 8;
+   } else {
+      try {
+         pos = fc::to_uint64( p.index_position );
+      } catch(...) {
+         EOS_ASSERT( false, chain::contract_table_query_exception, "Invalid index_position: ${p}", ("p", p.index_position));
+      }
+      if (pos < 2) {
+         primary = true;
+         pos = 0;
+      } else {
+         pos -= 2;
+      }
+   }
+   index |= (pos & 0x000000000000000FULL);
+   return index;
+}
+
 template<>
 uint64_t convert_to_type(const string& str, const string& desc) {
    uint64_t value = 0;
@@ -818,19 +863,30 @@ string get_table_type( const abi_def& abi, const name& table_name ) {
 read_only::get_table_rows_result read_only::get_table_rows( const read_only::get_table_rows_params& p )const {
    const abi_def abi = eosio::chain_apis::get_abi( db, p.code );
 
-   if (!p.key_type.empty() && !p.index_position.empty() && p.index_position != "first" && p.index_position != "primary" && p.index_position != "0" && p.index_position != "1") {
+   bool primary = false;
+   auto table_with_index = get_table_index_name( p, primary );
+   if( primary ) {
+      EOS_ASSERT( p.table == table_with_index, chain::contract_table_query_exception, "Invalid table name ${t}", ( "t", p.table ));
+      auto table_type = get_table_type( abi, p.table );
+      if( table_type == KEYi64 || p.key_type == "i64" || p.key_type == "name" ) {
+         return get_table_rows_ex<key_value_index>(p,abi);
+      }
+      EOS_ASSERT( false, chain::contract_table_query_exception,  "Invalid table type ${type}", ("type",table_type)("abi",abi));
+   } else {
+      EOS_ASSERT( !p.key_type.empty(), chain::contract_table_query_exception, "key type required for non-primary index" );
+
       if (p.key_type == "i64" || p.key_type == "name") {
-         return get_table_rows_by_seckey<index64_index, by_secondary, uint64_t>(p, abi, [](uint64_t v)->uint64_t {
+         return get_table_rows_by_seckey<index64_index, uint64_t>(p, abi, [](uint64_t v)->uint64_t {
             return v;
          });
       }
       else if (p.key_type == "i128") {
-         return get_table_rows_by_seckey<index128_index, by_secondary, uint128_t>(p, abi, [](uint128_t v)->uint128_t {
+         return get_table_rows_by_seckey<index128_index, uint128_t>(p, abi, [](uint128_t v)->uint128_t {
             return v;
          });
       }
       else if (p.key_type == "i256") {
-         return get_table_rows_by_seckey<index256_index, by_secondary, uint256_t>(p, abi, [](uint256_t v)->key256_t {
+         return get_table_rows_by_seckey<index256_index, uint256_t>(p, abi, [](uint256_t v)->key256_t {
             key256_t k;
             k[0] = ((uint128_t *)&v)[0];
             k[1] = ((uint128_t *)&v)[1];
@@ -838,13 +894,13 @@ read_only::get_table_rows_result read_only::get_table_rows( const read_only::get
          });
       }
       else if (p.key_type == "float64") {
-         return get_table_rows_by_seckey<index_double_index, by_secondary, double>(p, abi, [](double v)->float64_t {
+         return get_table_rows_by_seckey<index_double_index, double>(p, abi, [](double v)->float64_t {
             float64_t f = *(float64_t *)&v;
             return f;
          });
       }
       else if (p.key_type == "float128") {
-         return get_table_rows_by_seckey<index_long_double_index, by_secondary, double>(p, abi, [](double v)->float128_t{
+         return get_table_rows_by_seckey<index_long_double_index, double>(p, abi, [](double v)->float128_t{
             float64_t f = *(float64_t *)&v;
             float128_t f128;
             f64_to_f128M(f, &f128);
@@ -852,12 +908,6 @@ read_only::get_table_rows_result read_only::get_table_rows( const read_only::get
          });
       }
       EOS_ASSERT(false, chain::contract_table_query_exception,  "Unsupported secondary index type: ${t}", ("t", p.key_type));
-   } else {
-      auto table_type = get_table_type( abi, p.table );
-      if( table_type == KEYi64 || p.key_type == "i64" || p.key_type == "name" ) {
-         return get_table_rows_ex<key_value_index, by_scope_primary>(p,abi);
-      }
-      EOS_ASSERT( false, chain::contract_table_query_exception,  "Invalid table type ${type}", ("type",table_type)("abi",abi));
    }
 }
 
@@ -867,7 +917,7 @@ vector<asset> read_only::get_currency_balance( const read_only::get_currency_bal
    auto table_type = get_table_type( abi, "accounts" );
 
    vector<asset> results;
-   walk_table<key_value_index, by_scope_primary>(p.code, p.account, N(accounts), [&](const key_value_object& obj){
+   walk_key_value_table(p.code, p.account, N(accounts), [&](const key_value_object& obj){
       EOS_ASSERT( obj.value.size() >= sizeof(asset), chain::asset_type_exception, "Invalid data on table");
 
       asset cursor;
@@ -895,7 +945,7 @@ fc::variant read_only::get_currency_stats( const read_only::get_currency_stats_p
 
    uint64_t scope = ( eosio::chain::string_to_symbol( 0, boost::algorithm::to_upper_copy(p.symbol).c_str() ) >> 8 );
 
-   walk_table<key_value_index, by_scope_primary>(p.code, scope, N(stat), [&](const key_value_object& obj){
+   walk_key_value_table(p.code, scope, N(stat), [&](const key_value_object& obj){
       EOS_ASSERT( obj.value.size() >= sizeof(read_only::get_currency_stats_result), chain::asset_type_exception, "Invalid data on table");
 
       fc::datastream<const char *> ds(obj.value.data(), obj.value.size());

plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp

@@ -276,13 +276,13 @@ public:
       memcpy( data.data(), obj.value.data(), obj.value.size() );
    }
 
-   template<typename IndexType, typename Scope, typename Function>
-   void walk_table(const name& code, const name& scope, const name& table, Function f) const
+   template<typename Function>
+   void walk_key_value_table(const name& code, const name& scope, const name& table, Function f) const
    {
       const auto& d = db.db();
       const auto* t_id = d.find<chain::table_id_object, chain::by_code_scope_table>(boost::make_tuple(code, scope, table));
       if (t_id != nullptr) {
-         const auto &idx = d.get_index<IndexType, Scope>();
+         const auto &idx = d.get_index<chain::key_value_index, chain::by_scope_primary>();
          decltype(t_id->id) next_tid(t_id->id._id + 1);
          auto lower = idx.lower_bound(boost::make_tuple(t_id->id));
          auto upper = idx.lower_bound(boost::make_tuple(next_tid));
@@ -295,48 +295,9 @@ public:
       }
    }
 
-   static uint64_t get_table_index_name(const read_only::get_table_rows_params& p) {
-      // see multi_index packing of index name
-      const uint64_t table = p.table;
-      uint64_t index = table & 0xFFFFFFFFFFFFFFF0ULL;
-      EOS_ASSERT( index == table, chain::contract_table_query_exception, "Unsupported table name: ${n}", ("n", p.table) );
-
-      uint64_t pos = 0;
-      if (p.index_position.empty() || p.index_position == "first" || p.index_position == "primary") {
-      } else if (p.index_position == "second" || p.index_position == "secondary") {
-      } else if (p.index_position == "third" || p.index_position == "tertiary" || p.index_position == "ternary") {
-         pos = 1;
-      } else if (p.index_position == "fourth" || p.index_position == "quaternary") {
-         pos = 2;
-      } else if (p.index_position == "fifth" || p.index_position == "quinary") {
-         pos = 3;
-      } else if (p.index_position == "sixth" || p.index_position == "senary") {
-         pos = 4;
-      } else if (p.index_position == "seventh" || p.index_position == "septenary") {
-         pos = 5;
-      } else if (p.index_position == "eighth" || p.index_position == "octonary") {
-         pos = 6;
-      } else if (p.index_position == "ninth" || p.index_position == "nonary") {
-         pos = 7;
-      } else if (p.index_position == "tenth" || p.index_position == "denary") {
-         pos = 8;
-      } else {
-         try {
-            pos = fc::to_uint64( p.index_position );
-         } catch(...) {
-            EOS_ASSERT( false, chain::contract_table_query_exception, "Invalid index_position: ${p}", ("p", p.index_position));
-         }
-         if (pos < 2) {
-            pos = 0;
-         } else {
-            pos -= 2;
-         }
-      }
-      index |= (pos & 0x000000000000000FULL);
-      return index;
-   }
+   static uint64_t get_table_index_name(const read_only::get_table_rows_params& p, bool& primary);
 
-   template <typename IndexType, typename Scope, typename SecKeyType, typename ConvFn>
+   template <typename IndexType, typename SecKeyType, typename ConvFn>
    read_only::get_table_rows_result get_table_rows_by_seckey( const read_only::get_table_rows_params& p, const abi_def& abi, ConvFn conv )const {
       read_only::get_table_rows_result result;
       const auto& d = db.db();
@@ -345,11 +306,12 @@ public:
 
       abi_serializer abis;
       abis.set_abi(abi);
-      const uint64_t table_with_index = get_table_index_name(p);
+      bool primary = false;
+      const uint64_t table_with_index = get_table_index_name(p, primary);
       const auto* t_id = d.find<chain::table_id_object, chain::by_code_scope_table>(boost::make_tuple(p.code, scope, p.table));
       const auto* index_t_id = d.find<chain::table_id_object, chain::by_code_scope_table>(boost::make_tuple(p.code, scope, table_with_index));
       if (t_id != nullptr && index_t_id != nullptr) {
-         const auto &secidx = d.get_index<IndexType, Scope>();
+         const auto& secidx = d.get_index<IndexType, chain::by_secondary>();
          decltype(index_t_id->id) low_tid(index_t_id->id._id);
          decltype(index_t_id->id) next_tid(index_t_id->id._id + 1);
          auto lower = secidx.lower_bound(boost::make_tuple(low_tid));
@@ -382,9 +344,9 @@ public:
 
          unsigned int count = 0;
          auto itr = lower;
-         for (itr = lower; itr != upper; ++itr) {
+         for (; itr != upper; ++itr) {
 
-            const auto *itr2 = d.find<chain::key_value_object, chain::by_scope_primary>(boost::make_tuple(t_id->id, itr->primary_key));
+            const auto* itr2 = d.find<chain::key_value_object, chain::by_scope_primary>(boost::make_tuple(t_id->id, itr->primary_key));
             if (itr2 == nullptr) continue;
             copy_inline_row(*itr2, data);
 
@@ -405,7 +367,7 @@ public:
       return result;
    }
 
-   template <typename IndexType, typename Scope>
+   template <typename IndexType>
    read_only::get_table_rows_result get_table_rows_ex( const read_only::get_table_rows_params& p, const abi_def& abi )const {
       read_only::get_table_rows_result result;
       const auto& d = db.db();
@@ -416,7 +378,7 @@ public:
       abis.set_abi(abi);
       const auto* t_id = d.find<chain::table_id_object, chain::by_code_scope_table>(boost::make_tuple(p.code, scope, p.table));
       if (t_id != nullptr) {
-         const auto &idx = d.get_index<IndexType, Scope>();
+         const auto& idx = d.get_index<IndexType, chain::by_scope_primary>();
          decltype(t_id->id) next_tid(t_id->id._id + 1);
          auto lower = idx.lower_bound(boost::make_tuple(t_id->id));
          auto upper = idx.lower_bound(boost::make_tuple(next_tid));