cql3: expr: return more accurate error message for invalidated token() args
before this change, we just print out the addresses of the elements in `column_defs`, if the arguments passed to `token()` function are not valid. this is not quite helpful from the user's perspective. as user would be more interested in the values. also, we could print more accurate error message for different error. in this change, following 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 printing 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 #13468 Signed-off-by: Kefu Chai <kefu.chai@scylladb.com> Closes #13470
This commit is contained in:
@@ -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 {
|
||||
@@ -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(boost::adaptors::transform(pk, [](const column_definition& cd) {
|
||||
return cd.name_as_text();
|
||||
}), ", ")));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -236,4 +251,4 @@ binary_operator validate_and_prepare_new_restriction(const binary_operator& rest
|
||||
}
|
||||
|
||||
} // namespace expr
|
||||
} // namespace cql3
|
||||
} // namespace cql3
|
||||
|
||||
@@ -393,3 +393,18 @@ 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')
|
||||
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')
|
||||
with pytest.raises(InvalidRequest, match='all partition key components'):
|
||||
cql.execute(f'SELECT pk FROM {table} WHERE token(pk) = 0')
|
||||
with pytest.raises(InvalidRequest, match='partition key order'):
|
||||
cql.execute(f'SELECT pk FROM {table} WHERE token(ck, pk) = 0')
|
||||
|
||||
Reference in New Issue
Block a user