Files
scylladb/cql3/statements/schema_altering_statement.cc
Nadav Har'El 5984db047d Merge 'mv: forbid IS NOT NULL on columns outside the primary key' from Jan Ciołek
statement_restrictions: forbid IS NOT NULL on columns outside the primary key

IS NOT NULL is currently allowed only when creating materialized views.
It's used to convey that the view will not include any rows that would make the view's primary key columns NULL.

Generally materialized views allow to place restrictions on the primary key columns, but restrictions on the regular columns are forbidden. The exception was IS NOT NULL - it was allowed to write regular_col IS NOT NULL. The problem is that this restriction isn't respected, it's just silently ignored (see #10365).

Supporting IS NOT NULL on regular columns seems to be as hard as supporting any other restrictions on regular columns.
It would be a big effort, and there are some reasons why we don't support them.

For now let's forbid such restrictions, it's better to fail than be wrong silently.

Throwing a hard error would be a breaking change.
To avoid breaking existing code the reaction to an invalid IS NOT NULL restrictions is controlled by the `strict_is_not_null_in_views` flag.

This flag can have the following values:
* `true` - strict checking. Having an `IS NOT NULL` restriction on a column that doesn't belong to the view's primary key causes an error to be thrown.
* `warn` - allow invalid `IS NOT NULL` restrictions, but throw a warning. The invalid restrictions are silently ignored.
* `false` - allow invalid `IS NOT NULL` restricitons, without any warnings or errors. The invalid restrictions are silently ignored.

The default values for this flag are `warn` in `db::config` and `true` in scylla.yaml.

This way the existing clusters will have `warn` by default, so they'll get a warning if they try to create such an invalid view.

New clusters with fresh scylla.yaml will have the flag set to `true`, as scylla.yaml overwrites the default value in `db::config`.
New clusters will throw a hard error for invalid views, but in older existing clusters it will just be a warning.
This way we can maintain backwards compatibility, but still move forward by rejecting invalid queries on new clusters.

Fixes: #10365

Closes #13013

* github.com:scylladb/scylladb:
  boost/restriction_test: test the strict_is_not_null_in_views flag
  docs/cql/mv: columns outside of view's primary key can't be restricted
  cql-pytest: enable test_is_not_null_forbidden_in_filter
  statement_restrictions: forbid IS NOT NULL on columns outside the primary key
  schema_altering_statement: return warnings from prepare_schema_mutations()
  db/config: add strict_is_not_null_in_views config option
  statement_restrictions: add get_not_null_columns()
  test: remove invalid IS NOT NULL restrictions from tests
2023-06-07 12:12:19 +03:00

145 lines
5.0 KiB
C++

/*
* Copyright (C) 2015-present ScyllaDB
*
* Modified by ScyllaDB
*/
/*
* SPDX-License-Identifier: (AGPL-3.0-or-later and Apache-2.0)
*/
#include <seastar/core/coroutine.hh>
#include "cql3/statements/schema_altering_statement.hh"
#include "locator/abstract_replication_strategy.hh"
#include "data_dictionary/data_dictionary.hh"
#include "mutation/mutation.hh"
#include "cql3/query_processor.hh"
#include "transport/messages/result_message.hh"
#include "service/raft/raft_group_registry.hh"
#include "service/migration_manager.hh"
namespace cql3 {
namespace statements {
static logging::logger mylogger("schema_altering_statement");
schema_altering_statement::schema_altering_statement(timeout_config_selector timeout_selector)
: cf_statement(cf_name())
, cql_statement_no_metadata(timeout_selector)
, _is_column_family_level{false}
{
}
schema_altering_statement::schema_altering_statement(cf_name name, timeout_config_selector timeout_selector)
: cf_statement{std::move(name)}
, cql_statement_no_metadata(timeout_selector)
, _is_column_family_level{true}
{
}
future<> schema_altering_statement::grant_permissions_to_creator(const service::client_state&) const {
return make_ready_future<>();
}
bool schema_altering_statement::depends_on(std::string_view ks_name, std::optional<std::string_view> cf_name) const
{
return false;
}
uint32_t schema_altering_statement::get_bound_terms() const
{
return 0;
}
void schema_altering_statement::prepare_keyspace(const service::client_state& state)
{
if (_is_column_family_level) {
cf_statement::prepare_keyspace(state);
}
}
future<::shared_ptr<messages::result_message>>
schema_altering_statement::execute0(query_processor& qp, service::query_state& state, const query_options& options) const {
auto& mm = qp.get_migration_manager();
::shared_ptr<cql_transport::event::schema_change> ce;
if (this_shard_id() != 0) {
// execute all schema altering statements on a shard zero since this is where raft group 0 is
co_return ::make_shared<cql_transport::messages::result_message::bounce_to_shard>(0,
std::move(const_cast<cql3::query_options&>(options).take_cached_pk_function_calls()));
}
cql3::cql_warnings_vec warnings;
auto retries = mm.get_concurrent_ddl_retries();
while (true) {
try {
auto group0_guard = co_await mm.start_group0_operation();
auto [ret, m, cql_warnings] = co_await prepare_schema_mutations(qp, group0_guard.write_timestamp());
warnings = std::move(cql_warnings);
if (!m.empty()) {
auto description = format("CQL DDL statement: \"{}\"", raw_cql_statement);
co_await mm.announce(std::move(m), std::move(group0_guard), description);
}
ce = std::move(ret);
} catch (const service::group0_concurrent_modification&) {
mylogger.warn("Failed to execute DDL statement \"{}\" due to concurrent group 0 modification.{}.",
raw_cql_statement, retries ? " Retrying" : " Number of retries exceeded, giving up");
if (retries--) {
continue;
}
throw;
}
break;
}
// If an IF [NOT] EXISTS clause was used, this may not result in an actual schema change. To avoid doing
// extra work in the drivers to handle schema changes, we return an empty message in this case. (CASSANDRA-7600)
::shared_ptr<messages::result_message> result;
if (!ce) {
result = ::make_shared<messages::result_message::void_message>();
} else {
result = ::make_shared<messages::result_message::schema_change>(ce);
}
for (const sstring& warning : warnings) {
result->add_warning(warning);
}
co_return result;
}
future<::shared_ptr<messages::result_message>>
schema_altering_statement::execute(query_processor& qp, 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;
data_dictionary::database db = qp.db();
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(qp, state, options).then([this, &state, internal](::shared_ptr<messages::result_message> result) {
auto permissions_granted_fut = internal
? make_ready_future<>()
: grant_permissions_to_creator(state.get_client_state());
return permissions_granted_fut.then([result = std::move(result)] {
return result;
});
});
}
}
}