From 975e1c8fc60c1934fb84fa9da04718bb62ee9b8f Mon Sep 17 00:00:00 2001
From: Pekka Enberg <penberg@scylladb.com>
Date: Wed, 12 Apr 2017 20:24:57 +0300
Subject: [PATCH] cql3/statements/create_index_statement: Extract validations

Extract specific validations to separate functions to preserve the same
structure as Apache Cassandra code and make it easier to add support for
multiple index targets.
---
 cql3/statements/create_index_statement.cc | 78 +++++++++++++----------
 cql3/statements/create_index_statement.hh |  6 ++
 2 files changed, 52 insertions(+), 32 deletions(-)

diff --git a/cql3/statements/create_index_statement.cc b/cql3/statements/create_index_statement.cc
index 63df7c3c3e..195f68aa73 100644
--- a/cql3/statements/create_index_statement.cc
+++ b/cql3/statements/create_index_statement.cc
@@ -94,39 +94,11 @@ create_index_statement::validate(distributed<service::storage_proxy>& proxy, con
     bool is_frozen_collection = cd->type->is_collection() && !cd->type->is_multi_cell();
 
     if (is_frozen_collection) {
-        if (target->type != index_target::target_type::full) {
-            throw exceptions::invalid_request_exception(
-                    sprint("Cannot create index on %s of frozen<map> column %s",
-                            index_target::index_option(target->type),
-                            *target->column));
-        }
+        validate_for_frozen_collection(target);
     } else {
-        // validateNotFullIndex
-        if (target->type == index_target::target_type::full) {
-            throw exceptions::invalid_request_exception("full() indexes can only be created on frozen collections");
-        }
-        // validateIsValuesIndexIfTargetColumnNotCollection
-        if (!cd->type->is_collection()
-                && target->type != index_target::target_type::values) {
-            throw exceptions::invalid_request_exception(
-                    sprint(
-                            "Cannot create index on %s of column %s; only non-frozen collections support %s indexes",
-                            index_target::index_option(target->type),
-                            *target->column,
-                            index_target::index_option(target->type)));
-        }
-        // validateTargetColumnIsMapIfIndexInvolvesKeys
-        if (target->type == index_target::target_type::keys
-                || target->type == index_target::target_type::keys_and_values) {
-            if (!is_map) {
-                throw exceptions::invalid_request_exception(
-                        sprint(
-                                "Cannot create index on %s of column %s with non-map type",
-                                index_target::index_option(target->type),
-                                *target->column));
-
-            }
-        }
+        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) {
@@ -161,6 +133,48 @@ create_index_statement::validate(distributed<service::storage_proxy>& proxy, con
     }
 }
 
+void create_index_statement::validate_for_frozen_collection(::shared_ptr<index_target> target) const
+{
+    if (target->type != index_target::target_type::full) {
+        throw exceptions::invalid_request_exception(
+                sprint("Cannot create index on %s of frozen<map> column %s",
+                        index_target::index_option(target->type),
+                        *target->column));
+    }
+}
+
+void create_index_statement::validate_not_full_index(::shared_ptr<index_target> target) const
+{
+    if (target->type == index_target::target_type::full) {
+        throw exceptions::invalid_request_exception("full() indexes can only be created on frozen collections");
+    }
+}
+
+void create_index_statement::validate_is_values_index_if_target_column_not_collection(
+        const column_definition* cd, ::shared_ptr<index_target> target) const
+{
+    if (!cd->type->is_collection()
+            && target->type != index_target::target_type::values) {
+        throw exceptions::invalid_request_exception(
+                sprint("Cannot create index on %s of column %s; only non-frozen collections support %s indexes",
+                       index_target::index_option(target->type),
+                       *target->column,
+                       index_target::index_option(target->type)));
+    }
+}
+
+void create_index_statement::validate_target_column_is_map_if_index_involves_keys(bool is_map, ::shared_ptr<index_target> target) const
+{
+    if (target->type == index_target::target_type::keys
+            || target->type == index_target::target_type::keys_and_values) {
+        if (!is_map) {
+            throw exceptions::invalid_request_exception(
+                    sprint("Cannot create index on %s of column %s with non-map type",
+                           index_target::index_option(target->type), *target->column));
+        }
+    }
+}
+
 future<bool>
 create_index_statement::announce_migration(distributed<service::storage_proxy>& proxy, bool is_local_only) {
     throw std::runtime_error("Indexes are not supported yet");
diff --git a/cql3/statements/create_index_statement.hh b/cql3/statements/create_index_statement.hh
index 80742e7a67..86284c1c5d 100644
--- a/cql3/statements/create_index_statement.hh
+++ b/cql3/statements/create_index_statement.hh
@@ -88,6 +88,12 @@ public:
                 column_family());
     }
     virtual shared_ptr<prepared> prepare(database& db, cql_stats& stats) override;
+private:
+    void validate_for_frozen_collection(::shared_ptr<index_target> target) const;
+    void validate_not_full_index(::shared_ptr<index_target> target) const;
+    void validate_is_values_index_if_target_column_not_collection(const column_definition* cd,
+                                                                  ::shared_ptr<index_target> target) const;
+    void validate_target_column_is_map_if_index_involves_keys(bool is_map, ::shared_ptr<index_target> target) const;
 };
 
 }