From 73ee82031ba771be905a6fafe19db3c4e8f96038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Tue, 2 Jun 2015 10:39:16 +0200 Subject: [PATCH 1/7] thrift: verify in set_keyspace that the keyspace exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original code just assigns value to _ks_name which is a sstring so the try { } catch { } clause only gave the wrong impression that something is checked. Signed-off-by: Paweł Dziepak --- thrift/handler.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/thrift/handler.cc b/thrift/handler.cc index ffca81ffa2..52a0296859 100644 --- a/thrift/handler.cc +++ b/thrift/handler.cc @@ -111,12 +111,12 @@ public: } void set_keyspace(tcxx::function cob, tcxx::function exn_cob, const std::string& keyspace) { - try { + if (!_db.local().has_keyspace(keyspace)) { + complete_with_exception(std::move(exn_cob), + "keyspace %s does not exist", keyspace); + } else { _ks_name = keyspace; cob(); - } catch (std::out_of_range& e) { - return complete_with_exception(std::move(exn_cob), - "keyspace %s does not exist", keyspace); } } From 43d915f88105c8b1f7e81659c086fb779300f408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Tue, 2 Jun 2015 11:57:37 +0200 Subject: [PATCH 2/7] class_registrator: check whether the class exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without the check added in this patch if the class doesn't exist a std::bad_function_call is thrown which is not very informative. Signed-off-by: Paweł Dziepak --- utils/class_registrator.hh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/utils/class_registrator.hh b/utils/class_registrator.hh index 625d1a625d..d373d8fabb 100644 --- a/utils/class_registrator.hh +++ b/utils/class_registrator.hh @@ -4,6 +4,11 @@ #pragma once +class no_such_class : public std::runtime_error { +public: + using runtime_error::runtime_error; +}; + // BaseType is a base type of a type hierarchy that this registry will hold // Args... are parameters for object's constructor template @@ -45,7 +50,11 @@ struct class_registrator { template std::unique_ptr class_registry::create(const sstring& name, Args... args) { - return _classes[name](args...); + auto it = _classes.find(name); + if (it == _classes.end()) { + throw no_such_class(name); + } + return it->second(args...); } template From aa13da39526ce6b90ecda1076c5af910198b1391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Tue, 2 Jun 2015 11:59:45 +0200 Subject: [PATCH 3/7] thrift: translate no_such_class exception MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- thrift/handler.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/thrift/handler.cc b/thrift/handler.cc index 52a0296859..738069eaa5 100644 --- a/thrift/handler.cc +++ b/thrift/handler.cc @@ -16,6 +16,7 @@ #include "utils/UUID_gen.hh" #include #include +#include "utils/class_registrator.hh" using namespace ::apache::thrift; using namespace ::apache::thrift::protocol; @@ -61,6 +62,8 @@ public: // It's an expected exception, so assume the message // is fine. Also, we don't want to change its type. throw; + } catch (no_such_class& nc) { + throw make_exception("unable to find class '%s'", nc.what()); } catch (std::exception& e) { // Unexpected exception, wrap it throw ::apache::thrift::TException(std::string("Internal server error: ") + e.what()); From d50859907f69147929fef3fa98fbbb95e81317be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Tue, 2 Jun 2015 12:48:19 +0200 Subject: [PATCH 4/7] db: update keyspace_metadata when column family is added MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- database.cc | 4 +++- database.hh | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/database.cc b/database.cc index 3e5b2b14b2..bc62e2a9b1 100644 --- a/database.cc +++ b/database.cc @@ -531,7 +531,8 @@ void database::drop_keyspace(const sstring& name) { } void database::add_column_family(const utils::UUID& uuid, column_family&& cf) { - if (_keyspaces.count(cf.schema()->ks_name()) == 0) { + auto ks = _keyspaces.find(cf.schema()->ks_name()); + if (ks == _keyspaces.end()) { throw std::invalid_argument("Keyspace " + cf.schema()->ks_name() + " not defined"); } if (_column_families.count(uuid) != 0) { @@ -541,6 +542,7 @@ void database::add_column_family(const utils::UUID& uuid, column_family&& cf) { if (_ks_cf_to_uuid.count(kscf) != 0) { throw std::invalid_argument("Column family " + cf.schema()->cf_name() + " exists"); } + ks->second.add_column_family(cf.schema()); _column_families.emplace(uuid, std::move(cf)); _ks_cf_to_uuid.emplace(std::move(kscf), uuid); } diff --git a/database.hh b/database.hh index 8d48a1739a..fe391f529a 100644 --- a/database.hh +++ b/database.hh @@ -187,6 +187,9 @@ public: const lw_shared_ptr& user_types() const { return _user_types; } + void add_column_family(const schema_ptr& s) { + _cf_meta_data.emplace(s->cf_name(), s); + } }; class keyspace { @@ -213,6 +216,9 @@ public: locator::abstract_replication_strategy& get_replication_strategy(); column_family::config make_column_family_config(const schema& s) const; future<> make_directory_for_column_family(const sstring& name, utils::UUID uuid); + void add_column_family(const schema_ptr& s) { + _metadata->add_column_family(s); + } private: sstring column_family_directory(const sstring& name, utils::UUID uuid) const; }; From 8e66bfc9d40366ec53db24e39af1b84eee41ba75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Tue, 2 Jun 2015 12:48:46 +0200 Subject: [PATCH 5/7] db: add getter for database::_keyspaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- database.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/database.hh b/database.hh index fe391f529a..a96b12dd12 100644 --- a/database.hh +++ b/database.hh @@ -277,6 +277,7 @@ public: bool has_keyspace(const sstring& name) const; void update_keyspace(const sstring& name); void drop_keyspace(const sstring& name); + const auto& keyspaces() const { return _keyspaces; } column_family& find_column_family(const sstring& ks, const sstring& name) throw (no_such_column_family); const column_family& find_column_family(const sstring& ks, const sstring& name) const throw (no_such_column_family); column_family& find_column_family(const utils::UUID&) throw (no_such_column_family); From 13e8ca96f1230c3aeaabcd95f082a68f2fd22f25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Tue, 2 Jun 2015 14:04:33 +0200 Subject: [PATCH 6/7] compound: make compound_type::type() const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- compound.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compound.hh b/compound.hh index e7304d68d7..99427917d9 100644 --- a/compound.hh +++ b/compound.hh @@ -55,7 +55,7 @@ public: compound_type(compound_type&&) = default; - auto const& types() { + auto const& types() const { return _types; } From f32a02a94e6d2abd01f4f0775116f08c29891a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Tue, 2 Jun 2015 14:09:23 +0200 Subject: [PATCH 7/7] thrift: implement describe_keyspace[s] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- thrift/handler.cc | 94 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 88 insertions(+), 6 deletions(-) diff --git a/thrift/handler.cc b/thrift/handler.cc index 738069eaa5..128add2050 100644 --- a/thrift/handler.cc +++ b/thrift/handler.cc @@ -352,9 +352,11 @@ public: } void describe_keyspaces(tcxx::function const& _return)> cob, tcxx::function exn_cob) { - std::vector _return; - // FIXME: implement - return pass_unimplemented(exn_cob); + std::vector ret; + for (auto&& ks : _db.local().keyspaces()) { + ret.emplace_back(get_keyspace_definition(ks.second)); + } + cob(ret); } void describe_cluster_name(tcxx::function cob) { @@ -400,9 +402,15 @@ public: } void describe_keyspace(tcxx::function cob, tcxx::function exn_cob, const std::string& keyspace) { - KsDef _return; - // FIXME: implement - return pass_unimplemented(exn_cob); + try { + auto& ks = _db.local().find_keyspace(keyspace); + KsDef ret = get_keyspace_definition(ks); + cob(ret); + } + catch (no_such_keyspace& nsk) { + complete_with_exception(std::move(exn_cob), + "keyspace %s does not exist", keyspace); + } } void describe_splits(tcxx::function const& _return)> cob, tcxx::function exn_cob, const std::string& cfName, const std::string& start_token, const std::string& end_token, const int32_t keys_per_split) { @@ -559,6 +567,80 @@ public: } private: + static sstring class_from_data_type(const data_type& dt) { + static const std::unordered_map types = { + { "boolean", "BooleanType" }, + { "bytes", "BytesType" }, + { "double", "DoubleType" }, + { "int32", "Int32Type" }, + { "long", "LongType" }, + { "timestamp", "DateType" }, + { "timeuuid", "TimeUUIDType" }, + { "utf8", "UTF8Type" }, + { "uuid", "UUIDType" }, + // FIXME: missing types + }; + auto it = types.find(dt->name()); + if (it == types.end()) { + return sstring(" ") + dt->name(); + } + return sstring("org.apache.cassandra.db.marshal.") + it->second; + } + static sstring class_from_compound_type(const compound_type& ct) { + if (ct.is_singular()) { + return class_from_data_type(ct.types().front()); + } + sstring type = "org.apache.cassandra.db.marshal.CompositeType("; + for (auto& dt : ct.types()) { + type += class_from_data_type(dt); + if (&dt != &*ct.types().rbegin()) { + type += ","; + } + } + type += ")"; + return type; + } + static KsDef get_keyspace_definition(const keyspace& ks) { + auto&& meta = ks.metadata(); + KsDef def; + def.__set_name(meta->name()); + def.__set_strategy_class(meta->strategy_name()); + std::map options( + meta->strategy_options().begin(), + meta->strategy_options().end()); + def.__set_strategy_options(options); + std::vector cfs; + for (auto&& cf : meta->cf_meta_data()) { + // FIXME: skip cql3 column families + auto&& s = cf.second; + CfDef cf_def; + cf_def.__set_keyspace(s->ks_name()); + cf_def.__set_name(s->cf_name()); + cf_def.__set_key_validation_class(class_from_compound_type(*s->partition_key_type())); + if (s->clustering_key_size()) { + cf_def.__set_comparator_type(class_from_compound_type(*s->clustering_key_type())); + } else { + cf_def.__set_comparator_type(class_from_data_type(s->regular_column_name_type())); + } + cf_def.__set_comment(s->comment()); + cf_def.__set_bloom_filter_fp_chance(s->bloom_filter_fp_chance()); + if (s->regular_columns_count()) { + std::vector columns; + for (auto&& c : s->regular_columns()) { + ColumnDef c_def; + c_def.__set_name(c.name_as_text()); + c_def.__set_validation_class(class_from_data_type(c.type)); + columns.emplace_back(std::move(c_def)); + } + cf_def.__set_column_metadata(columns); + } + // FIXME: there are more fields that should be filled... + cfs.emplace_back(std::move(cf_def)); + } + def.__set_cf_defs(cfs); + def.__set_durable_writes(meta->durable_writes()); + return std::move(def); + } static column_family& lookup_column_family(database& db, const sstring& ks_name, const sstring& cf_name) { try { return db.find_column_family(ks_name, cf_name);