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/28 12:02:07 UTC

[GitHub] [iotdb] JackieTien97 commented on a change in pull request #5356: [IOTDB-2658] Generate logical plan for query statement

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