Skip to content

Commit

Permalink
cql3: ensure repeated values in IN clauses don't return repeated rows
Browse files Browse the repository at this point in the history
When the list of values in the IN list of a single column contains
duplicates, multiple executors are activated since the assumption
is that each value in the IN list corresponds to a different partition.
this results in the same row appearing in the result number times
corresponding to the duplication of the partition value.

Added queries for the in restriction unitest and fixed with a bad result check.

Fixes #2837
Tests: Queries as in the usecase from the GitHub issue in both forms ,
prepared and plain (using python driver),Unitest.

Signed-off-by: Eliran Sinvani <eliransin@scylladb.com>
Message-Id: <ad88b7218fa55466be7bc4303dc50326a3d59733.1534322238.git.eliransin@scylladb.com>
(cherry picked from commit d734d31)
  • Loading branch information
Eliran Sinvani authored and avikivity committed Aug 26, 2018
1 parent f475c65 commit e183e71
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 4 deletions.
12 changes: 10 additions & 2 deletions cql3/restrictions/single_column_restriction.hh
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,14 @@ public:
const query_options& options,
gc_clock::time_point now) const override;

virtual std::vector<bytes_opt> values_raw(const query_options& options) const = 0;

virtual std::vector<bytes_opt> values(const query_options& options) const override {
std::vector<bytes_opt> ret = values_raw(options);
std::sort(ret.begin(),ret.end());
ret.erase(std::unique(ret.begin(),ret.end()),ret.end());
return ret;
}
#if 0
@Override
protected final boolean isSupportedBy(SecondaryIndex index)
Expand All @@ -224,7 +232,7 @@ public:
return abstract_restriction::term_uses_function(_values, ks_name, function_name);
}

virtual std::vector<bytes_opt> values(const query_options& options) const override {
virtual std::vector<bytes_opt> values_raw(const query_options& options) const override {
std::vector<bytes_opt> ret;
for (auto&& v : _values) {
ret.emplace_back(to_bytes_opt(v->bind_and_get(options)));
Expand All @@ -249,7 +257,7 @@ public:
return false;
}

virtual std::vector<bytes_opt> values(const query_options& options) const override {
virtual std::vector<bytes_opt> values_raw(const query_options& options) const override {
auto&& lval = dynamic_pointer_cast<multi_item_terminal>(_marker->bind(options));
if (!lval) {
throw exceptions::invalid_request_exception("Invalid null value for IN restriction");
Expand Down
39 changes: 37 additions & 2 deletions tests/cql_query_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2076,10 +2076,9 @@ SEASTAR_TEST_CASE(test_in_restriction) {
assert_that(msg).is_rows().with_size(0);
return e.execute_cql("select r1 from tir where p1 in (2, 0, 2, 1);");
}).then([&e] (shared_ptr<cql_transport::messages::result_message> msg) {
assert_that(msg).is_rows().with_rows({
assert_that(msg).is_rows().with_rows_ignore_order({
{int32_type->decompose(4)},
{int32_type->decompose(0)},
{int32_type->decompose(4)},
{int32_type->decompose(1)},
{int32_type->decompose(2)},
{int32_type->decompose(3)},
Expand All @@ -2101,6 +2100,42 @@ SEASTAR_TEST_CASE(test_in_restriction) {
{int32_type->decompose(2)},
{int32_type->decompose(1)},
});
return e.prepare("select r1 from tir where p1 in ?");
}).then([&e] (cql3::prepared_cache_key_type prepared_id){
auto my_list_type = list_type_impl::get_instance(int32_type, true);
std::vector<cql3::raw_value> raw_values;
auto in_values_list = my_list_type->decompose(make_list_value(my_list_type,
list_type_impl::native_type{{int(2), int(0), int(2), int(1)}}));
raw_values.emplace_back(cql3::raw_value::make_value(in_values_list));
return e.execute_prepared(prepared_id,raw_values);
}).then([&e] (shared_ptr<cql_transport::messages::result_message> msg) {
assert_that(msg).is_rows().with_rows_ignore_order({
{int32_type->decompose(4)},
{int32_type->decompose(0)},
{int32_type->decompose(1)},
{int32_type->decompose(2)},
{int32_type->decompose(3)},
});
}).then([&e]{
return e.execute_cql("create table tir2 (p1 int, c1 int, r1 int, PRIMARY KEY (p1, c1,r1));").discard_result();
}).then([&e] {
e.require_table_exists("ks", "tir2");
return e.execute_cql("insert into tir2 (p1, c1, r1) values (0, 0, 0);").discard_result();
}).then([&e] {
return e.execute_cql("insert into tir2 (p1, c1, r1) values (1, 0, 1);").discard_result();
}).then([&e] {
return e.execute_cql("insert into tir2 (p1, c1, r1) values (1, 1, 2);").discard_result();
}).then([&e] {
return e.execute_cql("insert into tir2 (p1, c1, r1) values (1, 2, 3);").discard_result();
}).then([&e] {
return e.execute_cql("insert into tir2 (p1, c1, r1) values (2, 3, 4);").discard_result();
}).then([&e]{
return e.execute_cql("select r1 from tir2 where (c1,r1) in ((0, 1),(1,2),(0,1),(1,2),(3,3));");
}).then([&e] (shared_ptr<cql_transport::messages::result_message> msg) {
assert_that(msg).is_rows().with_rows_ignore_order({
{int32_type->decompose(1)},
{int32_type->decompose(2)},
});
});
});
}
Expand Down

0 comments on commit e183e71

Please sign in to comment.