diff --git a/CMakeLists.txt b/CMakeLists.txt index 312dd620e6..896e83c4bf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -349,7 +349,7 @@ set(scylla_sources cql3/cql3_type.cc cql3/expr/expression.cc cql3/expr/prepare_expr.cc - cql3/expr/to_restriction.cc + cql3/expr/restrictions.cc cql3/functions/aggregate_fcts.cc cql3/functions/castas_fcts.cc cql3/functions/error_injection_fcts.cc diff --git a/configure.py b/configure.py index da90966366..ab6b7c4804 100755 --- a/configure.py +++ b/configure.py @@ -761,7 +761,7 @@ scylla_core = (['replica/database.cc', 'cql3/maps.cc', 'cql3/values.cc', 'cql3/expr/expression.cc', - 'cql3/expr/to_restriction.cc', + 'cql3/expr/restrictions.cc', 'cql3/expr/prepare_expr.cc', 'cql3/functions/user_function.cc', 'cql3/functions/functions.cc', diff --git a/cql3/expr/expression.hh b/cql3/expr/expression.hh index fe200b3b7a..86debb8ea4 100644 --- a/cql3/expr/expression.hh +++ b/cql3/expr/expression.hh @@ -717,14 +717,6 @@ void fill_prepare_context(expression&, cql3::prepare_context&); // For example an expression can contain calls to nonpure functions. bool contains_bind_marker(const expression& e); -// Converts the given expression to the corresponding restriction instance. -// The expression does't have to be prepared, it will be prepared in this function. -// Needed for now, but in the future all restrictions will be replaced by expression. -::shared_ptr to_restriction(const expression&, - data_dictionary::database, - schema_ptr, - prepare_context&); - // Checks whether this expression contains restrictions on one single column. // There might be more than one restriction, but exactly one column. // The expression must be prepared. diff --git a/cql3/expr/to_restriction.cc b/cql3/expr/restrictions.cc similarity index 75% rename from cql3/expr/to_restriction.cc rename to cql3/expr/restrictions.cc index 367f780080..2f6f050e92 100644 --- a/cql3/expr/to_restriction.cc +++ b/cql3/expr/restrictions.cc @@ -6,15 +6,16 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -#include "cql3/column_specification.hh" -#include "expression.hh" +#include "restrictions.hh" +#include "cql3/statements/request_validations.hh" +#include "seastar/util/defer.hh" +#include "cql3/prepare_context.hh" #include "cql3/restrictions/multi_column_restriction.hh" #include "cql3/restrictions/token_restriction.hh" -#include "cql3/prepare_context.hh" -#include "schema.hh" namespace cql3 { namespace expr { + extern logging::logger expr_logger; namespace { @@ -87,31 +88,6 @@ void validate_single_column_relation(const column_value& lhs, oper_t oper, const } } -::shared_ptr new_single_column_restriction(column_value&& lhs_col, - oper_t oper, - expression&& prepared_rhs, - comparison_order order, - const schema& schema) { - validate_single_column_relation(lhs_col, oper, schema, false); - - ::shared_ptr r = ::make_shared(); - r->expression = binary_operator(std::move(lhs_col), oper, std::move(prepared_rhs), order); - return r; -} - -::shared_ptr new_subscripted_column_restriction(subscript&& lhs_sub, - oper_t oper, - expression&& prepared_rhs, - comparison_order order, - const schema& schema) { - const column_value& sub_col = get_subscripted_column(lhs_sub); - validate_single_column_relation(sub_col, oper, schema, true); - - ::shared_ptr r = ::make_shared(); - r->expression = binary_operator(std::move(lhs_sub), oper, std::move(prepared_rhs)); - return r; -} - std::vector to_column_definitions(const std::vector& cols) { std::vector result; result.reserve(cols.size()); @@ -145,20 +121,137 @@ void validate_multi_column_relation(const std::vector& } } +void validate_token_relation(const std::vector column_defs, oper_t oper, const schema& schema) { + auto pk = schema.partition_key_columns(); + if (!std::equal(column_defs.begin(), column_defs.end(), pk.begin(), + pk.end(), [](auto* c1, auto& c2) { + return c1 == &c2; // same, not "equal". + })) { + + throw exceptions::invalid_request_exception( + format("The token function arguments must be in the partition key order: {}", + std::to_string(column_defs))); + } +} + +void preliminary_binop_vaidation_checks(const binary_operator& binop) { + if (auto lhs_tup = as_if(&binop.lhs)) { + if (binop.op == oper_t::CONTAINS) { + throw exceptions::invalid_request_exception("CONTAINS cannot be used for Multi-column relations"); + } + + if (binop.op == oper_t::CONTAINS_KEY) { + throw exceptions::invalid_request_exception("CONTAINS_KEY cannot be used for Multi-column relations"); + } + + if (binop.op == oper_t::LIKE) { + throw exceptions::invalid_request_exception("LIKE cannot be used for Multi-column relations"); + } + + if (auto rhs_tup = as_if(&binop.rhs)) { + if (lhs_tup->elements.size() != rhs_tup->elements.size()) { + throw exceptions::invalid_request_exception( + format("Expected {} elements in value tuple, but got {}: {}", + lhs_tup->elements.size(), rhs_tup->elements.size(), *rhs_tup)); + } + } + } + + if (is(binop.lhs)) { + if (binop.op == oper_t::IN) { + throw exceptions::invalid_request_exception("IN cannot be used with the token function"); + } + + if (binop.op == oper_t::LIKE) { + throw exceptions::invalid_request_exception("LIKE cannot be used with the token function"); + } + + if (binop.op == oper_t::CONTAINS) { + throw exceptions::invalid_request_exception("CONTAINS cannot be used with the token function"); + } + + if (binop.op == oper_t::CONTAINS_KEY) { + throw exceptions::invalid_request_exception("CONTAINS_KEY cannot be used with the token function"); + } + } +} +} // anonymous namespace + +binary_operator validate_and_prepare_new_restriction(const binary_operator& restriction, + data_dictionary::database db, + schema_ptr schema, + prepare_context& ctx) { + // Perform basic initial checks + preliminary_binop_vaidation_checks(restriction); + + // Prepare the restriction + binary_operator prepared_binop = prepare_binary_operator(restriction, db, schema); + + // Fill prepare context + const column_value* lhs_pk_col_search_res = find_in_expression(prepared_binop.lhs, + [](const column_value& col) { + return col.col->is_partition_key(); + } + ); + auto reset_processing_pk_column = defer([&ctx] () noexcept { ctx.set_processing_pk_restrictions(false); }); + if (lhs_pk_col_search_res != nullptr) { + ctx.set_processing_pk_restrictions(true); + } + fill_prepare_context(prepared_binop.lhs, ctx); + fill_prepare_context(prepared_binop.rhs, ctx); + + // Check for disallowed operators + if (prepared_binop.op == oper_t::NEQ) { + throw exceptions::invalid_request_exception(format("Unsupported \"!=\" relation: {}", prepared_binop)); + } + + if (prepared_binop.op == oper_t::IS_NOT) { + bool rhs_is_null = is(prepared_binop.rhs) && as(prepared_binop.rhs).is_null(); + if (!rhs_is_null) { + throw exceptions::invalid_request_exception(format("Unsupported \"IS NOT\" relation: {}", prepared_binop)); + } + } + + // Perform more throughout validation depending on restriction type + if (auto col_val = as_if(&prepared_binop.lhs)) { + // Simple single column restriction + validate_single_column_relation(*col_val, prepared_binop.op, *schema, false); + } else if (auto sub = as_if(&prepared_binop.lhs)) { + // Subscripted single column restriction + const column_value& sub_col = get_subscripted_column(*sub); + validate_single_column_relation(sub_col, prepared_binop.op, *schema, true); + } else if (auto multi_col_tuple = as_if(&prepared_binop.lhs)) { + // Multi column restriction + std::vector lhs_cols = to_column_definitions(multi_col_tuple->elements); + std::vector> lhs_col_specs; + lhs_col_specs.reserve(multi_col_tuple->elements.size()); + + for (const column_definition* col_def : lhs_cols) { + lhs_col_specs.push_back(col_def->column_specification); + } + + validate_multi_column_relation(lhs_cols, prepared_binop.op); + } else if (auto lhs_token = as_if(&prepared_binop.lhs)) { + // Token restriction + std::vector column_defs = to_column_definitions(lhs_token->args); + validate_token_relation(column_defs, prepared_binop.op, *schema); + } else { + // Anything else + throw exceptions::invalid_request_exception( + format("expr::validate_and_prepare_new_restriction unhandled restriction: {}", prepared_binop)); + } + + return prepared_binop; +} + +namespace { + ::shared_ptr new_multi_column_restriction(const tuple_constructor& prepared_lhs_tuple, oper_t oper, - expression&& prepared_rhs, + expression prepared_rhs, comparison_order order, const schema_ptr& schema) { std::vector lhs_cols = to_column_definitions(prepared_lhs_tuple.elements); - std::vector> lhs_col_specs; - lhs_col_specs.reserve(prepared_lhs_tuple.elements.size()); - - for (const column_definition* col_def : lhs_cols) { - lhs_col_specs.push_back(col_def->column_specification); - } - - validate_multi_column_relation(lhs_cols, oper); if (oper == oper_t::EQ) { return ::make_shared(schema, @@ -238,135 +331,24 @@ void validate_multi_column_relation(const std::vector& fmt::format("new_multi_column_restriction operation type: {} not handled", oper)); } -void validate_token_relation(const std::vector column_defs, oper_t oper, const schema& schema) { - auto pk = schema.partition_key_columns(); - if (!std::equal(column_defs.begin(), column_defs.end(), pk.begin(), - pk.end(), [](auto* c1, auto& c2) { - return c1 == &c2; // same, not "equal". - })) { - - throw exceptions::invalid_request_exception( - format("The token function arguments must be in the partition key order: {}", - std::to_string(column_defs))); - } -} - -::shared_ptr new_token_restriction(token&& prepared_lhs_token, - oper_t oper, - expression&& prepared_rhs, - comparison_order order, - const schema& schema) { - std::vector column_defs = to_column_definitions(prepared_lhs_token.args); - validate_token_relation(column_defs, oper, schema); - - ::shared_ptr r = ::make_shared(column_defs); - r->expression = binary_operator(std::move(prepared_lhs_token), oper, std::move(prepared_rhs)); - - return r; -} - -void preliminary_binop_vaidation_checks(const binary_operator& binop) { - if (auto lhs_tup = as_if(&binop.lhs)) { - if (binop.op == oper_t::CONTAINS) { - throw exceptions::invalid_request_exception("CONTAINS cannot be used for Multi-column relations"); - } - - if (binop.op == oper_t::CONTAINS_KEY) { - throw exceptions::invalid_request_exception("CONTAINS_KEY cannot be used for Multi-column relations"); - } - - if (binop.op == oper_t::LIKE) { - throw exceptions::invalid_request_exception("LIKE cannot be used for Multi-column relations"); - } - - if (auto rhs_tup = as_if(&binop.rhs)) { - if (lhs_tup->elements.size() != rhs_tup->elements.size()) { - throw exceptions::invalid_request_exception( - format("Expected {} elements in value tuple, but got {}: {}", - lhs_tup->elements.size(), rhs_tup->elements.size(), *rhs_tup)); - } - } - } - - if (is(binop.lhs)) { - if (binop.op == oper_t::IN) { - throw exceptions::invalid_request_exception("IN cannot be used with the token function"); - } - - if (binop.op == oper_t::LIKE) { - throw exceptions::invalid_request_exception("LIKE cannot be used with the token function"); - } - - if (binop.op == oper_t::CONTAINS) { - throw exceptions::invalid_request_exception("CONTAINS cannot be used with the token function"); - } - - if (binop.op == oper_t::CONTAINS_KEY) { - throw exceptions::invalid_request_exception("CONTAINS_KEY cannot be used with the token function"); - } - } -} - } // anonymous namespace -::shared_ptr to_restriction(const expression& e, - data_dictionary::database db, - schema_ptr schema, - prepare_context& ctx) { - if (is(e)) { - throw exceptions::invalid_request_exception("Constant value by itself is not supported as a restriction yet"); +::shared_ptr convert_to_restriction(const binary_operator& prepared_binop, const schema_ptr& schema) { + if (is(prepared_binop.lhs) || is(prepared_binop.lhs)) { + ::shared_ptr r = ::make_shared(); + r->expression = prepared_binop; + return r; + } else if (auto lhs_tuple = as_if(&prepared_binop.lhs)) { + return new_multi_column_restriction(*lhs_tuple, prepared_binop.op, prepared_binop.rhs, prepared_binop.order, schema); + } else if (auto lhs_token = as_if(&prepared_binop.lhs)) { + std::vector column_defs = to_column_definitions(lhs_token->args); + ::shared_ptr r = ::make_shared(std::move(column_defs)); + r->expression = prepared_binop; + return r; + } else { + throw exceptions::invalid_request_exception( + format("expr::validate_and_prepare_new_restriction unhandled restriction: {}", prepared_binop)); } - - if (!is(e)) { - throw exceptions::invalid_request_exception("Restriction must be a binary operator"); - } - const binary_operator& binop_to_prepare = as(e); - - preliminary_binop_vaidation_checks(binop_to_prepare); - - binary_operator prepared_binop = prepare_binary_operator(binop_to_prepare, db, schema); - - const column_value* lhs_pk_col_search_res = find_in_expression(prepared_binop.lhs, - [](const column_value& col) { - return col.col->is_partition_key(); - } - ); - auto reset_processing_pk_column = defer([&ctx] () noexcept { ctx.set_processing_pk_restrictions(false); }); - if (lhs_pk_col_search_res != nullptr) { - ctx.set_processing_pk_restrictions(true); - } - fill_prepare_context(prepared_binop.lhs, ctx); - fill_prepare_context(prepared_binop.rhs, ctx); - - if (prepared_binop.op == oper_t::NEQ) { - throw exceptions::invalid_request_exception(format("Unsupported \"!=\" relation: {}", prepared_binop)); - } - - if (prepared_binop.op == oper_t::IS_NOT) { - bool rhs_is_null = is(prepared_binop.rhs) && as(prepared_binop.rhs).is_null(); - if (!rhs_is_null) { - throw exceptions::invalid_request_exception(format("Unsupported \"IS NOT\" relation: {}", prepared_binop)); - } - } - - if (auto col_val = as_if(&prepared_binop.lhs)) { - return new_single_column_restriction(std::move(*col_val), prepared_binop.op, std::move(prepared_binop.rhs), prepared_binop.order, *schema); - } - - if (auto sub = as_if(&prepared_binop.lhs)) { - return new_subscripted_column_restriction(std::move(*sub), prepared_binop.op, std::move(prepared_binop.rhs), prepared_binop.order, *schema); - } - - if (auto multi_col_tuple = as_if(&prepared_binop.lhs)) { - return new_multi_column_restriction(*multi_col_tuple, prepared_binop.op, std::move(prepared_binop.rhs), prepared_binop.order, schema); - } - - if (auto lhs_token = as_if(&prepared_binop.lhs)) { - return new_token_restriction(std::move(*lhs_token), prepared_binop.op, std::move(prepared_binop.rhs), prepared_binop.order, *schema); - } - - on_internal_error(expr_logger, format("expr::to_restriction unhandled case: {}", e)); } - } // namespace expr } // namespace cql3 \ No newline at end of file diff --git a/cql3/expr/restrictions.hh b/cql3/expr/restrictions.hh new file mode 100644 index 0000000000..7c3336b300 --- /dev/null +++ b/cql3/expr/restrictions.hh @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2022-present ScyllaDB + */ + +/* + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +#include "expression.hh" + +namespace cql3 { +namespace expr { + +// Given a restriction from the WHERE clause prepares it and performs some validation checks. +// It will also fill the prepare context automatically, there's no need to do that later. +binary_operator validate_and_prepare_new_restriction(const binary_operator& restriction, + data_dictionary::database db, + schema_ptr schema, + prepare_context& ctx); + + +// Converts a prepared binary operator to an instance of the restriction class. +// Doesn't perform any any validation checks. +::shared_ptr convert_to_restriction(const binary_operator& prepared_binop, + const schema_ptr& schema); + +} // namespace expr +} // namespace cql3 diff --git a/cql3/restrictions/statement_restrictions.cc b/cql3/restrictions/statement_restrictions.cc index ba007db38f..b3d328d2c5 100644 --- a/cql3/restrictions/statement_restrictions.cc +++ b/cql3/restrictions/statement_restrictions.cc @@ -31,6 +31,7 @@ #include "types/list.hh" #include "types/map.hh" #include "types/set.hh" +#include "cql3/expr/restrictions.hh" namespace cql3 { namespace restrictions { @@ -438,7 +439,10 @@ statement_restrictions::statement_restrictions(data_dictionary::database db, throw exceptions::invalid_request_exception(format("restriction '{}' is only supported in materialized view creation", relation)); } } else { - const auto restriction = expr::to_restriction(relation_expr, db, schema, ctx); + expr::binary_operator prepared_restriction = expr::validate_and_prepare_new_restriction(*relation_binop, db, schema, ctx); + add_restriction(prepared_restriction, schema, allow_filtering, for_view); + + const auto restriction = expr::convert_to_restriction(prepared_restriction, schema); if (dynamic_pointer_cast(restriction)) { _clustering_columns_restrictions = _clustering_columns_restrictions->merge_to(_schema, restriction); } else if (has_token(restriction->expression)) { @@ -454,9 +458,6 @@ statement_restrictions::statement_restrictions(data_dictionary::database db, } _where = _where.has_value() ? make_conjunction(std::move(*_where), restriction->expression) : restriction->expression; } - - expr::binary_operator prepared_restriction = expr::prepare_binary_operator(*relation_binop, db, schema); - add_restriction(prepared_restriction, schema, allow_filtering, for_view); } } if (_where.has_value()) {