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/27 14:30:06 UTC

[GitHub] [iotdb] liuminghui233 opened a new pull request #5356: [IOTDB-2658] Generate logical plan for query statement

liuminghui233 opened a new pull request #5356:
URL: https://github.com/apache/iotdb/pull/5356


   Generate logical plan for query statement ~


-- 
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] JackieTien97 commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);

Review comment:
       Actually I can't get the meaning of `PlanBuilder`, it seems that `PlanNode` is enough, there is no need for `PlanBuilder`.

##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/plan/node/source/SeriesScanNode.java
##########
@@ -99,6 +99,16 @@ public void setDataRegionReplicaSet(DataRegionReplicaSet dataRegion) {
     this.dataRegionReplicaSet = dataRegion;
   }
 
+  @Override
+  public String getDeviceName() {
+    return seriesPath.getDevice();
+  }
+
+  @Override
+  protected String getExpressionString() {
+    return seriesPath.toString();

Review comment:
       Better to use `seriesPath.fullPath()`, because `toString()` method will generate a new `String` object each time, especially you also use it in `hashCode`.




-- 
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] liuminghui233 commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/plan/node/source/SeriesScanNode.java
##########
@@ -99,6 +99,16 @@ public void setDataRegionReplicaSet(DataRegionReplicaSet dataRegion) {
     this.dataRegionReplicaSet = dataRegion;
   }
 
+  @Override
+  public String getDeviceName() {
+    return seriesPath.getDevice();
+  }
+
+  @Override
+  protected String getExpressionString() {
+    return seriesPath.toString();

Review comment:
       fixed




-- 
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] xingtanzjr commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);
+
+      if (queryStatement.getWhereCondition() != null) {
+        planBuilder =
+            planQueryFilter(planBuilder, queryStatement.getWhereCondition().getQueryFilter());
+      }
+
+      if (queryStatement.isGroupByLevel()) {
+        planBuilder =
+            planGroupByLevel(
+                planBuilder,
+                ((AggregationQueryStatement) queryStatement).getGroupByLevelComponent());
+      }
+
+      if (queryStatement instanceof FillQueryStatement) {
+        planBuilder =
+            planFill(planBuilder, ((FillQueryStatement) queryStatement).getFillComponent());
+      }
+
+      planBuilder = planFilterNull(planBuilder, queryStatement.getFilterNullComponent());
+      planBuilder = planSort(planBuilder, queryStatement.getResultOrder());
+      planBuilder = planLimit(planBuilder, queryStatement.getRowLimit());
+      planBuilder = planOffset(planBuilder, queryStatement.getRowOffset());
+      return planBuilder.getRoot();
+    }
+
+    private PlanBuilder planSelectComponent(QueryStatement queryStatement) {

Review comment:
       Sure, let's add a TODO here




-- 
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] xingtanzjr commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);
+
+      if (queryStatement.getWhereCondition() != null) {
+        planBuilder =
+            planQueryFilter(planBuilder, queryStatement.getWhereCondition().getQueryFilter());
+      }
+
+      if (queryStatement.isGroupByLevel()) {
+        planBuilder =
+            planGroupByLevel(
+                planBuilder,
+                ((AggregationQueryStatement) queryStatement).getGroupByLevelComponent());
+      }
+
+      if (queryStatement instanceof FillQueryStatement) {
+        planBuilder =
+            planFill(planBuilder, ((FillQueryStatement) queryStatement).getFillComponent());
+      }
+
+      planBuilder = planFilterNull(planBuilder, queryStatement.getFilterNullComponent());
+      planBuilder = planSort(planBuilder, queryStatement.getResultOrder());
+      planBuilder = planLimit(planBuilder, queryStatement.getRowLimit());
+      planBuilder = planOffset(planBuilder, queryStatement.getRowOffset());
+      return planBuilder.getRoot();
+    }
+
+    private PlanBuilder planSelectComponent(QueryStatement queryStatement) {

Review comment:
       Besides the series in ResultColumn, the related series in QueryFilter should also be considered here.




-- 
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] xingtanzjr commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);
+
+      if (queryStatement.getWhereCondition() != null) {
+        planBuilder =
+            planQueryFilter(planBuilder, queryStatement.getWhereCondition().getQueryFilter());
+      }
+
+      if (queryStatement.isGroupByLevel()) {
+        planBuilder =
+            planGroupByLevel(
+                planBuilder,
+                ((AggregationQueryStatement) queryStatement).getGroupByLevelComponent());
+      }
+
+      if (queryStatement instanceof FillQueryStatement) {
+        planBuilder =
+            planFill(planBuilder, ((FillQueryStatement) queryStatement).getFillComponent());
+      }
+
+      planBuilder = planFilterNull(planBuilder, queryStatement.getFilterNullComponent());
+      planBuilder = planSort(planBuilder, queryStatement.getResultOrder());
+      planBuilder = planLimit(planBuilder, queryStatement.getRowLimit());
+      planBuilder = planOffset(planBuilder, queryStatement.getRowOffset());
+      return planBuilder.getRoot();
+    }
+
+    private PlanBuilder planSelectComponent(QueryStatement queryStatement) {
+      Map<String, Set<SourceNode>> deviceNameToSourceNodesMap = new HashMap<>();
+      Map<String, DataRegionReplicaSet> deviceNameToDataRegionReplicaSetMap = new HashMap<>();
+
+      for (ResultColumn resultColumn : queryStatement.getSelectComponent().getResultColumns()) {
+        Set<SourceNode> sourceNodes = planResultColumn(resultColumn);
+        for (SourceNode sourceNode : sourceNodes) {
+          String deviceName = sourceNode.getDeviceName();
+          deviceNameToSourceNodesMap
+              .computeIfAbsent(deviceName, k -> new HashSet<>())
+              .add(sourceNode);
+          deviceNameToDataRegionReplicaSetMap.computeIfAbsent(
+              deviceName,
+              k -> analysis.getDataPartitionInfo().getDataRegionReplicaSet(k, null).get(0));
+          sourceNode.setDataRegionReplicaSet(deviceNameToDataRegionReplicaSetMap.get(deviceName));

Review comment:
       Is it necessary to set DataRegionReplicaSet to SourceNode here ?
   
   And... If there are several `DataRegionReplicaSet`s here, we only set one of them to the SourceNode, which is not correct




-- 
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] liuminghui233 commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);

Review comment:
       Indeed……if PlanBuilder still is meaningless later, I'll consider removing it.




-- 
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] liuminghui233 commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);
+
+      if (queryStatement.getWhereCondition() != null) {
+        planBuilder =
+            planQueryFilter(planBuilder, queryStatement.getWhereCondition().getQueryFilter());
+      }
+
+      if (queryStatement.isGroupByLevel()) {
+        planBuilder =
+            planGroupByLevel(
+                planBuilder,
+                ((AggregationQueryStatement) queryStatement).getGroupByLevelComponent());
+      }
+
+      if (queryStatement instanceof FillQueryStatement) {
+        planBuilder =
+            planFill(planBuilder, ((FillQueryStatement) queryStatement).getFillComponent());
+      }
+
+      planBuilder = planFilterNull(planBuilder, queryStatement.getFilterNullComponent());
+      planBuilder = planSort(planBuilder, queryStatement.getResultOrder());
+      planBuilder = planLimit(planBuilder, queryStatement.getRowLimit());
+      planBuilder = planOffset(planBuilder, queryStatement.getRowOffset());
+      return planBuilder.getRoot();
+    }
+
+    private PlanBuilder planSelectComponent(QueryStatement queryStatement) {

Review comment:
       I know~ At present, we can only handle fundamental queries and not consider many situations. I will continue to sort out and improve this week.




-- 
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] liuminghui233 commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);
+
+      if (queryStatement.getWhereCondition() != null) {
+        planBuilder =
+            planQueryFilter(planBuilder, queryStatement.getWhereCondition().getQueryFilter());
+      }
+
+      if (queryStatement.isGroupByLevel()) {
+        planBuilder =
+            planGroupByLevel(
+                planBuilder,
+                ((AggregationQueryStatement) queryStatement).getGroupByLevelComponent());
+      }
+
+      if (queryStatement instanceof FillQueryStatement) {
+        planBuilder =
+            planFill(planBuilder, ((FillQueryStatement) queryStatement).getFillComponent());
+      }
+
+      planBuilder = planFilterNull(planBuilder, queryStatement.getFilterNullComponent());
+      planBuilder = planSort(planBuilder, queryStatement.getResultOrder());
+      planBuilder = planLimit(planBuilder, queryStatement.getRowLimit());
+      planBuilder = planOffset(planBuilder, queryStatement.getRowOffset());
+      return planBuilder.getRoot();
+    }
+
+    private PlanBuilder planSelectComponent(QueryStatement queryStatement) {

Review comment:
       added




-- 
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] liuminghui233 commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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



##########
File path: server/src/main/java/org/apache/iotdb/db/mpp/sql/planner/LogicalPlanner.java
##########
@@ -76,8 +84,155 @@ public LogicalPlanVisitor(Analysis analysis) {
 
     @Override
     public PlanNode visitQuery(QueryStatement queryStatement, MPPQueryContext context) {
-      // TODO: Generate logical planNode tree for query statement
-      return null;
+      PlanBuilder planBuilder = planSelectComponent(queryStatement);
+
+      if (queryStatement.getWhereCondition() != null) {
+        planBuilder =
+            planQueryFilter(planBuilder, queryStatement.getWhereCondition().getQueryFilter());
+      }
+
+      if (queryStatement.isGroupByLevel()) {
+        planBuilder =
+            planGroupByLevel(
+                planBuilder,
+                ((AggregationQueryStatement) queryStatement).getGroupByLevelComponent());
+      }
+
+      if (queryStatement instanceof FillQueryStatement) {
+        planBuilder =
+            planFill(planBuilder, ((FillQueryStatement) queryStatement).getFillComponent());
+      }
+
+      planBuilder = planFilterNull(planBuilder, queryStatement.getFilterNullComponent());
+      planBuilder = planSort(planBuilder, queryStatement.getResultOrder());
+      planBuilder = planLimit(planBuilder, queryStatement.getRowLimit());
+      planBuilder = planOffset(planBuilder, queryStatement.getRowOffset());
+      return planBuilder.getRoot();
+    }
+
+    private PlanBuilder planSelectComponent(QueryStatement queryStatement) {
+      Map<String, Set<SourceNode>> deviceNameToSourceNodesMap = new HashMap<>();
+      Map<String, DataRegionReplicaSet> deviceNameToDataRegionReplicaSetMap = new HashMap<>();
+
+      for (ResultColumn resultColumn : queryStatement.getSelectComponent().getResultColumns()) {
+        Set<SourceNode> sourceNodes = planResultColumn(resultColumn);
+        for (SourceNode sourceNode : sourceNodes) {
+          String deviceName = sourceNode.getDeviceName();
+          deviceNameToSourceNodesMap
+              .computeIfAbsent(deviceName, k -> new HashSet<>())
+              .add(sourceNode);
+          deviceNameToDataRegionReplicaSetMap.computeIfAbsent(
+              deviceName,
+              k -> analysis.getDataPartitionInfo().getDataRegionReplicaSet(k, null).get(0));
+          sourceNode.setDataRegionReplicaSet(deviceNameToDataRegionReplicaSetMap.get(deviceName));

Review comment:
       Got it ~ I will remove it




-- 
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] xingtanzjr merged pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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


   


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