Skip to content

Commit

Permalink
Merge 'counters: Fix filtering of counters' from Juliusz
Browse files Browse the repository at this point in the history 
Queries with `ALLOW FILTERING` and constraints on counter
values used to be rejected as "unimplemented". The reason
was a missing tri-comparator, which is added in this patch.

Fixes #5635

* jul-stas-5635-filtering-on-counters:
  cql/tests: Added test for filtering on counter columns
  counters: add comparator and remove `unimplemented` from restrictions
  • Loading branch information
@psarna
psarna committed Apr 27, 2020
2 parents 1f90230 + afee590 commit c32faee
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 20 deletions.
18 changes: 0 additions & 18 deletions cql3/restrictions/statement_restrictions.cc
View file
Expand Up @@ -624,9 +624,6 @@ bool single_column_restriction::EQ::is_satisfied_by(const schema& schema,
const row& cells,
const query_options& options,
gc_clock::time_point now) const {
if (_column_def.type->is_counter()) {
fail(unimplemented::cause::COUNTERS);
}
auto operand = value(options);
if (operand) {
auto cell_value = get_value(schema, key, ckey, cells, now);
Expand All @@ -641,9 +638,6 @@ bool single_column_restriction::EQ::is_satisfied_by(const schema& schema,
}

bool single_column_restriction::EQ::is_satisfied_by(bytes_view data, const query_options& options) const {
if (_column_def.type->is_counter()) {
fail(unimplemented::cause::COUNTERS);
}
auto operand = value(options);
if (!operand) {
throw exceptions::invalid_request_exception(format("Invalid null value for {}", _column_def.name_as_text()));
Expand All @@ -657,9 +651,6 @@ bool single_column_restriction::IN::is_satisfied_by(const schema& schema,
const row& cells,
const query_options& options,
gc_clock::time_point now) const {
if (_column_def.type->is_counter()) {
fail(unimplemented::cause::COUNTERS);
}
auto cell_value = get_value(schema, key, ckey, cells, now);
if (!cell_value) {
return false;
Expand All @@ -673,9 +664,6 @@ bool single_column_restriction::IN::is_satisfied_by(const schema& schema,
}

bool single_column_restriction::IN::is_satisfied_by(bytes_view data, const query_options& options) const {
if (_column_def.type->is_counter()) {
fail(unimplemented::cause::COUNTERS);
}
auto operands = values(options);
return boost::algorithm::any_of(operands, [this, &data] (const bytes_opt& operand) {
return operand && _column_def.type->compare(*operand, data) == 0;
Expand Down Expand Up @@ -720,9 +708,6 @@ bool single_column_restriction::slice::is_satisfied_by(const schema& schema,
}

bool single_column_restriction::slice::is_satisfied_by(bytes_view data, const query_options& options) const {
if (_column_def.type->is_counter()) {
fail(unimplemented::cause::COUNTERS);
}
return to_range(_slice, options, _column_def.name_as_text()).contains(
data, _column_def.type->underlying_type()->as_tri_comparator());
}
Expand All @@ -733,9 +718,6 @@ bool single_column_restriction::contains::is_satisfied_by(const schema& schema,
const row& cells,
const query_options& options,
gc_clock::time_point now) const {
if (_column_def.type->is_counter()) {
fail(unimplemented::cause::COUNTERS);
}
if (!_column_def.type->is_collection()) {
return false;
}
Expand Down
29 changes: 29 additions & 0 deletions test/cql/counters_test.cql
View file
@@ -0,0 +1,29 @@
CREATE TABLE ks.tbl_cnt (pk int PRIMARY KEY, c1 counter, c2 counter);

-- insert some values in one column
UPDATE ks.tbl_cnt SET c1 = c1+1 WHERE pk = 1;
UPDATE ks.tbl_cnt SET c1 = c1+2 WHERE pk = 2;
UPDATE ks.tbl_cnt SET c1 = c1+3 WHERE pk = 3;
UPDATE ks.tbl_cnt SET c1 = c1+4 WHERE pk = 4;

-- test various filtering options on counter column
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 < 3 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 < 1 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 <= 3 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 > 2 AND pk = 4 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 >= 3 and pk = 3 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 > 4 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 in (-1, 2, 3) ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 = 0 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 = 1 ALLOW FILTERING;

-- now filter through untouched counters `c2` - they should appear as NULLs and evaluate as zeros
SELECT pk, c1, c2 FROM ks.tbl_cnt WHERE c2 = 0 ALLOW FILTERING;
SELECT pk, c2 FROM ks.tbl_cnt WHERE c2 < 0 ALLOW FILTERING;
SELECT pk, c2 FROM ks.tbl_cnt WHERE c2 > 0 ALLOW FILTERING;

-- delete `c1` and make sure it doesn't appear in filtering results
DELETE c1 from ks.tbl_cnt WHERE pk = 1;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 = 1 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 <= 1000 ALLOW FILTERING;
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 > -1000 ALLOW FILTERING;
190 changes: 190 additions & 0 deletions test/cql/counters_test.result
View file
@@ -0,0 +1,190 @@
CREATE TABLE ks.tbl_cnt (pk int PRIMARY KEY, c1 counter, c2 counter);
{
"status" : "ok"
}

-- insert some values in one column
UPDATE ks.tbl_cnt SET c1 = c1+1 WHERE pk = 1;
{
"status" : "ok"
}
UPDATE ks.tbl_cnt SET c1 = c1+2 WHERE pk = 2;
{
"status" : "ok"
}
UPDATE ks.tbl_cnt SET c1 = c1+3 WHERE pk = 3;
{
"status" : "ok"
}
UPDATE ks.tbl_cnt SET c1 = c1+4 WHERE pk = 4;
{
"status" : "ok"
}

-- test various filtering options on counter column
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 < 3 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "1",
"pk" : "1"
},
{
"c1" : "2",
"pk" : "2"
}
]
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 < 1 ALLOW FILTERING;
{
"rows" : null
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 <= 3 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "1",
"pk" : "1"
},
{
"c1" : "2",
"pk" : "2"
},
{
"c1" : "3",
"pk" : "3"
}
]
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 > 2 AND pk = 4 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "4",
"pk" : "4"
}
]
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 >= 3 and pk = 3 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "3",
"pk" : "3"
}
]
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 > 4 ALLOW FILTERING;
{
"rows" : null
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 in (-1, 2, 3) ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "2",
"pk" : "2"
},
{
"c1" : "3",
"pk" : "3"
}
]
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 = 0 ALLOW FILTERING;
{
"rows" : null
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 = 1 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "1",
"pk" : "1"
}
]
}

-- now filter through untouched counters `c2` - they should appear as NULLs and evaluate as zeros
SELECT pk, c1, c2 FROM ks.tbl_cnt WHERE c2 = 0 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "1",
"pk" : "1"
},
{
"c1" : "2",
"pk" : "2"
},
{
"c1" : "4",
"pk" : "4"
},
{
"c1" : "3",
"pk" : "3"
}
]
}
SELECT pk, c2 FROM ks.tbl_cnt WHERE c2 < 0 ALLOW FILTERING;
{
"rows" : null
}
SELECT pk, c2 FROM ks.tbl_cnt WHERE c2 > 0 ALLOW FILTERING;
{
"rows" : null
}

-- delete `c1` and make sure it doesn't appear in filtering results
DELETE c1 from ks.tbl_cnt WHERE pk = 1;
{
"status" : "ok"
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 = 1 ALLOW FILTERING;
{
"rows" : null
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 <= 1000 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "2",
"pk" : "2"
},
{
"c1" : "4",
"pk" : "4"
},
{
"c1" : "3",
"pk" : "3"
}
]
}
SELECT pk, c1 FROM ks.tbl_cnt WHERE c1 > -1000 ALLOW FILTERING;
{
"rows" :
[
{
"c1" : "2",
"pk" : "2"
},
{
"c1" : "4",
"pk" : "4"
},
{
"c1" : "3",
"pk" : "3"
}
]
}
6 changes: 4 additions & 2 deletions types.cc
View file
Expand Up @@ -1985,8 +1985,10 @@ struct compare_visitor {
int32_t operator()(const empty_type_impl&) { return 0; }
int32_t operator()(const tuple_type_impl& t) { return compare_aux(t, v1, v2); }
int32_t operator()(const counter_type_impl&) {
fail(unimplemented::cause::COUNTERS);
return 0;
// untouched (empty) counter evaluates as 0
const auto a = v1.empty() ? 0 : simple_type_traits<int64_t>::read_nonempty(v1);
const auto b = v2.empty() ? 0 : simple_type_traits<int64_t>::read_nonempty(v2);
return a == b ? 0 : a < b ? -1 : 1;
}
int32_t operator()(const decimal_type_impl& d) {
if (v1.empty()) {
Expand Down

0 comments on commit c32faee

Please sign in to comment.