From 82394debe68f450dffd19187a99655f4e433044b Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Thu, 13 Apr 2017 14:19:04 +0300 Subject: [PATCH] cql3/statements: Multiple index targets for CREATE INDEX --- cql3/Cql.g | 5 +- cql3/statements/create_index_statement.cc | 100 +++++++++++++++------- cql3/statements/create_index_statement.hh | 5 +- 3 files changed, 74 insertions(+), 36 deletions(-) diff --git a/cql3/Cql.g b/cql3/Cql.g index 6eefe07d57..d5fd7558c3 100644 --- a/cql3/Cql.g +++ b/cql3/Cql.g @@ -778,12 +778,13 @@ createIndexStatement returns [::shared_ptr expr] auto props = make_shared(); bool if_not_exists = false; auto name = ::make_shared(); + std::vector<::shared_ptr> targets; } : K_CREATE (K_CUSTOM { props->is_custom = true; })? K_INDEX (K_IF K_NOT K_EXISTS { if_not_exists = true; } )? - (idxName[name])? K_ON cf=columnFamilyName '(' id=indexIdent ')' + (idxName[name])? K_ON cf=columnFamilyName '(' (target1=indexIdent { targets.emplace_back(target1); } (',' target2=indexIdent { targets.emplace_back(target2); } )*)? ')' (K_USING cls=STRING_LITERAL { props->custom_class = sstring{$cls.text}; })? (K_WITH properties[props])? - { $expr = ::make_shared(cf, name, id, props, if_not_exists); } + { $expr = ::make_shared(cf, name, targets, props, if_not_exists); } ; indexIdent returns [::shared_ptr id] diff --git a/cql3/statements/create_index_statement.cc b/cql3/statements/create_index_statement.cc index e93a983778..ba203775bf 100644 --- a/cql3/statements/create_index_statement.cc +++ b/cql3/statements/create_index_statement.cc @@ -57,12 +57,12 @@ namespace statements { create_index_statement::create_index_statement(::shared_ptr name, ::shared_ptr index_name, - ::shared_ptr raw_target, + std::vector<::shared_ptr> raw_targets, ::shared_ptr properties, bool if_not_exists) : schema_altering_statement(name) , _index_name(index_name->get_idx()) - , _raw_target(raw_target) + , _raw_targets(raw_targets) , _properties(properties) , _if_not_exists(if_not_exists) { @@ -86,42 +86,59 @@ create_index_statement::validate(distributed& proxy, con throw exceptions::invalid_request_exception("Secondary indexes are not supported on materialized views"); } - auto target = _raw_target->prepare(schema); - auto cd = schema->get_column_definition(target->column->name()); - - if (cd == nullptr) { - throw exceptions::invalid_request_exception(sprint("No column definition found for column %s", *target->column)); + std::vector<::shared_ptr> targets; + for (auto& raw_target : _raw_targets) { + targets.emplace_back(raw_target->prepare(schema)); } - // Origin TODO: we could lift that limitation - if ((schema->is_dense() || !schema->thrift().has_compound_comparator()) && cd->kind != column_kind::regular_column) { - throw exceptions::invalid_request_exception("Secondary indexes are not supported on PRIMARY KEY columns in COMPACT STORAGE tables"); + if (targets.empty() && !_properties->is_custom) { + throw exceptions::invalid_request_exception("Only CUSTOM indexes can be created without specifying a target column"); } - if (cd->kind == column_kind::partition_key && cd->is_on_all_components()) { - throw exceptions::invalid_request_exception( - sprint( - "Cannot create secondary index on partition key column %s", - *target->column)); + if (targets.size() > 1) { + validate_targets_for_multi_column_index(targets); } - bool is_map = dynamic_cast(cd->type.get()) != nullptr - && dynamic_cast(cd->type.get())->is_map(); - bool is_frozen_collection = cd->type->is_collection() && !cd->type->is_multi_cell(); + for (auto& target : targets) { + auto cd = schema->get_column_definition(target->column->name()); - if (is_frozen_collection) { - validate_for_frozen_collection(target); - } else { - validate_not_full_index(target); - validate_is_values_index_if_target_column_not_collection(cd, target); - validate_target_column_is_map_if_index_involves_keys(is_map, target); - } + if (cd == nullptr) { + throw exceptions::invalid_request_exception( + sprint("No column definition found for column %s", *target->column)); + } - if (cd->idx_info.index_type != ::index_type::none) { - if (_if_not_exists) { - return; + // Origin TODO: we could lift that limitation + if ((schema->is_dense() || !schema->thrift().has_compound_comparator()) && + cd->kind != column_kind::regular_column) { + throw exceptions::invalid_request_exception( + "Secondary indexes are not supported on PRIMARY KEY columns in COMPACT STORAGE tables"); + } + + if (cd->kind == column_kind::partition_key && cd->is_on_all_components()) { + throw exceptions::invalid_request_exception( + sprint( + "Cannot create secondary index on partition key column %s", + *target->column)); + } + + bool is_map = dynamic_cast(cd->type.get()) != nullptr + && dynamic_cast(cd->type.get())->is_map(); + bool is_frozen_collection = cd->type->is_collection() && !cd->type->is_multi_cell(); + + if (is_frozen_collection) { + validate_for_frozen_collection(target); } else { - throw exceptions::invalid_request_exception("Index already exists"); + validate_not_full_index(target); + validate_is_values_index_if_target_column_not_collection(cd, target); + validate_target_column_is_map_if_index_involves_keys(is_map, target); + } + + if (cd->idx_info.index_type != ::index_type::none) { + if (_if_not_exists) { + return; + } else { + throw exceptions::invalid_request_exception("Index already exists"); + } } } @@ -170,6 +187,20 @@ void create_index_statement::validate_target_column_is_map_if_index_involves_key } } +void create_index_statement::validate_targets_for_multi_column_index(std::vector<::shared_ptr> targets) const +{ + if (!_properties->is_custom) { + throw exceptions::invalid_request_exception("Only CUSTOM indexes support multiple columns"); + } + std::unordered_set<::shared_ptr> columns; + for (auto& target : targets) { + if (columns.count(target->column) > 0) { + throw exceptions::invalid_request_exception(sprint("Duplicate column %s in index target list", target->column->name())); + } + columns.emplace(target->column); + } +} + future create_index_statement::announce_migration(distributed& proxy, bool is_local_only) { if (!service::get_local_storage_service().cluster_supports_indexes()) { @@ -177,11 +208,16 @@ create_index_statement::announce_migration(distributed& } auto& db = proxy.local().get_db().local(); auto schema = db.find_schema(keyspace(), column_family()); - auto target = _raw_target->prepare(schema); + std::vector<::shared_ptr> targets; + for (auto& raw_target : _raw_targets) { + targets.emplace_back(raw_target->prepare(schema)); + } sstring accepted_name = _index_name; if (accepted_name.empty()) { std::experimental::optional index_name_root; - index_name_root = target->column->to_string(); + if (targets.size() == 1) { + index_name_root = targets[0]->column->to_string(); + } accepted_name = db.get_available_index_name(keyspace(), column_family(), index_name_root); } index_metadata_kind kind; @@ -192,7 +228,7 @@ create_index_statement::announce_migration(distributed& } else { kind = schema->is_compound() ? index_metadata_kind::composites : index_metadata_kind::keys; } - auto index = make_index_metadata(schema, { target }, accepted_name, kind, index_options); + auto index = make_index_metadata(schema, targets, accepted_name, kind, index_options); auto existing_index = schema->find_index_noname(index); if (existing_index) { if (_if_not_exists) { diff --git a/cql3/statements/create_index_statement.hh b/cql3/statements/create_index_statement.hh index 67a1561a1f..ac7fc71c37 100644 --- a/cql3/statements/create_index_statement.hh +++ b/cql3/statements/create_index_statement.hh @@ -67,14 +67,14 @@ namespace statements { /** A CREATE INDEX statement parsed from a CQL query. */ class create_index_statement : public schema_altering_statement { const sstring _index_name; - const ::shared_ptr _raw_target; + const std::vector<::shared_ptr> _raw_targets; const ::shared_ptr _properties; const bool _if_not_exists; public: create_index_statement(::shared_ptr name, ::shared_ptr index_name, - ::shared_ptr raw_target, + std::vector<::shared_ptr> raw_targets, ::shared_ptr properties, bool if_not_exists); future<> check_access(const service::client_state& state) override; @@ -94,6 +94,7 @@ private: void validate_is_values_index_if_target_column_not_collection(const column_definition* cd, ::shared_ptr target) const; void validate_target_column_is_map_if_index_involves_keys(bool is_map, ::shared_ptr target) const; + void validate_targets_for_multi_column_index(std::vector<::shared_ptr> targets) const; static index_metadata make_index_metadata(schema_ptr schema, const std::vector<::shared_ptr>& targets, const sstring& name,