diff --git a/cql3/statements/alter_table_statement.cc b/cql3/statements/alter_table_statement.cc index 3fc90987c7d1..30aa489cfe17 100644 --- a/cql3/statements/alter_table_statement.cc +++ b/cql3/statements/alter_table_statement.cc @@ -398,20 +398,19 @@ alter_table_statement::prepare_schema_mutations(query_processor& qp, api::timest std::unique_ptr cql3::statements::alter_table_statement::prepare(data_dictionary::database db, cql_stats& stats) { + auto t = db.try_find_table(keyspace(), column_family()); + std::optional s = t ? std::make_optional(t->schema()) : std::nullopt; + std::optional warning = check_restricted_table_properties(db, s, keyspace(), column_family(), *_properties); + if (warning) { + mylogger.warn("{}", *warning); + } return std::make_unique(make_shared(*this)); } future<::shared_ptr> alter_table_statement::execute(query_processor& qp, service::query_state& state, const query_options& options, std::optional guard) const { - auto s = validation::validate_column_family(qp.db(), keyspace(), column_family()); - std::optional warning = check_restricted_table_properties(qp, s, keyspace(), column_family(), *_properties); - return schema_altering_statement::execute(qp, state, options, std::move(guard)).then([warning = std::move(warning)] (::shared_ptr msg) { - if (warning) { - msg->add_warning(*warning); - mylogger.warn("{}", *warning); - } - return msg; - }); + validation::validate_column_family(qp.db(), keyspace(), column_family()); + return schema_altering_statement::execute(qp, state, options, std::move(guard)); } } diff --git a/cql3/statements/create_table_statement.cc b/cql3/statements/create_table_statement.cc index f0e1525740d9..9d21b31eae24 100644 --- a/cql3/statements/create_table_statement.cc +++ b/cql3/statements/create_table_statement.cc @@ -181,6 +181,10 @@ std::unique_ptr create_table_statement::raw_statement::prepa if (_properties.properties()->get_synchronous_updates_flag()) { throw exceptions::invalid_request_exception(format("The synchronous_updates option is only applicable to materialized views, not to base tables")); } + std::optional warning = check_restricted_table_properties(db, std::nullopt, keyspace(), column_family(), *_properties.properties()); + if (warning) { + mylogger.warn("{}", *warning); + } const bool has_default_ttl = _properties.properties()->get_default_time_to_live() > 0; auto stmt = ::make_shared(*_cf_name, _properties.properties(), _if_not_exists, _static_columns, _properties.properties()->get_id()); @@ -422,7 +426,7 @@ void create_table_statement::raw_statement::add_column_alias(::shared_ptr check_restricted_table_properties( - query_processor& qp, + data_dictionary::database db, std::optional schema, const sstring& keyspace, const sstring& table, const cf_prop_defs& cfprops) @@ -453,7 +457,7 @@ std::optional check_restricted_table_properties( std::map options = (strategy) ? cfprops.get_compaction_type_options() : (*schema)->compaction_strategy_options(); sstables::time_window_compaction_strategy_options twcs_options(options); long ttl = (cfprops.has_property(cf_prop_defs::KW_DEFAULT_TIME_TO_LIVE)) ? cfprops.get_default_time_to_live() : current_ttl.count(); - auto max_windows = qp.db().get_config().twcs_max_window_count(); + auto max_windows = db.get_config().twcs_max_window_count(); // It may happen that an user tries to update an unrelated table property. Allow the request through. if (!cfprops.has_property(cf_prop_defs::KW_DEFAULT_TIME_TO_LIVE) && !strategy) { @@ -473,7 +477,7 @@ std::optional check_restricted_table_properties( "highly discouraged.", ttl, twcs_options.get_sstable_window_size().count(), window_count, max_windows)); } } else { - switch (qp.db().get_config().restrict_twcs_without_default_ttl()) { + switch (db.get_config().restrict_twcs_without_default_ttl()) { case db::tri_mode_restriction_t::mode::TRUE: throw exceptions::configuration_exception( "TimeWindowCompactionStrategy tables without a strict default_time_to_live setting " @@ -492,18 +496,6 @@ std::optional check_restricted_table_properties( return std::nullopt; } -future<::shared_ptr> -create_table_statement::execute(query_processor& qp, service::query_state& state, const query_options& options, std::optional guard) const { - std::optional warning = check_restricted_table_properties(qp, std::nullopt, keyspace(), column_family(), *_properties); - return schema_altering_statement::execute(qp, state, options, std::move(guard)).then([warning = std::move(warning)] (::shared_ptr msg) { - if (warning) { - msg->add_warning(*warning); - mylogger.warn("{}", *warning); - } - return msg; - }); -} - } } diff --git a/cql3/statements/create_table_statement.hh b/cql3/statements/create_table_statement.hh index 4f4ade0dc7db..d7af486a0178 100644 --- a/cql3/statements/create_table_statement.hh +++ b/cql3/statements/create_table_statement.hh @@ -77,9 +77,6 @@ public: virtual future<> grant_permissions_to_creator(const service::client_state&) const override; - virtual future<::shared_ptr> - execute(query_processor& qp, service::query_state& state, const query_options& options, std::optional guard) const override; - schema_ptr get_cf_meta_data(const data_dictionary::database) const; class raw_statement; @@ -127,7 +124,7 @@ public: }; std::optional check_restricted_table_properties( - query_processor& qp, + data_dictionary::database db, std::optional schema, const sstring& keyspace, const sstring& table, const cf_prop_defs& cfprops); diff --git a/test/boost/cql_query_test.cc b/test/boost/cql_query_test.cc index a04c9e8f282e..33068d68c481 100644 --- a/test/boost/cql_query_test.cc +++ b/test/boost/cql_query_test.cc @@ -103,15 +103,17 @@ SEASTAR_TEST_CASE(test_create_twcs_table_no_ttl) { e.execute_cql("ALTER TABLE tbl WITH default_time_to_live=60;").get(); // LiveUpdate and enforce TTL to be defined e.execute_cql("UPDATE system.config SET value='true' WHERE name='restrict_twcs_without_default_ttl';").get(); - assert_that_failed(e.execute_cql("CREATE TABLE tbl2 (a int, b int, PRIMARY KEY (a)) WITH " - "compaction = {'class': 'TimeWindowCompactionStrategy', " - "'compaction_window_size': '1', 'compaction_window_unit': 'MINUTES'};")); + // default_time_to_live option is required + BOOST_REQUIRE_THROW(e.execute_cql("CREATE TABLE tbl2 (a int, b int, PRIMARY KEY (a)) WITH " + "compaction = {'class': 'TimeWindowCompactionStrategy', " + "'compaction_window_size': '1', 'compaction_window_unit': 'MINUTES'};").get(), exceptions::configuration_exception); e.execute_cql("CREATE TABLE tbl2 (a int, b int, PRIMARY KEY (a)) WITH " "compaction = {'class': 'TimeWindowCompactionStrategy', " "'compaction_window_size': '1', 'compaction_window_unit': 'MINUTES'} AND " "default_time_to_live=60").get(); BOOST_REQUIRE(e.local_db().has_schema("ks", "tbl2")); - assert_that_failed(e.execute_cql("ALTER TABLE tbl WITH default_time_to_live=0;")); + // default_time_to_live option must not be set to 0. + BOOST_REQUIRE_THROW(e.execute_cql("ALTER TABLE tbl WITH default_time_to_live=0;").get(), exceptions::configuration_exception); // LiveUpdate and disable the check, then try table creation again e.execute_cql("UPDATE system.config SET value='false' WHERE name='restrict_twcs_without_default_ttl';").get(); e.execute_cql("CREATE TABLE tbl3 (a int, b int, PRIMARY KEY (a)) WITH " @@ -130,10 +132,10 @@ SEASTAR_TEST_CASE(test_twcs_max_window) { // Hardcode restriction to max 10 windows e.execute_cql("UPDATE system.config SET value='10' WHERE name='twcs_max_window_count';").get(); // Creating a TWCS table with a large number of windows/buckets should fail - assert_that_failed(e.execute_cql("CREATE TABLE tbl (a int, b int, PRIMARY KEY (a)) WITH " - "compaction = {'class': 'TimeWindowCompactionStrategy', " - "'compaction_window_size': '1', 'compaction_window_unit': 'HOURS'} " - "AND default_time_to_live=86400;")); + BOOST_REQUIRE_THROW(e.execute_cql("CREATE TABLE tbl (a int, b int, PRIMARY KEY (a)) WITH " + "compaction = {'class': 'TimeWindowCompactionStrategy', " + "'compaction_window_size': '1', 'compaction_window_unit': 'HOURS'} " + "AND default_time_to_live=86400;").get(), exceptions::configuration_exception); // However the creation of a table within bounds should succeed e.execute_cql("CREATE TABLE tbl (a int, b int, PRIMARY KEY (a)) WITH " "compaction = {'class': 'TimeWindowCompactionStrategy', " @@ -168,19 +170,19 @@ SEASTAR_TEST_CASE(test_twcs_restrictions_mixed) { // Cenario 3: STCS->TWCS with large TTL value e.execute_cql("CREATE TABLE tbl3 (a int PRIMARY KEY, b int) WITH default_time_to_live = 8640000;").get(); BOOST_REQUIRE(e.local_db().has_schema("ks", "tbl3")); - assert_that_failed(e.execute_cql("ALTER TABLE tbl3 WITH compaction = {'class': 'TimeWindowCompactionStrategy'};")); + BOOST_REQUIRE_THROW(e.execute_cql("ALTER TABLE tbl3 WITH compaction = {'class': 'TimeWindowCompactionStrategy'};").get(), exceptions::configuration_exception); // Cenario 4: TWCS table with small to large TTL e.execute_cql("CREATE TABLE tbl4 (a int PRIMARY KEY, b int) WITH compaction = " "{'class': 'TimeWindowCompactionStrategy'} AND default_time_to_live = 60;").get(); BOOST_REQUIRE(e.local_db().has_schema("ks", "tbl4")); - assert_that_failed(e.execute_cql("ALTER TABLE tbl4 WITH default_time_to_live = 86400000;")); + BOOST_REQUIRE_THROW(e.execute_cql("ALTER TABLE tbl4 WITH default_time_to_live = 86400000;").get(), exceptions::configuration_exception); // Cenario 5: No TTL TWCS to large TTL and then small TTL e.execute_cql("CREATE TABLE tbl5 (a int PRIMARY KEY, b int) WITH compaction = " "{'class': 'TimeWindowCompactionStrategy'}").get(); BOOST_REQUIRE(e.local_db().has_schema("ks", "tbl5")); - assert_that_failed(e.execute_cql("ALTER TABLE tbl5 WITH default_time_to_live = 86400000;")); + BOOST_REQUIRE_THROW(e.execute_cql("ALTER TABLE tbl5 WITH default_time_to_live = 86400000;").get(), exceptions::configuration_exception); e.execute_cql("ALTER TABLE tbl5 WITH default_time_to_live = 60;").get(); // Cenario 6: twcs_max_window_count LiveUpdate - Decrease TTL @@ -216,7 +218,7 @@ SEASTAR_TEST_CASE(test_twcs_restrictions_mixed) { // Cenario 10: Large TTL STCS table, fail to switch to TWCS with no TTL e.execute_cql("CREATE TABLE tbl10 (a int PRIMARY KEY, b int) WITH default_time_to_live = 8640000;").get(); BOOST_REQUIRE(e.local_db().has_schema("ks", "tbl10")); - assert_that_failed(e.execute_cql("ALTER TABLE tbl10 WITH compaction = {'class': 'TimeWindowCompactionStrategy'};")); + BOOST_REQUIRE_THROW(e.execute_cql("ALTER TABLE tbl10 WITH compaction = {'class': 'TimeWindowCompactionStrategy'};").get(), exceptions::configuration_exception); e.execute_cql("ALTER TABLE tbl10 WITH compaction = {'class': 'TimeWindowCompactionStrategy'} AND default_time_to_live = 0;").get(); // Cenario 11: Ensure default_time_to_live updates reference existing table properties @@ -224,21 +226,21 @@ SEASTAR_TEST_CASE(test_twcs_restrictions_mixed) { "{'class': 'TimeWindowCompactionStrategy', 'compaction_window_size': '1', " "'compaction_window_unit': 'MINUTES'} AND default_time_to_live=3000;").get(); BOOST_REQUIRE(e.local_db().has_schema("ks", "tbl11")); - assert_that_failed(e.execute_cql("ALTER TABLE tbl11 WITH default_time_to_live=3600;")); + BOOST_REQUIRE_THROW(e.execute_cql("ALTER TABLE tbl11 WITH default_time_to_live=3600;").get(), exceptions::configuration_exception); e.execute_cql("ALTER TABLE tbl11 WITH compaction = {'class': 'TimeWindowCompactionStrategy', " "'compaction_window_size': '2', 'compaction_window_unit': 'MINUTES'};").get(); e.execute_cql("ALTER TABLE tbl11 WITH default_time_to_live=3600;").get(); // Cenario 12: Ensure that window sizes <= 0 are forbidden - assert_that_failed(e.execute_cql("CREATE TABLE tbl12 (a int PRIMARY KEY, b int) WITH compaction = " - "{'class': 'TimeWindowCompactionStrategy', 'compaction_window_size': '0'};")); - assert_that_failed(e.execute_cql("CREATE TABLE tbl12 (a int PRIMARY KEY, b int) WITH compaction = " - "{'class': 'TimeWindowCompactionStrategy', 'compaction_window_size': -65535};")); + BOOST_REQUIRE_THROW(e.execute_cql("CREATE TABLE tbl12 (a int PRIMARY KEY, b int) WITH compaction = " + "{'class': 'TimeWindowCompactionStrategy', 'compaction_window_size': '0'};").get(), exceptions::configuration_exception); + BOOST_REQUIRE_THROW(e.execute_cql("CREATE TABLE tbl12 (a int PRIMARY KEY, b int) WITH compaction = " + "{'class': 'TimeWindowCompactionStrategy', 'compaction_window_size': -65535};").get(), exceptions::configuration_exception); e.execute_cql("CREATE TABLE tbl12 (a int PRIMARY KEY, b int) WITH compaction = " "{'class': 'TimeWindowCompactionStrategy', 'compaction_window_size': 1};").get(); BOOST_REQUIRE(e.local_db().has_schema("ks", "tbl12")); - assert_that_failed(e.execute_cql("ALTER TABLE tbl12 WITH compaction = {'class': 'TimeWindowCompactionStrategy', " - "'compaction_window_size': 0};")); + BOOST_REQUIRE_THROW(e.execute_cql("ALTER TABLE tbl12 WITH compaction = {'class': 'TimeWindowCompactionStrategy', " + "'compaction_window_size': 0};").get(), exceptions::configuration_exception); }); }