Transform mutation data into result_row_view when invoking expr::is_satisfied_by() #7215
Assignees
Labels
area/internals
an issue which refers to some internal class or something which has little exposure to users and is
feature/enhancement
Milestone
Comments
slivne
added
area/internals
nyh
added a commit
that referenced
this issue
Jul 11, 2021
This series unifies the interface for checking if CQL restrictions are satisfied. Previously, an additional mutation-based approach was added in the materialized views layer, but the decision was reached that it's better to have a single API based on partition slices. With that, the regular selection path gets simplified at the cost of more complicated view generation path, which is a good tradeoff. Note that in order to unify the interface, the view layer performs ugly transformations in order to adjust the input for `is_satisfied_by`. Reviewers, please take a close look at this code (`matches_view_filter`, `clustering_prefix_matches`, `partition_key_matches`), because it looks error-prone and relies on dirty internals of our serialization layer. If somebody has a better suggestion on how to do the transformation, I'm all ears. Tests: unit(release), manual(playing with materialized views with custom filters) Fixes #7215 Closes #8979 * github.com:scylladb/scylla: db,view,table: drop unneeded time point parameter cql3,expr: unify get_value cql3,expr: purge mutation-based is_satisfied_by db,view: migrate key checks from the deprecated is_satisfied_by db,view: migrate checking view filter to new is_satisfied_by multishard_mutation_query: make the data builder public db,view: move make_partition_slice helper function up
nyh
added a commit
that referenced
this issue
Jul 12, 2021
This series unifies the interface for checking if CQL restrictions are satisfied. Previously, an additional mutation-based approach was added in the materialized views layer, but the decision was reached that it's better to have a single API based on partition slices. With that, the regular selection path gets simplified at the cost of more complicated view generation path, which is a good tradeoff. Note that in order to unify the interface, the view layer performs ugly transformations in order to adjust the input for `is_satisfied_by`. Reviewers, please take a close look at this code (`matches_view_filter`, `clustering_prefix_matches`, `partition_key_matches`), because it looks error-prone and relies on dirty internals of our serialization layer. If somebody has a better suggestion on how to do the transformation, I'm all ears. Tests: unit(release), manual(playing with materialized views with custom filters) Fixes #7215 Closes #8979 * github.com:scylladb/scylla: db,view,table: drop unneeded time point parameter cql3,expr: unify get_value cql3,expr: purge mutation-based is_satisfied_by db,view: migrate key checks from the deprecated is_satisfied_by db,view: migrate checking view filter to new is_satisfied_by db,view: add a helper result builder class db,view: move make_partition_slice helper function up
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are currently two versions of
is_satisfied_by(), one for partition slice and one for mutation:https://github.com/scylladb/scylla/blob/df3ea2443bd398be714bb51114af184278582933/cql3/expr/expression.hh#L94-L105
As requested in this comment, we should drop the mutation version, and those callers should transform the mutation data into result_row_view.
The text was updated successfully, but these errors were encountered: