[GitHub] [iotdb] SilverNarcissus commented on a change in pull request #4326: [IOTDB-1883] Extension of schema template to tree-structured

[GitBox <gi...@apache.org>]

SilverNarcissus commented on a change in pull request #4326:
URL: https://github.com/apache/iotdb/pull/4326#discussion_r744396601



##########
File path: thrift/src/main/thrift/rpc.thrift
##########
@@ -366,9 +366,41 @@ struct TSCreateSchemaTemplateReq {
   4: required list<list<string>> measurements
   5: required list<list<i32>> dataTypes
   6: required list<list<i32>> encodings
+  7: required list<list<i32>> compressors

Review comment:
       Please sync rpc-changelist.md

##########
File path: docs/UserGuide/API/Programming-Java-Native-API.md
##########
@@ -402,36 +401,152 @@ Open a session and specifies whether the Leader cache is enabled. Note that this
 * encodings: List of encodings, if it is a single measurement, just put it's encoding into
 *     a list and add to encodings if it is a vector measurement, put all encodings of the
 *     vector into a list and add to encodings
-* compressors: List of compressors                            
+* compressors: List of compressors, if it is a single measurement, just put it's 
+*     compressor into a list and add to encodings if it is a vector measurement, put all 
+*     encodings of the vector into a list and add to encodings
 void createSchemaTemplate(
       String templateName,
-      List<String> schemaName,
       List<List<String>> measurements,
       List<List<TSDataType>> dataTypes,
       List<List<TSEncoding>> encodings,
       List<CompressionType> compressors)
 ```
 
-Create a measurement template, the param description at above
+Create a measurement template, the param description as above. As measurement templates could be tree-structured, parameter 'measurement' in above method could be a path to the measurement, be like: 'vehicle.GPS.x' . 
 
-``` 
+You can also create instances of Template, InternalNode and MeasurementNode to depict the structure of the template, and use belowed interface to create it.
 
-void setSchemaTemplate(String templateName, String prefixPath)
+```java
+public void createSchemaTemplate(Template template);
+
+Class Template {
+    private String name;
+    private boolean directShareTime;
+    Map<String, Node> children;
+    public Template(String name, boolean isShareTime);
+    
+    public void addToTemplate(Node node);
+    public void deleteFromTemplate(String name);
+    public void setShareTime(boolean shareTime);
+}
+
+Abstract Class Node {
+    private String name;
+    public void addChild(Node node);
+    public void deleteChild(Node node);
+}
+
+Class InternalNode extends Node {
+    boolean shareTime;
+    Map<String, Node> children;
+    public void setShareTime(boolean shareTime);
+    public InternalNode(String name, boolean isShareTime);
+}
 
+Class MeasurementNode extends Node {
+    TSDataType dataType;
+    TSEncoding encoding;
+    CompressionType compressor;
+    public MeasurementNode(String name, 
+                           TSDataType dataType, 
+                           TSEncoding encoding,
+                          CompressionType compressor);
+}
 ```
 
-Set the measurement template named 'templateName' at path 'prefixPath'. You should firstly create the template using
+A snippet of using above Method and Class：
+
+```java
+MeasurementNode nodeX = new MeasurementNode("x", TSDataType.FLOAT, TSEncoding.RLE, CompressionType.SNAPPY);
+MeasurementNode nodeY = new MeasurementNode("y", TSDataType.FLOAT, TSEncoding.RLE, CompressionType.SNAPPY);
+MeasurementNode nodeSpeed = new MeasurementNode("speed", TSDataType.DOUBLE, TSEncoding.GORILLA, CompressionType.SNAPPY);
 
+InternalNode internalGPS = new InternalNode("GPS", true);
+InternalNode internalVehicle = new InternalNode("vehicle", false);
+
+internalGPS.addChild(nodeX);
+internalGPS.addChild(nodeY);
+internalVehicle.addChild(GPS);
+internalVehicle.addChild(nodeSpeed);
+
+Template template = new Template("treeTemplateExample");
+template.addToTemplate(internalGPS);
+template.addToTemplate(internalVehicle);
+template.addToTemplate(nodeSpeed);
+
+createSchemaTemplate(template);
 ```
 
-void createSchemaTemplate
+After measurement template created, you can edit the template with belowed APIs.
+
+**Attention: templates had been set could not be pruned.**
+
+```java
+// Add aligned measurements to a template
+public void addAlignedMeasurementsInTemplate(String templateName,
+    						  String[] measurementsPath,
+                              TSDataType[] dataTypes,
+                              TSEncoding[] encodings,
+                              CompressionType[] compressors);
+
+// Add one alinged measurement to a template

Review comment:
       (1) typo: 'alinged'
   (2) we really need add one aligned measurement?

##########
File path: server/src/test/java/org/apache/iotdb/db/metadata/MManagerBasicTest.java
##########
@@ -904,12 +908,91 @@ public void testTemplate() throws MetadataException {
     }
   }
 
+  @Test
+  public void testTemplateInnerTree() {
+    CreateTemplatePlan plan = getTreeTemplatePlan();
+    Template template;
+    MManager manager = IoTDB.metaManager;
+
+    try {
+      manager.createSchemaTemplate(plan);
+      template = manager.getTemplate("treeTemplate");
+      assertEquals(4, template.getMeasurementsCount());
+      assertEquals("d1", template.getPathNodeInTemplate("d1").getName());
+      assertEquals(null, template.getPathNodeInTemplate("notExists"));
+      assertEquals("[GPS]", template.getAllAlignedPrefix().toString());
+
+      String[] alignedMeasurements = {"to.be.prefix.s1", "to.be.prefix.s2"};
+      TSDataType[] dataTypes = {TSDataType.INT32, TSDataType.INT32};
+      TSEncoding[] encodings = {TSEncoding.RLE, TSEncoding.RLE};
+      CompressionType[] compressionTypes = {CompressionType.SNAPPY, CompressionType.SNAPPY};
+      template.addAlignedMeasurements(alignedMeasurements, dataTypes, encodings, compressionTypes);
+
+      assertEquals("[GPS, to.be.prefix]", template.getAllAlignedPrefix().toString());
+      assertEquals("[s1, s2]", template.getAlignedMeasurements("to.be.prefix").toString());
+
+      template.deleteAlignedPrefix("to.be.prefix");
+
+      assertEquals("[GPS]", template.getAllAlignedPrefix().toString());
+      assertEquals(null, template.getDirectNode("prefix"));
+      assertEquals("to", template.getDirectNode("to").getName());
+
+      try {
+        template.deleteMeasurements("a.single");

Review comment:
       we should add a fail() after this line

##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/template/Template.java
##########
@@ -180,6 +208,442 @@ public String getMeasurementNodeName(String measurementName) {
     return res;
   }
 
+  // region construct template tree
+  /** Construct aligned measurements, checks prefix equality, path duplication and conflict */
+  private void constructTemplateTree(String[] alignedPaths, IMeasurementSchema[] schemas)

Review comment:
       This method contains too much if else.  Could you simplify it? Or you can add some comments for each path
   suggestion: exception case should as front of the method as they can

##########
File path: session/src/test/java/org/apache/iotdb/session/SessionTest.java
##########
@@ -205,6 +210,118 @@ public void setTimeout() throws StatementExecutionException {
     Assert.assertEquals(60000, session.getQueryTimeout());
   }
 
+  @Test
+  public void createSchemaTemplateWithTreeStructure()
+      throws IoTDBConnectionException, StatementExecutionException, IOException {
+    session = new Session("127.0.0.1", 6667, "root", "root", ZoneId.of("+05:00"));
+    session.open();
+
+    Template template = new Template("treeTemplate", true);
+    Node iNodeGPS = new InternalNode("GPS", false);
+    Node iNodeV = new InternalNode("vehicle", true);
+    Node mNodeX =
+        new MeasurementNode("x", TSDataType.FLOAT, TSEncoding.RLE, CompressionType.SNAPPY);
+    Node mNodeY =
+        new MeasurementNode("y", TSDataType.FLOAT, TSEncoding.RLE, CompressionType.SNAPPY);
+
+    template.addToTemplate(mNodeX);
+    iNodeGPS.addChild(mNodeX);
+    iNodeGPS.addChild(mNodeY);
+    iNodeV.addChild(mNodeX);
+    iNodeV.addChild(iNodeGPS);
+    iNodeV.addChild(mNodeY);
+    template.addToTemplate(iNodeGPS);
+    template.addToTemplate(iNodeV);
+    template.addToTemplate(mNodeY);
+
+    session.createSchemaTemplate(template);
+    assertEquals(
+        "[vehicle.GPS.y, x, vehicle.GPS.x, y, GPS.x, vehicle.x, GPS.y, vehicle.y]",
+        session.showMeasurementsInTemplate("treeTemplate").toString());
+    assertEquals(8, session.countMeasurementsInTemplate("treeTemplate"));

Review comment:
       Maybe we should add some insert and select test after change the template




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