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 2022/03/31 08:13:48 UTC

[GitHub] [iotdb] JackieTien97 commented on a change in pull request #5355: [IOTDB-2748] Writing statement and writing process of coordinator

JackieTien97 commented on a change in pull request #5355:
URL: https://github.com/apache/iotdb/pull/5355#discussion_r839277114



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/common/schematree/PathPatternTree.java
##########
@@ -42,6 +44,20 @@
    */
   protected boolean isPrefixMatchPath;
 
+  public PathPatternTree(PartialPath deivcePath, String[] measurements) {
+    // TODO
+    this.root = new PathPatternNode(SQLConstant.ROOT);
+    this.pathList = new ArrayList<>();
+  };
+
+  public PathPatternTree(Map<PartialPath, List<String>> devices) {
+    // TODO
+    this.root = new PathPatternNode(SQLConstant.ROOT);
+    this.pathList = new ArrayList<>();
+  };
+
+  public void serialize(OutputStream baos) throws IOException {}

Review comment:
       We've already got a `serialize(ByteBuffer buffer)` method, why adding this new one?

##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/analyze/Analyzer.java
##########
@@ -196,27 +193,66 @@ public Analysis visitAlterTimeseries(
     @Override
     public Analysis visitInsertTablet(
         InsertTabletStatement insertTabletStatement, MPPQueryContext context) {
-      // TODO(INSERT) device + time range -> PartitionInfo
       DataPartitionQueryParam dataPartitionQueryParam = new DataPartitionQueryParam();
       dataPartitionQueryParam.setDeviceId(insertTabletStatement.getDevicePath().getFullPath());
-      // TODO(INSERT) calculate the time partition id list
-      //      dataPartitionQueryParam.setTimePartitionIdList();
+      dataPartitionQueryParam.setTimePartitionIdList(insertTabletStatement.getTimePartitionIds());
       PartitionInfo partitionInfo = partitionFetcher.fetchPartitionInfo(dataPartitionQueryParam);
 
-      // TODO(INSERT) get each time series schema according to SchemaPartitionInfo in PartitionInfo
-      PathPatternTree patternTree = new PathPatternTree();
-      patternTree.appendPaths(
-          insertTabletStatement.getDevicePath(),
-          Arrays.asList(insertTabletStatement.getMeasurements()));
-      SchemaTree schemaTree = schemaFetcher.fetchSchema(patternTree);
+      SchemaTree schemaTree =
+          IoTDBDescriptor.getInstance().getConfig().isAutoCreateSchemaEnabled()
+              ? schemaFetcher.fetchSchemaWithAutoCreate(

Review comment:
       You should pass the data type if auto created enabled because we need to create timeseries using the passing data type.

##########
File path: node-commons/src/main/java/org/apache/iotdb/commons/partition/DataPartitionInfo.java
##########
@@ -50,6 +48,18 @@ public void setDataPartitionMap(
         .collect(Collectors.toList());
   }
 
+  public List<DataRegionReplicaSet> getDataRegionReplicaSetForWriting(

Review comment:
       Add more java doc for these two methods to indicate that they will return the latest dataRegionReplicaSet for each time partition which is different from querying.

##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/analyze/Analyzer.java
##########
@@ -196,27 +193,66 @@ public Analysis visitAlterTimeseries(
     @Override
     public Analysis visitInsertTablet(
         InsertTabletStatement insertTabletStatement, MPPQueryContext context) {
-      // TODO(INSERT) device + time range -> PartitionInfo
       DataPartitionQueryParam dataPartitionQueryParam = new DataPartitionQueryParam();
       dataPartitionQueryParam.setDeviceId(insertTabletStatement.getDevicePath().getFullPath());
-      // TODO(INSERT) calculate the time partition id list
-      //      dataPartitionQueryParam.setTimePartitionIdList();
+      dataPartitionQueryParam.setTimePartitionIdList(insertTabletStatement.getTimePartitionIds());
       PartitionInfo partitionInfo = partitionFetcher.fetchPartitionInfo(dataPartitionQueryParam);
 
-      // TODO(INSERT) get each time series schema according to SchemaPartitionInfo in PartitionInfo
-      PathPatternTree patternTree = new PathPatternTree();
-      patternTree.appendPaths(
-          insertTabletStatement.getDevicePath(),
-          Arrays.asList(insertTabletStatement.getMeasurements()));
-      SchemaTree schemaTree = schemaFetcher.fetchSchema(patternTree);
+      SchemaTree schemaTree =
+          IoTDBDescriptor.getInstance().getConfig().isAutoCreateSchemaEnabled()
+              ? schemaFetcher.fetchSchemaWithAutoCreate(
+                  new PathPatternTree(
+                      insertTabletStatement.getDevicePath(),
+                      insertTabletStatement.getMeasurements()))

Review comment:
       I think there is no need to wrap it as a `PathPatternTree`, directly pass one `deviceId` and `List<String> measurements` and `List<TSDataType>`.

##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/analyze/Analyzer.java
##########
@@ -196,27 +193,66 @@ public Analysis visitAlterTimeseries(
     @Override
     public Analysis visitInsertTablet(
         InsertTabletStatement insertTabletStatement, MPPQueryContext context) {
-      // TODO(INSERT) device + time range -> PartitionInfo
       DataPartitionQueryParam dataPartitionQueryParam = new DataPartitionQueryParam();
       dataPartitionQueryParam.setDeviceId(insertTabletStatement.getDevicePath().getFullPath());
-      // TODO(INSERT) calculate the time partition id list
-      //      dataPartitionQueryParam.setTimePartitionIdList();
+      dataPartitionQueryParam.setTimePartitionIdList(insertTabletStatement.getTimePartitionIds());
       PartitionInfo partitionInfo = partitionFetcher.fetchPartitionInfo(dataPartitionQueryParam);
 
-      // TODO(INSERT) get each time series schema according to SchemaPartitionInfo in PartitionInfo
-      PathPatternTree patternTree = new PathPatternTree();
-      patternTree.appendPaths(
-          insertTabletStatement.getDevicePath(),
-          Arrays.asList(insertTabletStatement.getMeasurements()));
-      SchemaTree schemaTree = schemaFetcher.fetchSchema(patternTree);
+      SchemaTree schemaTree =
+          IoTDBDescriptor.getInstance().getConfig().isAutoCreateSchemaEnabled()
+              ? schemaFetcher.fetchSchemaWithAutoCreate(
+                  new PathPatternTree(
+                      insertTabletStatement.getDevicePath(),
+                      insertTabletStatement.getMeasurements()))
+              : schemaFetcher.fetchSchema(
+                  new PathPatternTree(
+                      insertTabletStatement.getDevicePath(),
+                      insertTabletStatement.getMeasurements()));
 
       Analysis analysis = new Analysis();
       analysis.setSchemaTree(schemaTree);
-      // TODO(INSERT) do type check here
+
+      if (!insertTabletStatement.checkDataType(schemaTree)) {
+        throw new SemanticException("Data type mismatch");
+      }
       analysis.setStatement(insertTabletStatement);
       analysis.setDataPartitionInfo(partitionInfo.getDataPartitionInfo());
       analysis.setSchemaPartitionInfo(partitionInfo.getSchemaPartitionInfo());
       return analysis;
     }
+
+    public Analysis visitInsertRow(InsertRowStatement insertRowStatement, MPPQueryContext context) {
+      DataPartitionQueryParam dataPartitionQueryParam = new DataPartitionQueryParam();
+      dataPartitionQueryParam.setDeviceId(insertRowStatement.getDevicePath().getFullPath());
+      dataPartitionQueryParam.setTimePartitionIdList(insertRowStatement.getTimePartitionIds());
+      PartitionInfo partitionInfo = partitionFetcher.fetchPartitionInfo(dataPartitionQueryParam);
+
+      SchemaTree schemaTree =
+          IoTDBDescriptor.getInstance().getConfig().isAutoCreateSchemaEnabled()
+              ? schemaFetcher.fetchSchemaWithAutoCreate(
+                  new PathPatternTree(
+                      insertRowStatement.getDevicePath(), insertRowStatement.getMeasurements()))

Review comment:
       same as above




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