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/02/25 08:09:20 UTC

[GitHub] [iotdb] LebronAl commented on a change in pull request #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan

LebronAl commented on a change in pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#discussion_r582621629



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -426,58 +428,77 @@ public synchronized boolean containsKey(PartialPath key) {
   }
 
   /**
-   * create storage groups for CreateTimeseriesPlan and InsertPlan, also create timeseries for
-   * InsertPlan. Only the two kind of plans can use this method.
+   * create storage groups for CreateTimeseriesPlan, CreateMultiTimeseriesPlan and InsertPlan, also
+   * create timeseries for InsertPlan. Only the three kind of plans can use this method.
    */
   public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsistencyException {
-    // try to set storage group
-    List<PartialPath> deviceIds;
-    // only handle InsertPlan, CreateTimeSeriesPlan and CreateMultiTimeSeriesPlan currently,
-    if (plan instanceof InsertPlan
-        && !(plan instanceof InsertMultiTabletPlan)
-        && !(plan instanceof InsertRowsPlan)) {
-      // InsertMultiTabletPlan and InsertRowsPlan have multiple devices, and other types of
-      // InsertPlan have only one device.
-      deviceIds = Collections.singletonList(((InsertPlan) plan).getDeviceId());
+    List<PartialPath> storageGroups = new ArrayList<>();
+    // for InsertPlan, try to just use deviceIds to get related storage groups because there's no
+    // need to call getPaths to concat deviceId and sensor as they will gain same result.
+    // for CreateTimeSeriesPlan, use getPath() to get timeseries to get related storage group
+    // for CreateTimeMultiSeriesPlan, use getPaths() to get all timeseries to get related storage
+    // groups

Review comment:
       OK

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -426,58 +428,77 @@ public synchronized boolean containsKey(PartialPath key) {
   }
 
   /**
-   * create storage groups for CreateTimeseriesPlan and InsertPlan, also create timeseries for
-   * InsertPlan. Only the two kind of plans can use this method.
+   * create storage groups for CreateTimeseriesPlan, CreateMultiTimeseriesPlan and InsertPlan, also
+   * create timeseries for InsertPlan. Only the three kind of plans can use this method.
    */
   public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsistencyException {
-    // try to set storage group
-    List<PartialPath> deviceIds;
-    // only handle InsertPlan, CreateTimeSeriesPlan and CreateMultiTimeSeriesPlan currently,
-    if (plan instanceof InsertPlan
-        && !(plan instanceof InsertMultiTabletPlan)
-        && !(plan instanceof InsertRowsPlan)) {
-      // InsertMultiTabletPlan and InsertRowsPlan have multiple devices, and other types of
-      // InsertPlan have only one device.
-      deviceIds = Collections.singletonList(((InsertPlan) plan).getDeviceId());
+    List<PartialPath> storageGroups = new ArrayList<>();
+    // for InsertPlan, try to just use deviceIds to get related storage groups because there's no
+    // need to call getPaths to concat deviceId and sensor as they will gain same result.
+    // for CreateTimeSeriesPlan, use getPath() to get timeseries to get related storage group

Review comment:
       OK




----------------------------------------------------------------
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.

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