You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/07/21 07:04:59 UTC

[GitHub] [iotdb] ijihang opened a new pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

ijihang opened a new pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608


   JR:https://issues.apache.org/jira/browse/IOTDB-959


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] coveralls commented on pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#issuecomment-883987015


   
   [![Coverage Status](https://coveralls.io/builds/41532712/badge)](https://coveralls.io/builds/41532712)
   
   Coverage increased (+0.01%) to 68.146% when pulling **6f773794c397c5b65ca5bb6d0584876b8f914781 on ijihang:CreateStorageGroup** into **6e9f3404442108c08493351d1447e6d2c1ec1347 on apache:master**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] coveralls edited a comment on pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#issuecomment-883987015


   
   [![Coverage Status](https://coveralls.io/builds/41561373/badge)](https://coveralls.io/builds/41561373)
   
   Coverage remained the same at 68.134% when pulling **69175ca7757d23d6f141fe51909681f117ea04cc on ijihang:CreateStorageGroup** into **6e9f3404442108c08493351d1447e6d2c1ec1347 on apache:master**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] coveralls edited a comment on pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#issuecomment-883987015


   
   [![Coverage Status](https://coveralls.io/builds/41544613/badge)](https://coveralls.io/builds/41544613)
   
   Coverage increased (+0.005%) to 68.139% when pulling **7057cb438402ec143969eb2ea0768f572230f318 on ijihang:CreateStorageGroup** into **6e9f3404442108c08493351d1447e6d2c1ec1347 on apache:master**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] Alima777 commented on a change in pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#discussion_r674803830



##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBCreateStorageGroupIT.java
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.integration;
+
+import org.apache.iotdb.db.utils.EnvironmentUtils;
+import org.apache.iotdb.jdbc.Config;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Notice that, all test begins with "IoTDB" is integration test. All test which will start the
+ * IoTDB server should be defined as integration test.
+ */
+public class IoTDBCreateStorageGroupIT {
+  private Statement statement;
+  private Connection connection;
+
+  @Before
+  public void setUp() throws Exception {
+    EnvironmentUtils.envSetUp();
+
+    Class.forName(Config.JDBC_DRIVER_NAME);
+    connection = DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
+    statement = connection.createStatement();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    statement.close();
+    connection.close();
+    EnvironmentUtils.cleanEnv();
+  }
+
+  /** The test creates three storage groups */
+  @Test
+  public void testCreateStorageGroup() throws Exception {
+    String[] storageGroups = {"root.sg1", "root.sg2", "root.sg3"};
+
+    for (String storageGroup : storageGroups) {
+      statement.execute(String.format("create storage group %s", storageGroup));
+    }
+
+    // ensure that current StorageGroup in cache is right.
+    createStorageGroupTool(storageGroups);
+
+    EnvironmentUtils.stopDaemon();
+    setUp();

Review comment:
       ok :D




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] Alima777 commented on a change in pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#discussion_r674445233



##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBCreateStorageGroupIT.java
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.integration;
+
+import org.apache.iotdb.db.utils.EnvironmentUtils;
+import org.apache.iotdb.jdbc.Config;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Notice that, all test begins with "IoTDB" is integration test. All test which will start the
+ * IoTDB server should be defined as integration test.
+ */
+public class IoTDBCreateStorageGroupIT {
+  private Statement statement;
+  private Connection connection;
+
+  @Before
+  public void setUp() throws Exception {
+    EnvironmentUtils.envSetUp();
+
+    Class.forName(Config.JDBC_DRIVER_NAME);
+    connection = DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
+    statement = connection.createStatement();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    statement.close();
+    connection.close();
+    EnvironmentUtils.cleanEnv();
+  }
+
+  /** The test creates three storage groups */
+  @Test
+  public void testCreateStorageGroup() throws Exception {
+    String[] storageGroups = {"root.sg1", "root.sg2", "root.sg3"};
+
+    for (String storageGroup : storageGroups) {
+      statement.execute(String.format("create storage group %s", storageGroup));
+    }
+
+    // ensure that current StorageGroup in cache is right.
+    createStorageGroupTool(storageGroups);
+
+    EnvironmentUtils.stopDaemon();
+    setUp();
+
+    // ensure StorageGroup in cache is right after recovering.
+    createStorageGroupTool(storageGroups);
+  }
+
+  private void createStorageGroupTool(String[] storageGroups) throws SQLException {
+    boolean hasResult = statement.execute("show storage group");
+    Assert.assertTrue(hasResult);
+
+    List<String> resultList = new ArrayList<>();
+    try (ResultSet resultSet = statement.getResultSet()) {
+      while (resultSet.next()) {
+        String storageGroupPath = resultSet.getString("storage group");
+        resultList.add(storageGroupPath);
+      }
+    }
+    Assert.assertEquals(3, resultList.size());
+
+    List<String> collect =
+        resultList.stream()
+            .sorted(Comparator.comparingInt(e -> e.split("\\.").length))
+            .collect(Collectors.toList());

Review comment:
       `resultList = resultList.stream().sorted().collect(Collectors.toList());` is ok

##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBCreateStorageGroupIT.java
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.integration;
+
+import org.apache.iotdb.db.utils.EnvironmentUtils;
+import org.apache.iotdb.jdbc.Config;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Notice that, all test begins with "IoTDB" is integration test. All test which will start the
+ * IoTDB server should be defined as integration test.
+ */
+public class IoTDBCreateStorageGroupIT {
+  private Statement statement;
+  private Connection connection;
+
+  @Before
+  public void setUp() throws Exception {
+    EnvironmentUtils.envSetUp();
+
+    Class.forName(Config.JDBC_DRIVER_NAME);
+    connection = DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
+    statement = connection.createStatement();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    statement.close();
+    connection.close();
+    EnvironmentUtils.cleanEnv();
+  }
+
+  /** The test creates three storage groups */
+  @Test
+  public void testCreateStorageGroup() throws Exception {
+    String[] storageGroups = {"root.sg1", "root.sg2", "root.sg3"};
+
+    for (String storageGroup : storageGroups) {
+      statement.execute(String.format("create storage group %s", storageGroup));
+    }
+
+    // ensure that current StorageGroup in cache is right.
+    createStorageGroupTool(storageGroups);
+
+    EnvironmentUtils.stopDaemon();
+    setUp();

Review comment:
       Why a recover? It seems the following test you exectued does not need that.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] Alima777 commented on a change in pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#discussion_r673796170



##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/SqlBase.g4
##########
@@ -110,6 +110,7 @@ statement
       cqSelectIntoClause #createContinuousQueryStatement
     | DROP (CONTINUOUS QUERY | CQ) continuousQueryName=ID #dropContinuousQueryStatement
     | SHOW (CONTINUOUS QUERIES | CQS) #showContinuousQueriesStatement
+    | CREATE STORAGE GROUP prefixPath #createStorageGroup

Review comment:
       Move this next to `SET STORAGE GROUP ...` to put the same type command together.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] ijihang commented on a change in pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
ijihang commented on a change in pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#discussion_r673998997



##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/SqlBase.g4
##########
@@ -110,6 +110,7 @@ statement
       cqSelectIntoClause #createContinuousQueryStatement
     | DROP (CONTINUOUS QUERY | CQ) continuousQueryName=ID #dropContinuousQueryStatement
     | SHOW (CONTINUOUS QUERIES | CQS) #showContinuousQueriesStatement
+    | CREATE STORAGE GROUP prefixPath #createStorageGroup

Review comment:
       Thanks, new commit add integration test and move sql




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] ijihang commented on a change in pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
ijihang commented on a change in pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#discussion_r674451815



##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBCreateStorageGroupIT.java
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.integration;
+
+import org.apache.iotdb.db.utils.EnvironmentUtils;
+import org.apache.iotdb.jdbc.Config;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Notice that, all test begins with "IoTDB" is integration test. All test which will start the
+ * IoTDB server should be defined as integration test.
+ */
+public class IoTDBCreateStorageGroupIT {
+  private Statement statement;
+  private Connection connection;
+
+  @Before
+  public void setUp() throws Exception {
+    EnvironmentUtils.envSetUp();
+
+    Class.forName(Config.JDBC_DRIVER_NAME);
+    connection = DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
+    statement = connection.createStatement();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    statement.close();
+    connection.close();
+    EnvironmentUtils.cleanEnv();
+  }
+
+  /** The test creates three storage groups */
+  @Test
+  public void testCreateStorageGroup() throws Exception {
+    String[] storageGroups = {"root.sg1", "root.sg2", "root.sg3"};
+
+    for (String storageGroup : storageGroups) {
+      statement.execute(String.format("create storage group %s", storageGroup));
+    }
+
+    // ensure that current StorageGroup in cache is right.
+    createStorageGroupTool(storageGroups);
+
+    EnvironmentUtils.stopDaemon();
+    setUp();
+
+    // ensure StorageGroup in cache is right after recovering.
+    createStorageGroupTool(storageGroups);
+  }
+
+  private void createStorageGroupTool(String[] storageGroups) throws SQLException {
+    boolean hasResult = statement.execute("show storage group");
+    Assert.assertTrue(hasResult);
+
+    List<String> resultList = new ArrayList<>();
+    try (ResultSet resultSet = statement.getResultSet()) {
+      while (resultSet.next()) {
+        String storageGroupPath = resultSet.getString("storage group");
+        resultList.add(storageGroupPath);
+      }
+    }
+    Assert.assertEquals(3, resultList.size());
+
+    List<String> collect =
+        resultList.stream()
+            .sorted(Comparator.comparingInt(e -> e.split("\\.").length))
+            .collect(Collectors.toList());

Review comment:
       yes 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] ijihang commented on a change in pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
ijihang commented on a change in pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608#discussion_r674451564



##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBCreateStorageGroupIT.java
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.integration;
+
+import org.apache.iotdb.db.utils.EnvironmentUtils;
+import org.apache.iotdb.jdbc.Config;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Notice that, all test begins with "IoTDB" is integration test. All test which will start the
+ * IoTDB server should be defined as integration test.
+ */
+public class IoTDBCreateStorageGroupIT {
+  private Statement statement;
+  private Connection connection;
+
+  @Before
+  public void setUp() throws Exception {
+    EnvironmentUtils.envSetUp();
+
+    Class.forName(Config.JDBC_DRIVER_NAME);
+    connection = DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
+    statement = connection.createStatement();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    statement.close();
+    connection.close();
+    EnvironmentUtils.cleanEnv();
+  }
+
+  /** The test creates three storage groups */
+  @Test
+  public void testCreateStorageGroup() throws Exception {
+    String[] storageGroups = {"root.sg1", "root.sg2", "root.sg3"};
+
+    for (String storageGroup : storageGroups) {
+      statement.execute(String.format("create storage group %s", storageGroup));
+    }
+
+    // ensure that current StorageGroup in cache is right.
+    createStorageGroupTool(storageGroups);
+
+    EnvironmentUtils.stopDaemon();
+    setUp();

Review comment:
       Test whether the storage group still exists when the iotdb is stopped and then restarted




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] HTHou merged pull request #3608: [IOTDB-959]Add createStorageGroup Grammar

Posted by GitBox <gi...@apache.org>.
HTHou merged pull request #3608:
URL: https://github.com/apache/iotdb/pull/3608


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org