-
Notifications
You must be signed in to change notification settings - Fork 3.8k
support secondary key in get table without the need of modifying abi file #4053
Conversation
programs/cleos/main.cpp
Outdated
@@ -1735,6 +1737,8 @@ int main( int argc, char** argv ) { | |||
getTable->add_option( "-k,--key", table_key, localized("The name of the key to index by as defined by the abi, defaults to primary key") ); | |||
getTable->add_option( "-L,--lower", lower, localized("JSON representation of lower bound value of key, defaults to first") ); | |||
getTable->add_option( "-U,--upper", upper, localized("JSON representation of upper bound value value of key, defaults to last") ); | |||
getTable->add_option( "-t,--seckeytype", sec_key_type, localized("Secondary key type(i64,i128,i256,float64,float128)")); | |||
getTable->add_option( "-p,--secindexposition", sec_index_position, localized("Secondary index position, default to 0(first secondary index)")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we will support diff primary key types in the future, the abbreviated -t -p
are too generic. Also too much implementation detail here. Seems odd to talk about first, second, third, etc secondary index. From the users perspective it should just be first (primary), second, third, etc.
How about:
"--index", "Index number, 1 - primary (first), 2 - secondary index (in order defined by mult_index), 3 - third index, etc."
"--key-type", "The key type of --index, primary currently only supports (i64), all others support (i64, i128, i256, float64, float128)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"-k,--key" should either be removed or made to work. Currently, it is not used for anything. However, to not break existing tools, leave it as part of the json, but mark it as deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wanderingbort suggested also supporting "primary", "secondary", etc as args to --index so the user does not have to use literal numbers.
This is direly needed, especially for eosjs / scatter which make use of the RPC. |
… is expected by multi_index.
This feels like a workaround for a mistake: the ABI format cleanly distinguishes among types, except for the indexes. Maybe we should revisit that. "senary"? "denary"? Are these Portuguese words? Italian? Could we abandon using words as numbers? |
Might have gotten a little carried away. I'm ok with removing the word support or limiting it to primary, secondary, third. |
@tbfleming I agree however, there will be some non-zero amount of time between pushing a new version of the ABI format which includes typed indices and when existing ABIs are migrated to it. Even if this gets deprecated in the next release, we will have coverage for our users until then. I do not think this is the last word on the usability of these indices. |
return index; | ||
} | ||
|
||
template <typename IndexType, typename Scope, typename SecKeyType, typename ConvFn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like Scope
is no longer really needed as a template parameter and a relic of the days when we had multiple intrusive indices. All of the callers use by_secondary
and it is only used in a way where by_secondary
is always the right choice. We should probably simplify it out.
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") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only called when we are explicitly not talking about the primary key. However, the naming of the method and the operation would imply that there is no difference between the primary and secondary indices. (They both return 0 for the index byte).
Can we rename this such that it is obvious that this only works on the non-primary indices and let "first", "primary" etc fall through to the throw
when it is misused?
…move unneeded Scope template params.
not support search by string? |
you can use |
this is my struct , if i want search by name or phone,how to get it struct chaninfo {
|
We don't support std::string as key of any indexes. However you can find a way to encode it into 64/128/256 bit integers. |
Does the abi have to be updated as well in the |
#3695
New parameter:
Command example:
Get all the bids sorted by highbid:
Get all the producers which have votes equal to zero: