[GitHub] [iotdb] neuyilan commented on a change in pull request #3240: support “all” keyword when execute fill query

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

neuyilan commented on a change in pull request #3240:
URL: https://github.com/apache/iotdb/pull/3240#discussion_r636584847



##########
File path: server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java
##########
@@ -1351,7 +1351,39 @@ public void parseFillClause(FillClauseContext ctx, QueryOperator queryOp) {
     List<TypeClauseContext> list = ctx.typeClause();
     Map<TSDataType, IFill> fillTypes = new EnumMap<>(TSDataType.class);
     for (TypeClauseContext typeClause : list) {
-      parseTypeClause(typeClause, fillTypes);
+      if (typeClause.ALL() != null) {
+        if (typeClause.linearClause() != null) {
+          throw new SQLParserException("fill all doesn't support linear fill");
+        }
+        IFill fill;
+        if (typeClause.previousUntilLastClause() != null) {
+          long preRange;
+          if (typeClause.previousUntilLastClause().DURATION() != null) {
+            preRange =
+                DatetimeUtils.convertDurationStrToLong(
+                    typeClause.previousUntilLastClause().DURATION().getText());
+          } else {
+            preRange = IoTDBDescriptor.getInstance().getConfig().getDefaultFillInterval();
+          }
+          fill = new PreviousFill(preRange, true);
+        } else {
+          long preRange;
+          if (typeClause.previousClause().DURATION() != null) {
+            preRange =
+                DatetimeUtils.convertDurationStrToLong(
+                    typeClause.previousClause().DURATION().getText());
+          } else {
+            preRange = IoTDBDescriptor.getInstance().getConfig().getDefaultFillInterval();
+          }
+          fill = new PreviousFill(preRange);
+        }
+        for (TSDataType tsDataType : TSDataType.values()) {
+          fillTypes.put(tsDataType, fill.copy());
+        }
+        break;
+      } else {
+        parseTypeClause(typeClause, fillTypes);
+      }

Review comment:
       LGTM, but in terms of code readability, I have the following suggestions:
   In fact, All the code in this section is used for `parseTypeClause()`, so it may be better to extract a public method called `parseTypeClause()`. For type all, it can be extracted as a `private parseAllTypeClause ()`, The origin `parseTypeClause()` method  can be changed to private `parsePrimitiveTypeClause()`.
   What's your opinion?
   




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