refactor: query filters#4833
Conversation
6d4f55f to
bfb2f31
Compare
|
Are projections smt we can introduce later? |
Yeah, adding projections in the future is possible. Would also shave off a few specialized queries we have. Although I haven't really been thinking about possible designs. |
Degradation in PK quires might introduce overhead into permission checks in the executor, so we should be careful with it. |
|
Can you describe briefly describe idea behind new predicates? Like i can see now that we have What are stages predicate go though? |
I've pushed some docs, including explanations of this topic. You would probably want to take a look at |
|
I think that we should get rid of singular queries. We should have queries that return multiple results but can be filtered to return just one. Would you agree? Edit: I see that you mention this can be done in a separate PR. I agree |
I've already removed all singular queries that can be implemented as a query + filter. Some of them can be removed (by introducing projections into the query system), but others, like |
3b0f875 to
5047a82
Compare
cd45578 to
4656ecc
Compare
- remove `cant_filter_singular_query`: this is now much clearer, as the singular and iterable queries have different entrypoints, no need to test this anymore. - update `transparent_api_private_item`: use a different private item Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…w dsl, integrate into the client Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
This also fixes how an instruction failing to find a domain or an asset definition gets reported by the executor (by returning a FindError instead of a string) Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…ndXXXMetadata` Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…aw_filter` -> `filter` Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…p some others Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
- `PredicateTrait` -> `EvaluatePredicate` - `IterableQueryIterator` -> `QueryIterator` - `IterableQueryBuilder` -> `QueryBuilder` Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…the queries This renames prometheus metrics and `FindAllParameters` variant of `SingularQueryBox` enum Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…ension trait Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…onna be the "default" Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
- `ClientQueryCursor` and `SmartContractQueryCursor` -> `QueryCursor` - `ClientQueryError` -> `QueryError` - `QueryRequest::StartIterable` -> `QueryRequest::Start` - `QueryRequest::ContinueIterable` -> `QueryRequest::Continue` - `Pagination`'s `start` field -> `offset` Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…tQueryRequestHead::assemble` private Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…g error in executor Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
…from public API Signed-off-by: ⭐️NINIKA⭐️ <dcnick3@users.noreply.github.com>
7a86f22 to
6ff772d
Compare
- target newer Iroha version, with a new type-safe query system (hyperledger-iroha/iroha#4833) - restructure modules - do not format autogenerated code Signed-off-by: 0x009922 <43530070+0x009922@users.noreply.github.com>
Description
This PR introduces a lot of changes to the query subsystem. I tried to make the changes gradually in smaller PRs, but it ended up being very intertwined =(
Summary of changes
PredicateBoxthere is a predicate type for each queryable type (likeAccountPredicateBox), withCompountPredicate<T>providing the logical operations on top.Iterator::filterclosure (|obj| obj.field.inner_field.starts_with("hello")) and is inspired by diesel's DSL.QueryExecutortraitThe primary key question
Currently we have a bunch of queries that give you an object using its Primary Key (like
FindAccountById). They can be implemented by filtering a basis query that returns all objects of such type (likeFindAllAccounts). However, with naïve implementation (which this PR currently does) this would result inO(n)complexity instead ofO(log n)that we currently have.There is a way to do these lookups fast: just pattern-match the filter on the shape
|obj| obj.id.eq(expected_id)and use the primary key index instead of linear scan. We can even use a similar approach to speed up other types of queries, if deemed necessary.However, merging it as-is would mean that the primary queries would be slow. I can probably add the PK queries back for now, if that's unacceptable.
Future work
These are improvements I see that can be made to this PR, but I would prefer them to be implemented in separate PRs. I'll convent them to issues when this PR gets closer to getting merged.
Perf:
Ergonomics:
client::account::all()et al, as this API doesn't provide much value compared toFindAccounts::new()and doesn't exist in smart contracts (revise query API on client #4892)client::domain::by_id, with them still relying on filtered queriesimpl Into<String>for ids (Make methods accepting ids by value generic over its referencedness #4832)Misc:
Displayrepresentations for queries & filters (right now printing them withDebug)Linked issue
Closes #4569, #4720, #3720, #3671.
Checklist
I still have a lot of doc strings, an overview of the predicate DSL implementation and some tests to write, hence marking this PR as a draft.
#[warn(missing_docs)]SignedQueryCandidate