Skip to content

Commit

Permalink
cql3: expr: return more accurate error message for invalidated token(…
Browse files Browse the repository at this point in the history
…) args

before this change, we just print out the addresses of the elements
in `column_defs`, if the argument passed to `token()` function are
not valid. this is not quite helpful from user's perspective, as
user would be more interested in the values. also, we could have
more accurate error message for different error.

in this change, following how Cassandra 4.1's behavior, three cases
are identified, and corresponding errors are returned respectively:

* duplicated partition keys
* wrong order of partition key
* missing keys

where, if the partition key order is wrong, instead of the keys specified
by user, the correct order is printed in the error message for helping
user to correct the `token()` function.

for better performance, the checks are performed only if the keys
do not match, based on the assumption that the error handling path
is not likely to be executed.

tests are added accordingly. they tested with Canssandra 4.1.1 also.

Fixes scylladb#13468
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
  • Loading branch information
tchaikov committed Apr 11, 2023
1 parent fd817e1 commit 5c4fd96
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 3 deletions.
21 changes: 18 additions & 3 deletions cql3/expr/restrictions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@
*/

#include "cql3/statements/request_validations.hh"
#include "exceptions/exceptions.hh"
#include "schema/schema.hh"
#include "seastar/util/defer.hh"
#include "cql3/prepare_context.hh"
#include "types/list.hh"
#include <iterator>
#include <ranges>

namespace cql3 {
namespace expr {
Expand Down Expand Up @@ -95,10 +99,21 @@ void validate_token_relation(const std::vector<const column_definition*> column_
pk.end(), [](auto* c1, auto& c2) {
return c1 == &c2; // same, not "equal".
})) {

std::vector<const column_definition*> unique_columns;
std::ranges::unique_copy(column_defs, std::back_inserter(unique_columns));
if (unique_columns.size() < column_defs.size()) {
throw exceptions::invalid_request_exception(
"The token() function contains duplicate partition key components");
}
if (unique_columns.size() < pk.size()) {
throw exceptions::invalid_request_exception(
"The token() function must be applied to all partition key components or none of them");
}
throw exceptions::invalid_request_exception(
format("The token function arguments must be in the partition key order: {}",
std::to_string(column_defs)));
fmt::join(std::views::transform(pk, [](const column_definition& cd) {
return cd.name_as_text();
}), ", ")));
}
}

Expand Down Expand Up @@ -236,4 +251,4 @@ binary_operator validate_and_prepare_new_restriction(const binary_operator& rest
}

} // namespace expr
} // namespace cql3
} // namespace cql3
16 changes: 16 additions & 0 deletions test/cql-pytest/test_filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,3 +393,19 @@ def test_filter_and_fetch_size(cql, test_keyspace, use_index, driver_bug_1):
s.fetch_size = 3
results = cql.execute(s)
assert len(results.current_rows) == 3

# token() function should either take all parition key components or none of them,
# if the key(s) are specified, they should be listed in the paritition key order
# Reproduces #13468
def test_filter_token(cql, test_keyspace):
with new_test_table(cql, test_keyspace, 'pk int, ck int, x int, PRIMARY KEY (pk, ck)') as table:
with pytest.raises(InvalidRequest, match='duplicate partition key'):
cql.execute(f'SELECT pk FROM {table} WHERE token(pk, pk) = 0 ALLOW FILTERING')
with new_test_table(cql, test_keyspace, 'pk int, ck int, x int, PRIMARY KEY ((pk, ck))') as table:
with pytest.raises(InvalidRequest, match='all partition key components'):
cql.execute(f'SELECT pk FROM {table} WHERE token(x) = 0 ALLOW FILTERING')
with pytest.raises(InvalidRequest, match='all partition key components'):
cql.execute(f'SELECT pk FROM {table} WHERE token(pk) = 0 ALLOW FILTERING')
with pytest.raises(InvalidRequest, match='partition key order'):
cql.execute(f'SELECT pk FROM {table} WHERE token(ck, pk) = 0 ALLOW FILTERING')
cql.execute(f'SELECT pk FROM {table} WHERE token(ck, pk) = 0 ALLOW FILTERING')

0 comments on commit 5c4fd96

Please sign in to comment.