alter tag/edge drop column, and the column to be delete is as TTL column

vesoft-inc · May 27, 2019 · 66386fc · 66386fc
1 parent 55b0c26
commit 66386fc
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 9 deletions.
diff --git a/src/graph/test/SchemaTest.cpp b/src/graph/test/SchemaTest.cpp
@@ -512,11 +512,12 @@ TEST_F(SchemaTest, TTLtest) {
         ASSERT_TRUE(verifyResult(resp, expected, expectedProp));
     }
     {
+        // when the column is as TTL column, droping column failed
         cpp2::ExecutionResponse resp;
         std::string query = "ALTER TAG woman "
                             "Drop (row_timestamp)";
         auto code = client->execute(query, resp);
-        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        ASSERT_NE(cpp2::ErrorCode::SUCCEEDED, code);
     }
     sleep(FLAGS_load_data_interval_secs + 1);
     {
@@ -527,14 +528,62 @@ TEST_F(SchemaTest, TTLtest) {
             {"email", "string"},
             {"age", "int"},
             {"gender", "string"},
+            {"row_timestamp", "timestamp"},
         };
-        // The correct behavior here has not been determined
         std::vector<uniform_tuple_t<std::string, 2>> expectedProp{
             {"ttl_duration", "200"},
             {"ttl_col", "row_timestamp"},
         };
         ASSERT_TRUE(verifyResult(resp, expected, expectedProp));
     }
+    {
+        // First remove TTL property, then drop column
+        cpp2::ExecutionResponse resp;
+        std::string query = "ALTER TAG woman "
+                            "ttl_col = age";
+        auto code = client->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+    }
+    sleep(FLAGS_load_data_interval_secs + 1);
+    {
+        cpp2::ExecutionResponse resp;
+        std::string query = "DESCRIBE TAG woman";
+        client->execute(query, resp);
+        std::vector<uniform_tuple_t<std::string, 2>> expected{
+            {"email", "string"},
+            {"age", "int"},
+            {"gender", "string"},
+            {"row_timestamp", "timestamp"},
+        };
+        std::vector<uniform_tuple_t<std::string, 2>> expectedProp{
+            {"ttl_duration", "200"},
+            {"ttl_col", "age"},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected, expectedProp));
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        std::string query = "ALTER TAG woman "
+                            "Drop (row_timestamp)";
+        auto code = client->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+    }
+    sleep(FLAGS_load_data_interval_secs + 1);
+    {
+        cpp2::ExecutionResponse resp;
+        std::string query = "DESCRIBE TAG woman";
+        client->execute(query, resp);
+        std::vector<uniform_tuple_t<std::string, 2>> expected{
+            {"email", "string"},
+            {"age", "int"},
+            {"gender", "string"},
+        };
+        std::vector<uniform_tuple_t<std::string, 2>> expectedProp{
+            {"ttl_duration", "200"},
+            {"ttl_col", "age"},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected, expectedProp));
+    }
 
     {
         cpp2::ExecutionResponse resp;
@@ -687,7 +736,7 @@ TEST_F(SchemaTest, TTLtest) {
         std::string query = "ALTER EDGE woman "
                             "Drop (row_timestamp)";
         auto code = client->execute(query, resp);
-        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+        ASSERT_NE(cpp2::ErrorCode::SUCCEEDED, code);
     }
     sleep(FLAGS_load_data_interval_secs + 1);
     {
@@ -698,14 +747,61 @@ TEST_F(SchemaTest, TTLtest) {
             {"email", "string"},
             {"age", "int"},
             {"gender", "string"},
+            {"row_timestamp", "timestamp"},
         };
-        // The correct behavior here has not been determined
         std::vector<uniform_tuple_t<std::string, 2>> expectedProp{
             {"ttl_duration", "200"},
             {"ttl_col", "row_timestamp"},
         };
         ASSERT_TRUE(verifyResult(resp, expected, expectedProp));
     }
+    {
+        cpp2::ExecutionResponse resp;
+        std::string query = "ALTER EDGE woman "
+                            "ttl_col = age";
+        auto code = client->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+    }
+    sleep(FLAGS_load_data_interval_secs + 1);
+    {
+        cpp2::ExecutionResponse resp;
+        std::string query = "DESCRIBE EDGE woman";
+        client->execute(query, resp);
+        std::vector<uniform_tuple_t<std::string, 2>> expected{
+            {"email", "string"},
+            {"age", "int"},
+            {"gender", "string"},
+            {"row_timestamp", "timestamp"},
+        };
+        std::vector<uniform_tuple_t<std::string, 2>> expectedProp{
+            {"ttl_duration", "200"},
+            {"ttl_col", "age"},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected, expectedProp));
+    }
+    {
+        cpp2::ExecutionResponse resp;
+        std::string query = "ALTER EDGE woman "
+                            "drop (row_timestamp)";
+        auto code = client->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
+    }
+    sleep(FLAGS_load_data_interval_secs + 1);
+    {
+        cpp2::ExecutionResponse resp;
+        std::string query = "DESCRIBE EDGE woman";
+        client->execute(query, resp);
+        std::vector<uniform_tuple_t<std::string, 2>> expected{
+            {"email", "string"},
+            {"age", "int"},
+            {"gender", "string"},
+        };
+        std::vector<uniform_tuple_t<std::string, 2>> expectedProp{
+            {"ttl_duration", "200"},
+            {"ttl_col", "age"},
+        };
+        ASSERT_TRUE(verifyResult(resp, expected, expectedProp));
+    }
 
     {
         cpp2::ExecutionResponse resp;

diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift
@@ -25,6 +25,7 @@ enum ErrorCode {
     E_EXISTED        = -22,
     E_NOT_FOUND      = -23,
     E_INVALID_HOST   = -24,
+    E_NOT_DROP       = -25,
 
     // KV Failure
     E_STORE_FAILURE          = -31,

diff --git a/src/meta/MetaServiceUtils.cpp b/src/meta/MetaServiceUtils.cpp
@@ -261,6 +261,7 @@ std::string MetaServiceUtils::assembleSegmentKey(const std::string& segment,
 }
 
 cpp2::ErrorCode MetaServiceUtils::alterColumnDefs(std::vector<nebula::cpp2::ColumnDef>& cols,
+                                                  nebula::cpp2::SchemaProp&  prop,
                                                   const nebula::cpp2::ColumnDef col,
                                                   const cpp2::AlterSchemaOptionType type) {
     switch (type) {
@@ -287,10 +288,17 @@ cpp2::ErrorCode MetaServiceUtils::alterColumnDefs(std::vector<nebula::cpp2::Colu
         }
         case cpp2::AlterSchemaOptionType::DROP:
         {
+            auto colName = col.get_name();
             for (auto it = cols.begin(); it != cols.end(); ++it) {
-                if (col.get_name() == it->get_name()) {
-                    cols.erase(it);
-                    return cpp2::ErrorCode::SUCCEEDED;
+                if (colName == it->get_name()) {
+                    // Check if there is a TTL on the column to be deleted
+                    if (prop.get_ttl_col() != colName) {
+                        cols.erase(it);
+                        return cpp2::ErrorCode::SUCCEEDED;
+                    } else {
+                        LOG(WARNING) << "Column not be deleted, as a TTL column : " << colName;
+                        return cpp2::ErrorCode::E_NOT_DROP;
+                    }
                 }
             }
             break;

diff --git a/src/meta/MetaServiceUtils.h b/src/meta/MetaServiceUtils.h
@@ -84,6 +84,7 @@ class MetaServiceUtils final {
     static std::string assembleSegmentKey(const std::string& segment, const std::string& key);
 
     static cpp2::ErrorCode alterColumnDefs(std::vector<nebula::cpp2::ColumnDef>& cols,
+                                           nebula::cpp2::SchemaProp&  prop,
                                            const nebula::cpp2::ColumnDef col,
                                            const cpp2::AlterSchemaOptionType op);
 

diff --git a/src/meta/processors/AlterEdgeProcessor.cpp b/src/meta/processors/AlterEdgeProcessor.cpp
@@ -44,7 +44,7 @@ void AlterEdgeProcessor::process(const cpp2::AlterEdgeReq& req) {
     for (auto& schemaOption : schemaOptions) {
         auto& cols = schemaOption.get_schema().get_columns();
         for (auto& col : cols) {
-            auto retCode = MetaServiceUtils::alterColumnDefs(columns, col, schemaOption.type);
+            auto retCode = MetaServiceUtils::alterColumnDefs(columns, prop, col, schemaOption.type);
             if (retCode != cpp2::ErrorCode::SUCCEEDED) {
                 LOG(WARNING) << "Alter edge option error " << static_cast<int32_t>(retCode);
                 resp_.set_code(retCode);

diff --git a/src/meta/processors/AlterTagProcessor.cpp b/src/meta/processors/AlterTagProcessor.cpp
@@ -44,7 +44,7 @@ void AlterTagProcessor::process(const cpp2::AlterTagReq& req) {
     for (auto& schemaOption : schemaOptions) {
         auto& cols = schemaOption.get_schema().get_columns();
         for (auto& col : cols) {
-            auto retCode = MetaServiceUtils::alterColumnDefs(columns, col, schemaOption.type);
+            auto retCode = MetaServiceUtils::alterColumnDefs(columns, prop, col, schemaOption.type);
             if (retCode != cpp2::ErrorCode::SUCCEEDED) {
                 LOG(WARNING) << "Alter tag option error " << static_cast<int32_t>(retCode);
                 resp_.set_code(retCode);