From a544ca64e2cfe3e592deb41f4d317ddc7bdc4c47 Mon Sep 17 00:00:00 2001 From: Piotr Sarna Date: Thu, 2 Jul 2020 10:35:19 +0200 Subject: [PATCH] cql3: refuse to change schema internally for distributed tables Changing the schemas via internal calls to CQL is dangerous, since the changes are not propagated to other nodes. Thus, it should never be used for regular distributed tables. The guarding code was already added for ALTER TABLE statement and it's now expanded to cover all schema altering statements. Tests: unit(dev) Fixes #6700 --- cql3/statements/alter_table_statement.cc | 6 ------ cql3/statements/schema_altering_statement.cc | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/cql3/statements/alter_table_statement.cc b/cql3/statements/alter_table_statement.cc index 0b61f11c0c..2da3452711 100644 --- a/cql3/statements/alter_table_statement.cc +++ b/cql3/statements/alter_table_statement.cc @@ -291,12 +291,6 @@ future> alter_table_statement::a throw exceptions::invalid_request_exception("Cannot use ALTER TABLE on Materialized View"); } - const auto& ks = db.find_keyspace(keyspace()); - auto replication_type = ks.get_replication_strategy().get_type(); - if (is_local_only && replication_type != locator::replication_strategy_type::local) { - throw std::logic_error(format("Internal queries should not try to alter table schema for non-local tables, because it leads to inconsistencies: {}.{}", - s->ks_name(), s->cf_name())); - } auto cfm = schema_builder(s); if (_properties->get_id()) { diff --git a/cql3/statements/schema_altering_statement.cc b/cql3/statements/schema_altering_statement.cc index 32be4da70a..546fc26303 100644 --- a/cql3/statements/schema_altering_statement.cc +++ b/cql3/statements/schema_altering_statement.cc @@ -40,6 +40,8 @@ */ #include "cql3/statements/schema_altering_statement.hh" +#include "locator/abstract_replication_strategy.hh" +#include "database.hh" #include "transport/messages/result_message.hh" @@ -110,6 +112,19 @@ schema_altering_statement::execute0(service::storage_proxy& proxy, service::quer future<::shared_ptr> schema_altering_statement::execute(service::storage_proxy& proxy, service::query_state& state, const query_options& options) const { bool internal = state.get_client_state().is_internal(); + if (internal) { + auto replication_type = locator::replication_strategy_type::everywhere_topology; + database& db = proxy.get_db().local(); + if (_cf_name && _cf_name->has_keyspace()) { + const auto& ks = db.find_keyspace(_cf_name->get_keyspace()); + replication_type = ks.get_replication_strategy().get_type(); + } + if (replication_type != locator::replication_strategy_type::local) { + sstring info = _cf_name ? _cf_name->to_string() : "schema"; + throw std::logic_error(format("Attempted to modify {} via internal query: such schema changes are not propagated and thus illegal", info)); + } + } + return execute0(proxy, state, options, internal).then([this, &state, internal](::shared_ptr result) { auto permissions_granted_fut = internal ? make_ready_future<>()