[GitHub] [incubator-iotdb] Alima777 commented on a change in pull request #1650: [IOTDB-848] support order by time asc/desc

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

Alima777 commented on a change in pull request #1650:
URL: https://github.com/apache/incubator-iotdb/pull/1650#discussion_r480882930



##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/strategy/SqlBase.g4
##########
@@ -208,12 +208,13 @@ fromClause
     ;
 
 specialClause
-    : specialLimit
-    | groupByTimeClause specialLimit?
-    | groupByFillClause specialLimit?
+    : specialLimit? orderByTimeClause?
+    | groupByTimeClause specialLimit? orderByTimeClause?
+    | groupByFillClause specialLimit? orderByTimeClause?
+    | orderByTimeClause? specialLimit?

Review comment:
       ```suggestion
       | orderByTimeClause specialLimit?
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/LogicalGenerator.java
##########
@@ -1045,6 +1046,18 @@ public void enterOffsetClause(OffsetClauseContext ctx) {
     }
   }
 
+  @Override
+  public void enterOrderByTimeClause(OrderByTimeClauseContext ctx) {
+    super.enterOrderByTimeClause(ctx);
+    queryOp.setColumn(ctx.TIME().getText());
+    if (ctx.ASC() == null) {
+      queryOp.setAscending(false);
+    }
+    if (ctx.DESC() == null) {

Review comment:
       It could be omitted, keeping default value.

##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/strategy/SqlBase.g4
##########
@@ -208,12 +208,13 @@ fromClause
     ;
 
 specialClause
-    : specialLimit
-    | groupByTimeClause specialLimit?
-    | groupByFillClause specialLimit?
+    : specialLimit? orderByTimeClause?

Review comment:
       the first Clause should not be with `?`

##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/query/timegenerator/TimeGenerator.java
##########
@@ -107,4 +107,8 @@ protected abstract IBatchReader generateNewBatchReader(SingleSeriesExpression ex
   public boolean hasOrNode() {
     return hasOrNode;
   }
+
+  protected boolean isAscending() {
+    return true;

Review comment:
       What about an abstract method here?

##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/RawQueryDataSetWithValueFilter.java
##########
@@ -43,10 +43,11 @@
    * @param dataTypes     time series data type
    * @param timeGenerator EngineTimeGenerator object
    * @param readers       readers in List(IReaderByTimeStamp) structure
+   * @param ascending

Review comment:
       Add some comments?




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