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 2021/06/25 07:00:51 UTC

[GitHub] [iotdb] ijihang opened a new pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

ijihang opened a new pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452


   jira:https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-1453?filter=allissues
   Test Query statement is:select * from root.ln.wf01.wt01 where time <5 and time > 10
   cluster version result set is:
   ![image](https://user-images.githubusercontent.com/55303647/123383187-7568ad00-d5c5-11eb-97ca-78e52193fcd4.png)
   server version result set is:
   ![image](https://user-images.githubusercontent.com/55303647/123383424-b6f95800-d5c5-11eb-9d57-25388d546db0.png)
   
   I think the cluster result set is true.


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



[GitHub] [iotdb] coveralls edited a comment on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868301377


   
   [![Coverage Status](https://coveralls.io/builds/40924721/badge)](https://coveralls.io/builds/40924721)
   
   Coverage decreased (-0.4%) to 67.656% when pulling **15837556051250a9f92ee41e22e19522a6e6d3ae on ijihang:fixSingleError** into **7ac39ef139e590b566acc60643bd2e29c0cb4bee on apache:master**.
   


-- 
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] coveralls edited a comment on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868301377


   
   [![Coverage Status](https://coveralls.io/builds/40924650/badge)](https://coveralls.io/builds/40924650)
   
   Coverage decreased (-0.4%) to 67.668% when pulling **15837556051250a9f92ee41e22e19522a6e6d3ae on ijihang:fixSingleError** into **7ac39ef139e590b566acc60643bd2e29c0cb4bee on apache:master**.
   


-- 
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] neuyilan commented on a change in pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

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



##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+

Review comment:
       Is those code duplicated with the `org.apache.iotdb.cluster.utils.PartitionUtils.Intervals` ? if yes, why repeat coding in two places?
   Don't repeat yourself.

##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+
+  public static Intervals extractTimeInterval(Filter filter) {
+    if (filter == null) {
+      return Intervals.ALL_INTERVAL;
+    }
+    // and, or, not, value, time, group by
+    // eq, neq, gt, gteq, lt, lteq, in
+    if (filter instanceof AndFilter) {
+      AndFilter andFilter = ((AndFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(andFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(andFilter.getRight());
+      return leftIntervals.intersection(rightIntervals);
+    } else if (filter instanceof OrFilter) {
+      OrFilter orFilter = ((OrFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(orFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(orFilter.getRight());
+      return leftIntervals.union(rightIntervals);
+    } else if (filter instanceof NotFilter) {
+      NotFilter notFilter = ((NotFilter) filter);
+      return extractTimeInterval(notFilter.getFilter()).not();
+    } else if (filter instanceof TimeFilter.TimeGt) {
+      TimeFilter.TimeGt timeGt = ((TimeFilter.TimeGt) filter);
+      return new Intervals(((long) timeGt.getValue()) + 1, Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeGtEq) {
+      TimeFilter.TimeGtEq timeGtEq = ((TimeFilter.TimeGtEq) filter);
+      return new Intervals(((long) timeGtEq.getValue()), Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeEq) {
+      TimeFilter.TimeEq timeEq = ((TimeFilter.TimeEq) filter);
+      return new Intervals(((long) timeEq.getValue()), ((long) timeEq.getValue()));
+    } else if (filter instanceof TimeFilter.TimeNotEq) {
+      TimeFilter.TimeNotEq timeNotEq = ((TimeFilter.TimeNotEq) filter);
+      Intervals intervals = new Intervals();
+      intervals.addInterval(Long.MIN_VALUE, (long) timeNotEq.getValue() - 1);
+      intervals.addInterval((long) timeNotEq.getValue() + 1, Long.MAX_VALUE);
+      return intervals;
+    } else if (filter instanceof TimeFilter.TimeLt) {
+      TimeFilter.TimeLt timeLt = ((TimeFilter.TimeLt) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLt.getValue() - 1);
+    } else if (filter instanceof TimeFilter.TimeLtEq) {
+      TimeFilter.TimeLtEq timeLtEq = ((TimeFilter.TimeLtEq) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLtEq.getValue());
+    } else if (filter instanceof TimeFilter.TimeIn) {
+      TimeFilter.TimeIn timeIn = ((TimeFilter.TimeIn) filter);
+      Intervals intervals = new Intervals();
+      for (Object value : timeIn.getValues()) {
+        long time = ((long) value);
+        intervals.addInterval(time, time);
+      }
+      return intervals;
+    } else if (filter instanceof GroupByFilter) {
+      GroupByFilter groupByFilter = ((GroupByFilter) filter);
+      return new Intervals(groupByFilter.getStartTime(), groupByFilter.getEndTime() + 1);
+    }
+    // value filter
+    return Intervals.ALL_INTERVAL;
+  }
+
+  /** All intervals are closed. */
+  public static class Intervals extends ArrayList<Long> {
+
+    static final Intervals ALL_INTERVAL = new Intervals(Long.MIN_VALUE, Long.MAX_VALUE);
+
+    public Intervals() {
+      super();
+    }
+
+    Intervals(long lowerBound, long upperBound) {
+      super();
+      addInterval(lowerBound, upperBound);
+    }
+
+    public int getIntervalSize() {
+      return size() / 2;
+    }
+
+    public long getLowerBound(int index) {
+      return get(index * 2);
+    }
+
+    public long getUpperBound(int index) {
+      return get(index * 2 + 1);
+    }
+
+    void setLowerBound(int index, long lb) {
+      set(index * 2, lb);
+    }
+
+    void setUpperBound(int index, long ub) {
+      set(index * 2 + 1, ub);
+    }
+
+    public void addInterval(long lowerBound, long upperBound) {
+      add(lowerBound);
+      add(upperBound);
+    }
+
+    Intervals intersection(Intervals that) {
+      Intervals result = new Intervals();
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      for (int i = 0; i < thisSize; i++) {
+        for (int j = 0; j < thatSize; j++) {
+          long thisLB = this.getLowerBound(i);
+          long thisUB = this.getUpperBound(i);
+          long thatLB = that.getLowerBound(i);
+          long thatUB = that.getUpperBound(i);
+          if (thisUB >= thatLB) {
+            if (thisUB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thisUB);
+            } else if (thisLB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thatUB);
+            }
+          }
+        }
+      }
+      return result;
+    }
+
+    /**
+     * The union is implemented by merge, so the two intervals must be ordered.
+     *
+     * @param that
+     * @return
+     */
+    Intervals union(Intervals that) {
+      if (this.isEmpty()) {
+        return that;
+      } else if (that.isEmpty()) {
+        return this;
+      }
+      Intervals result = new Intervals();
+
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      int thisIndex = 0;
+      int thatIndex = 0;
+      // merge the heads of the two intervals
+      while (thisIndex < thisSize && thatIndex < thatSize) {
+        long thisLB = this.getLowerBound(thisIndex);
+        long thisUB = this.getUpperBound(thisIndex);
+        long thatLB = that.getLowerBound(thatIndex);
+        long thatUB = that.getUpperBound(thatIndex);
+        if (thisLB <= thatLB) {
+          result.mergeLast(thisLB, thisUB);
+          thisIndex++;
+        } else {
+          result.mergeLast(thatLB, thatUB);
+          thatIndex++;
+        }
+      }
+      // merge the remaining intervals
+      Intervals remainingIntervals = thisIndex < thisSize ? this : that;
+      int remainingIndex = thisIndex < thisSize ? thisIndex : thatIndex;
+      mergeRemainingIntervals(remainingIndex, remainingIntervals, result);
+
+      return result;
+    }
+
+    private void mergeRemainingIntervals(
+        int remainingIndex, Intervals remainingIntervals, Intervals result) {
+      for (int i = remainingIndex; i < remainingIntervals.getIntervalSize(); i++) {
+        long lb = remainingIntervals.getLowerBound(i);
+        long ub = remainingIntervals.getUpperBound(i);
+        result.mergeLast(lb, ub);
+      }
+    }
+
+    /**
+     * Merge an interval of [lowerBound, upperBound] with the last interval if they can be merged,
+     * or just add it as the last interval if its lowerBound is larger than the upperBound of the
+     * last interval. If the upperBound of the new interval is less than the lowerBound of the last
+     * interval, nothing will be done.
+     *
+     * @param lowerBound
+     * @param upperBound
+     */
+    private void mergeLast(long lowerBound, long upperBound) {
+      if (getIntervalSize() == 0) {
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      int lastIndex = getIntervalSize() - 1;
+      long lastLB = getLowerBound(lastIndex);
+      long lastUB = getUpperBound(lastIndex);
+      if (lowerBound > lastUB + 1) {
+        // e.g., last [3, 5], new [7, 10], just add the new interval
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      if (upperBound < lastLB - 1) {
+        // e.g., last [7, 10], new [3, 5], do nothing
+        return;
+      }
+      // merge the new interval into the last one
+      setLowerBound(lastIndex, Math.min(lastLB, lowerBound));
+      setUpperBound(lastIndex, Math.max(lastUB, upperBound));
+    }
+
+    public Intervals not() {
+      if (isEmpty()) {
+        return ALL_INTERVAL;
+      }
+      Intervals result = new Intervals();
+      long firstLB = getLowerBound(0);
+      if (firstLB != Long.MIN_VALUE) {
+        result.addInterval(Long.MIN_VALUE, firstLB - 1);
+      }
+
+      int intervalSize = getIntervalSize();
+      for (int i = 0; i < intervalSize - 1; i++) {
+        long currentUB = getUpperBound(i);
+        long nextLB = getLowerBound(i + 1);
+        if (currentUB + 1 <= nextLB - 1) {
+          result.addInterval(currentUB + 1, nextLB - 1);
+        }
+      }
+
+      long lastUB = getUpperBound(result.getIntervalSize() - 1);
+      if (lastUB != Long.MAX_VALUE) {
+        result.addInterval(lastUB + 1, Long.MAX_VALUE);
+      }
+      return result;
+    }
+  }

Review comment:
       Is those code duplicated with the org.apache.iotdb.cluster.utils.PartitionUtils.Intervals ? if yes, why repeat coding in two places?
   Don't repeat yourself.

##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+
+  public static Intervals extractTimeInterval(Filter filter) {
+    if (filter == null) {
+      return Intervals.ALL_INTERVAL;
+    }
+    // and, or, not, value, time, group by
+    // eq, neq, gt, gteq, lt, lteq, in
+    if (filter instanceof AndFilter) {
+      AndFilter andFilter = ((AndFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(andFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(andFilter.getRight());
+      return leftIntervals.intersection(rightIntervals);
+    } else if (filter instanceof OrFilter) {
+      OrFilter orFilter = ((OrFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(orFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(orFilter.getRight());
+      return leftIntervals.union(rightIntervals);
+    } else if (filter instanceof NotFilter) {
+      NotFilter notFilter = ((NotFilter) filter);
+      return extractTimeInterval(notFilter.getFilter()).not();
+    } else if (filter instanceof TimeFilter.TimeGt) {
+      TimeFilter.TimeGt timeGt = ((TimeFilter.TimeGt) filter);
+      return new Intervals(((long) timeGt.getValue()) + 1, Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeGtEq) {
+      TimeFilter.TimeGtEq timeGtEq = ((TimeFilter.TimeGtEq) filter);
+      return new Intervals(((long) timeGtEq.getValue()), Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeEq) {
+      TimeFilter.TimeEq timeEq = ((TimeFilter.TimeEq) filter);
+      return new Intervals(((long) timeEq.getValue()), ((long) timeEq.getValue()));
+    } else if (filter instanceof TimeFilter.TimeNotEq) {
+      TimeFilter.TimeNotEq timeNotEq = ((TimeFilter.TimeNotEq) filter);
+      Intervals intervals = new Intervals();
+      intervals.addInterval(Long.MIN_VALUE, (long) timeNotEq.getValue() - 1);
+      intervals.addInterval((long) timeNotEq.getValue() + 1, Long.MAX_VALUE);
+      return intervals;
+    } else if (filter instanceof TimeFilter.TimeLt) {
+      TimeFilter.TimeLt timeLt = ((TimeFilter.TimeLt) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLt.getValue() - 1);
+    } else if (filter instanceof TimeFilter.TimeLtEq) {
+      TimeFilter.TimeLtEq timeLtEq = ((TimeFilter.TimeLtEq) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLtEq.getValue());
+    } else if (filter instanceof TimeFilter.TimeIn) {
+      TimeFilter.TimeIn timeIn = ((TimeFilter.TimeIn) filter);
+      Intervals intervals = new Intervals();
+      for (Object value : timeIn.getValues()) {
+        long time = ((long) value);
+        intervals.addInterval(time, time);
+      }
+      return intervals;
+    } else if (filter instanceof GroupByFilter) {
+      GroupByFilter groupByFilter = ((GroupByFilter) filter);
+      return new Intervals(groupByFilter.getStartTime(), groupByFilter.getEndTime() + 1);
+    }
+    // value filter
+    return Intervals.ALL_INTERVAL;
+  }
+
+  /** All intervals are closed. */
+  public static class Intervals extends ArrayList<Long> {
+
+    static final Intervals ALL_INTERVAL = new Intervals(Long.MIN_VALUE, Long.MAX_VALUE);
+
+    public Intervals() {
+      super();
+    }
+
+    Intervals(long lowerBound, long upperBound) {
+      super();
+      addInterval(lowerBound, upperBound);
+    }
+
+    public int getIntervalSize() {
+      return size() / 2;
+    }
+
+    public long getLowerBound(int index) {
+      return get(index * 2);
+    }
+
+    public long getUpperBound(int index) {
+      return get(index * 2 + 1);
+    }
+
+    void setLowerBound(int index, long lb) {
+      set(index * 2, lb);
+    }
+
+    void setUpperBound(int index, long ub) {
+      set(index * 2 + 1, ub);
+    }
+
+    public void addInterval(long lowerBound, long upperBound) {
+      add(lowerBound);
+      add(upperBound);
+    }
+
+    Intervals intersection(Intervals that) {
+      Intervals result = new Intervals();
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      for (int i = 0; i < thisSize; i++) {
+        for (int j = 0; j < thatSize; j++) {
+          long thisLB = this.getLowerBound(i);
+          long thisUB = this.getUpperBound(i);
+          long thatLB = that.getLowerBound(i);
+          long thatUB = that.getUpperBound(i);
+          if (thisUB >= thatLB) {
+            if (thisUB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thisUB);
+            } else if (thisLB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thatUB);
+            }
+          }
+        }
+      }
+      return result;
+    }
+
+    /**
+     * The union is implemented by merge, so the two intervals must be ordered.
+     *
+     * @param that
+     * @return
+     */
+    Intervals union(Intervals that) {
+      if (this.isEmpty()) {
+        return that;
+      } else if (that.isEmpty()) {
+        return this;
+      }
+      Intervals result = new Intervals();
+
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      int thisIndex = 0;
+      int thatIndex = 0;
+      // merge the heads of the two intervals
+      while (thisIndex < thisSize && thatIndex < thatSize) {
+        long thisLB = this.getLowerBound(thisIndex);
+        long thisUB = this.getUpperBound(thisIndex);
+        long thatLB = that.getLowerBound(thatIndex);
+        long thatUB = that.getUpperBound(thatIndex);
+        if (thisLB <= thatLB) {
+          result.mergeLast(thisLB, thisUB);
+          thisIndex++;
+        } else {
+          result.mergeLast(thatLB, thatUB);
+          thatIndex++;
+        }
+      }
+      // merge the remaining intervals
+      Intervals remainingIntervals = thisIndex < thisSize ? this : that;
+      int remainingIndex = thisIndex < thisSize ? thisIndex : thatIndex;
+      mergeRemainingIntervals(remainingIndex, remainingIntervals, result);
+
+      return result;
+    }
+
+    private void mergeRemainingIntervals(
+        int remainingIndex, Intervals remainingIntervals, Intervals result) {
+      for (int i = remainingIndex; i < remainingIntervals.getIntervalSize(); i++) {
+        long lb = remainingIntervals.getLowerBound(i);
+        long ub = remainingIntervals.getUpperBound(i);
+        result.mergeLast(lb, ub);
+      }
+    }
+
+    /**
+     * Merge an interval of [lowerBound, upperBound] with the last interval if they can be merged,
+     * or just add it as the last interval if its lowerBound is larger than the upperBound of the
+     * last interval. If the upperBound of the new interval is less than the lowerBound of the last
+     * interval, nothing will be done.
+     *
+     * @param lowerBound
+     * @param upperBound
+     */
+    private void mergeLast(long lowerBound, long upperBound) {
+      if (getIntervalSize() == 0) {
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      int lastIndex = getIntervalSize() - 1;
+      long lastLB = getLowerBound(lastIndex);
+      long lastUB = getUpperBound(lastIndex);
+      if (lowerBound > lastUB + 1) {
+        // e.g., last [3, 5], new [7, 10], just add the new interval
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      if (upperBound < lastLB - 1) {
+        // e.g., last [7, 10], new [3, 5], do nothing
+        return;
+      }
+      // merge the new interval into the last one
+      setLowerBound(lastIndex, Math.min(lastLB, lowerBound));
+      setUpperBound(lastIndex, Math.max(lastUB, upperBound));
+    }
+
+    public Intervals not() {
+      if (isEmpty()) {
+        return ALL_INTERVAL;
+      }
+      Intervals result = new Intervals();
+      long firstLB = getLowerBound(0);
+      if (firstLB != Long.MIN_VALUE) {
+        result.addInterval(Long.MIN_VALUE, firstLB - 1);
+      }
+
+      int intervalSize = getIntervalSize();
+      for (int i = 0; i < intervalSize - 1; i++) {
+        long currentUB = getUpperBound(i);
+        long nextLB = getLowerBound(i + 1);
+        if (currentUB + 1 <= nextLB - 1) {
+          result.addInterval(currentUB + 1, nextLB - 1);
+        }
+      }
+
+      long lastUB = getUpperBound(result.getIntervalSize() - 1);
+      if (lastUB != Long.MAX_VALUE) {
+        result.addInterval(lastUB + 1, Long.MAX_VALUE);
+      }
+      return result;
+    }
+  }

Review comment:
       Is those code duplicated with the `org.apache.iotdb.cluster.utils.PartitionUtils.Intervals` ? if yes, why repeat coding in two places?
   Don't repeat yourself.




-- 
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] coveralls commented on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868301377


   
   [![Coverage Status](https://coveralls.io/builds/40875804/badge)](https://coveralls.io/builds/40875804)
   
   Coverage decreased (-0.01%) to 68.071% when pulling **6a1c1aa938cc49731db7e953b34bd37269a53a31 on ijihang:fixSingleError** into **7ac39ef139e590b566acc60643bd2e29c0cb4bee on apache:master**.
   


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



[GitHub] [iotdb] coveralls edited a comment on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868301377


   
   [![Coverage Status](https://coveralls.io/builds/40924961/badge)](https://coveralls.io/builds/40924961)
   
   Coverage decreased (-0.4%) to 67.676% when pulling **15837556051250a9f92ee41e22e19522a6e6d3ae on ijihang:fixSingleError** into **7ac39ef139e590b566acc60643bd2e29c0cb4bee on apache:master**.
   


-- 
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] neuyilan commented on a change in pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

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



##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+
+  public static Intervals extractTimeInterval(Filter filter) {
+    if (filter == null) {
+      return Intervals.ALL_INTERVAL;
+    }
+    // and, or, not, value, time, group by
+    // eq, neq, gt, gteq, lt, lteq, in
+    if (filter instanceof AndFilter) {
+      AndFilter andFilter = ((AndFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(andFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(andFilter.getRight());
+      return leftIntervals.intersection(rightIntervals);
+    } else if (filter instanceof OrFilter) {
+      OrFilter orFilter = ((OrFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(orFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(orFilter.getRight());
+      return leftIntervals.union(rightIntervals);
+    } else if (filter instanceof NotFilter) {
+      NotFilter notFilter = ((NotFilter) filter);
+      return extractTimeInterval(notFilter.getFilter()).not();
+    } else if (filter instanceof TimeFilter.TimeGt) {
+      TimeFilter.TimeGt timeGt = ((TimeFilter.TimeGt) filter);
+      return new Intervals(((long) timeGt.getValue()) + 1, Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeGtEq) {
+      TimeFilter.TimeGtEq timeGtEq = ((TimeFilter.TimeGtEq) filter);
+      return new Intervals(((long) timeGtEq.getValue()), Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeEq) {
+      TimeFilter.TimeEq timeEq = ((TimeFilter.TimeEq) filter);
+      return new Intervals(((long) timeEq.getValue()), ((long) timeEq.getValue()));
+    } else if (filter instanceof TimeFilter.TimeNotEq) {
+      TimeFilter.TimeNotEq timeNotEq = ((TimeFilter.TimeNotEq) filter);
+      Intervals intervals = new Intervals();
+      intervals.addInterval(Long.MIN_VALUE, (long) timeNotEq.getValue() - 1);
+      intervals.addInterval((long) timeNotEq.getValue() + 1, Long.MAX_VALUE);
+      return intervals;
+    } else if (filter instanceof TimeFilter.TimeLt) {
+      TimeFilter.TimeLt timeLt = ((TimeFilter.TimeLt) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLt.getValue() - 1);
+    } else if (filter instanceof TimeFilter.TimeLtEq) {
+      TimeFilter.TimeLtEq timeLtEq = ((TimeFilter.TimeLtEq) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLtEq.getValue());
+    } else if (filter instanceof TimeFilter.TimeIn) {
+      TimeFilter.TimeIn timeIn = ((TimeFilter.TimeIn) filter);
+      Intervals intervals = new Intervals();
+      for (Object value : timeIn.getValues()) {
+        long time = ((long) value);
+        intervals.addInterval(time, time);
+      }
+      return intervals;
+    } else if (filter instanceof GroupByFilter) {
+      GroupByFilter groupByFilter = ((GroupByFilter) filter);
+      return new Intervals(groupByFilter.getStartTime(), groupByFilter.getEndTime() + 1);
+    }
+    // value filter
+    return Intervals.ALL_INTERVAL;
+  }
+
+  /** All intervals are closed. */
+  public static class Intervals extends ArrayList<Long> {
+
+    static final Intervals ALL_INTERVAL = new Intervals(Long.MIN_VALUE, Long.MAX_VALUE);
+
+    public Intervals() {
+      super();
+    }
+
+    Intervals(long lowerBound, long upperBound) {
+      super();
+      addInterval(lowerBound, upperBound);
+    }
+
+    public int getIntervalSize() {
+      return size() / 2;
+    }
+
+    public long getLowerBound(int index) {
+      return get(index * 2);
+    }
+
+    public long getUpperBound(int index) {
+      return get(index * 2 + 1);
+    }
+
+    void setLowerBound(int index, long lb) {
+      set(index * 2, lb);
+    }
+
+    void setUpperBound(int index, long ub) {
+      set(index * 2 + 1, ub);
+    }
+
+    public void addInterval(long lowerBound, long upperBound) {
+      add(lowerBound);
+      add(upperBound);
+    }
+
+    Intervals intersection(Intervals that) {
+      Intervals result = new Intervals();
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      for (int i = 0; i < thisSize; i++) {
+        for (int j = 0; j < thatSize; j++) {
+          long thisLB = this.getLowerBound(i);
+          long thisUB = this.getUpperBound(i);
+          long thatLB = that.getLowerBound(i);
+          long thatUB = that.getUpperBound(i);
+          if (thisUB >= thatLB) {
+            if (thisUB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thisUB);
+            } else if (thisLB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thatUB);
+            }
+          }
+        }
+      }
+      return result;
+    }
+
+    /**
+     * The union is implemented by merge, so the two intervals must be ordered.
+     *
+     * @param that
+     * @return
+     */
+    Intervals union(Intervals that) {
+      if (this.isEmpty()) {
+        return that;
+      } else if (that.isEmpty()) {
+        return this;
+      }
+      Intervals result = new Intervals();
+
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      int thisIndex = 0;
+      int thatIndex = 0;
+      // merge the heads of the two intervals
+      while (thisIndex < thisSize && thatIndex < thatSize) {
+        long thisLB = this.getLowerBound(thisIndex);
+        long thisUB = this.getUpperBound(thisIndex);
+        long thatLB = that.getLowerBound(thatIndex);
+        long thatUB = that.getUpperBound(thatIndex);
+        if (thisLB <= thatLB) {
+          result.mergeLast(thisLB, thisUB);
+          thisIndex++;
+        } else {
+          result.mergeLast(thatLB, thatUB);
+          thatIndex++;
+        }
+      }
+      // merge the remaining intervals
+      Intervals remainingIntervals = thisIndex < thisSize ? this : that;
+      int remainingIndex = thisIndex < thisSize ? thisIndex : thatIndex;
+      mergeRemainingIntervals(remainingIndex, remainingIntervals, result);
+
+      return result;
+    }
+
+    private void mergeRemainingIntervals(
+        int remainingIndex, Intervals remainingIntervals, Intervals result) {
+      for (int i = remainingIndex; i < remainingIntervals.getIntervalSize(); i++) {
+        long lb = remainingIntervals.getLowerBound(i);
+        long ub = remainingIntervals.getUpperBound(i);
+        result.mergeLast(lb, ub);
+      }
+    }
+
+    /**
+     * Merge an interval of [lowerBound, upperBound] with the last interval if they can be merged,
+     * or just add it as the last interval if its lowerBound is larger than the upperBound of the
+     * last interval. If the upperBound of the new interval is less than the lowerBound of the last
+     * interval, nothing will be done.
+     *
+     * @param lowerBound
+     * @param upperBound
+     */
+    private void mergeLast(long lowerBound, long upperBound) {
+      if (getIntervalSize() == 0) {
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      int lastIndex = getIntervalSize() - 1;
+      long lastLB = getLowerBound(lastIndex);
+      long lastUB = getUpperBound(lastIndex);
+      if (lowerBound > lastUB + 1) {
+        // e.g., last [3, 5], new [7, 10], just add the new interval
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      if (upperBound < lastLB - 1) {
+        // e.g., last [7, 10], new [3, 5], do nothing
+        return;
+      }
+      // merge the new interval into the last one
+      setLowerBound(lastIndex, Math.min(lastLB, lowerBound));
+      setUpperBound(lastIndex, Math.max(lastUB, upperBound));
+    }
+
+    public Intervals not() {
+      if (isEmpty()) {
+        return ALL_INTERVAL;
+      }
+      Intervals result = new Intervals();
+      long firstLB = getLowerBound(0);
+      if (firstLB != Long.MIN_VALUE) {
+        result.addInterval(Long.MIN_VALUE, firstLB - 1);
+      }
+
+      int intervalSize = getIntervalSize();
+      for (int i = 0; i < intervalSize - 1; i++) {
+        long currentUB = getUpperBound(i);
+        long nextLB = getLowerBound(i + 1);
+        if (currentUB + 1 <= nextLB - 1) {
+          result.addInterval(currentUB + 1, nextLB - 1);
+        }
+      }
+
+      long lastUB = getUpperBound(result.getIntervalSize() - 1);
+      if (lastUB != Long.MAX_VALUE) {
+        result.addInterval(lastUB + 1, Long.MAX_VALUE);
+      }
+      return result;
+    }
+  }

Review comment:
       Is those code duplicated with the org.apache.iotdb.cluster.utils.PartitionUtils.Intervals ? if yes, why repeat coding in two places?
   Don't repeat yourself.




-- 
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] coveralls edited a comment on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868301377






-- 
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] jixuan1989 commented on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868291410


   Hi, IMO, the server version is better...  considering you are using a relational database, Oracel or MySQL, and query using the SQL: `select * from table where id >5 and id <3`, will you get an exception?
   
   But, we indeed can optimize the execution logic, to return empty resultset ASAP. 


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



[GitHub] [iotdb] neuyilan commented on a change in pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

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



##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+
+  public static Intervals extractTimeInterval(Filter filter) {
+    if (filter == null) {
+      return Intervals.ALL_INTERVAL;
+    }
+    // and, or, not, value, time, group by
+    // eq, neq, gt, gteq, lt, lteq, in
+    if (filter instanceof AndFilter) {
+      AndFilter andFilter = ((AndFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(andFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(andFilter.getRight());
+      return leftIntervals.intersection(rightIntervals);
+    } else if (filter instanceof OrFilter) {
+      OrFilter orFilter = ((OrFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(orFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(orFilter.getRight());
+      return leftIntervals.union(rightIntervals);
+    } else if (filter instanceof NotFilter) {
+      NotFilter notFilter = ((NotFilter) filter);
+      return extractTimeInterval(notFilter.getFilter()).not();
+    } else if (filter instanceof TimeFilter.TimeGt) {
+      TimeFilter.TimeGt timeGt = ((TimeFilter.TimeGt) filter);
+      return new Intervals(((long) timeGt.getValue()) + 1, Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeGtEq) {
+      TimeFilter.TimeGtEq timeGtEq = ((TimeFilter.TimeGtEq) filter);
+      return new Intervals(((long) timeGtEq.getValue()), Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeEq) {
+      TimeFilter.TimeEq timeEq = ((TimeFilter.TimeEq) filter);
+      return new Intervals(((long) timeEq.getValue()), ((long) timeEq.getValue()));
+    } else if (filter instanceof TimeFilter.TimeNotEq) {
+      TimeFilter.TimeNotEq timeNotEq = ((TimeFilter.TimeNotEq) filter);
+      Intervals intervals = new Intervals();
+      intervals.addInterval(Long.MIN_VALUE, (long) timeNotEq.getValue() - 1);
+      intervals.addInterval((long) timeNotEq.getValue() + 1, Long.MAX_VALUE);
+      return intervals;
+    } else if (filter instanceof TimeFilter.TimeLt) {
+      TimeFilter.TimeLt timeLt = ((TimeFilter.TimeLt) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLt.getValue() - 1);
+    } else if (filter instanceof TimeFilter.TimeLtEq) {
+      TimeFilter.TimeLtEq timeLtEq = ((TimeFilter.TimeLtEq) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLtEq.getValue());
+    } else if (filter instanceof TimeFilter.TimeIn) {
+      TimeFilter.TimeIn timeIn = ((TimeFilter.TimeIn) filter);
+      Intervals intervals = new Intervals();
+      for (Object value : timeIn.getValues()) {
+        long time = ((long) value);
+        intervals.addInterval(time, time);
+      }
+      return intervals;
+    } else if (filter instanceof GroupByFilter) {
+      GroupByFilter groupByFilter = ((GroupByFilter) filter);
+      return new Intervals(groupByFilter.getStartTime(), groupByFilter.getEndTime() + 1);
+    }
+    // value filter
+    return Intervals.ALL_INTERVAL;
+  }
+
+  /** All intervals are closed. */
+  public static class Intervals extends ArrayList<Long> {
+
+    static final Intervals ALL_INTERVAL = new Intervals(Long.MIN_VALUE, Long.MAX_VALUE);
+
+    public Intervals() {
+      super();
+    }
+
+    Intervals(long lowerBound, long upperBound) {
+      super();
+      addInterval(lowerBound, upperBound);
+    }
+
+    public int getIntervalSize() {
+      return size() / 2;
+    }
+
+    public long getLowerBound(int index) {
+      return get(index * 2);
+    }
+
+    public long getUpperBound(int index) {
+      return get(index * 2 + 1);
+    }
+
+    void setLowerBound(int index, long lb) {
+      set(index * 2, lb);
+    }
+
+    void setUpperBound(int index, long ub) {
+      set(index * 2 + 1, ub);
+    }
+
+    public void addInterval(long lowerBound, long upperBound) {
+      add(lowerBound);
+      add(upperBound);
+    }
+
+    Intervals intersection(Intervals that) {
+      Intervals result = new Intervals();
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      for (int i = 0; i < thisSize; i++) {
+        for (int j = 0; j < thatSize; j++) {
+          long thisLB = this.getLowerBound(i);
+          long thisUB = this.getUpperBound(i);
+          long thatLB = that.getLowerBound(i);
+          long thatUB = that.getUpperBound(i);
+          if (thisUB >= thatLB) {
+            if (thisUB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thisUB);
+            } else if (thisLB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thatUB);
+            }
+          }
+        }
+      }
+      return result;
+    }
+
+    /**
+     * The union is implemented by merge, so the two intervals must be ordered.
+     *
+     * @param that
+     * @return
+     */
+    Intervals union(Intervals that) {
+      if (this.isEmpty()) {
+        return that;
+      } else if (that.isEmpty()) {
+        return this;
+      }
+      Intervals result = new Intervals();
+
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      int thisIndex = 0;
+      int thatIndex = 0;
+      // merge the heads of the two intervals
+      while (thisIndex < thisSize && thatIndex < thatSize) {
+        long thisLB = this.getLowerBound(thisIndex);
+        long thisUB = this.getUpperBound(thisIndex);
+        long thatLB = that.getLowerBound(thatIndex);
+        long thatUB = that.getUpperBound(thatIndex);
+        if (thisLB <= thatLB) {
+          result.mergeLast(thisLB, thisUB);
+          thisIndex++;
+        } else {
+          result.mergeLast(thatLB, thatUB);
+          thatIndex++;
+        }
+      }
+      // merge the remaining intervals
+      Intervals remainingIntervals = thisIndex < thisSize ? this : that;
+      int remainingIndex = thisIndex < thisSize ? thisIndex : thatIndex;
+      mergeRemainingIntervals(remainingIndex, remainingIntervals, result);
+
+      return result;
+    }
+
+    private void mergeRemainingIntervals(
+        int remainingIndex, Intervals remainingIntervals, Intervals result) {
+      for (int i = remainingIndex; i < remainingIntervals.getIntervalSize(); i++) {
+        long lb = remainingIntervals.getLowerBound(i);
+        long ub = remainingIntervals.getUpperBound(i);
+        result.mergeLast(lb, ub);
+      }
+    }
+
+    /**
+     * Merge an interval of [lowerBound, upperBound] with the last interval if they can be merged,
+     * or just add it as the last interval if its lowerBound is larger than the upperBound of the
+     * last interval. If the upperBound of the new interval is less than the lowerBound of the last
+     * interval, nothing will be done.
+     *
+     * @param lowerBound
+     * @param upperBound
+     */
+    private void mergeLast(long lowerBound, long upperBound) {
+      if (getIntervalSize() == 0) {
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      int lastIndex = getIntervalSize() - 1;
+      long lastLB = getLowerBound(lastIndex);
+      long lastUB = getUpperBound(lastIndex);
+      if (lowerBound > lastUB + 1) {
+        // e.g., last [3, 5], new [7, 10], just add the new interval
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      if (upperBound < lastLB - 1) {
+        // e.g., last [7, 10], new [3, 5], do nothing
+        return;
+      }
+      // merge the new interval into the last one
+      setLowerBound(lastIndex, Math.min(lastLB, lowerBound));
+      setUpperBound(lastIndex, Math.max(lastUB, upperBound));
+    }
+
+    public Intervals not() {
+      if (isEmpty()) {
+        return ALL_INTERVAL;
+      }
+      Intervals result = new Intervals();
+      long firstLB = getLowerBound(0);
+      if (firstLB != Long.MIN_VALUE) {
+        result.addInterval(Long.MIN_VALUE, firstLB - 1);
+      }
+
+      int intervalSize = getIntervalSize();
+      for (int i = 0; i < intervalSize - 1; i++) {
+        long currentUB = getUpperBound(i);
+        long nextLB = getLowerBound(i + 1);
+        if (currentUB + 1 <= nextLB - 1) {
+          result.addInterval(currentUB + 1, nextLB - 1);
+        }
+      }
+
+      long lastUB = getUpperBound(result.getIntervalSize() - 1);
+      if (lastUB != Long.MAX_VALUE) {
+        result.addInterval(lastUB + 1, Long.MAX_VALUE);
+      }
+      return result;
+    }
+  }

Review comment:
       Is those code duplicated with the `org.apache.iotdb.cluster.utils.PartitionUtils.Intervals` ? if yes, why repeat coding in two places?
   Don't repeat yourself.




-- 
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] coveralls edited a comment on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868301377


   
   [![Coverage Status](https://coveralls.io/builds/40967339/badge)](https://coveralls.io/builds/40967339)
   
   Coverage decreased (-0.9%) to 67.162% when pulling **681e4c4f09bde4bf31afdd1f27ec8341ac30fde2 on ijihang:fixSingleError** into **7ac39ef139e590b566acc60643bd2e29c0cb4bee on apache:master**.
   


-- 
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] mychaow merged pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

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


   


-- 
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] ijihang commented on a change in pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

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



##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+
+  public static Intervals extractTimeInterval(Filter filter) {
+    if (filter == null) {
+      return Intervals.ALL_INTERVAL;
+    }
+    // and, or, not, value, time, group by
+    // eq, neq, gt, gteq, lt, lteq, in
+    if (filter instanceof AndFilter) {
+      AndFilter andFilter = ((AndFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(andFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(andFilter.getRight());
+      return leftIntervals.intersection(rightIntervals);
+    } else if (filter instanceof OrFilter) {
+      OrFilter orFilter = ((OrFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(orFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(orFilter.getRight());
+      return leftIntervals.union(rightIntervals);
+    } else if (filter instanceof NotFilter) {
+      NotFilter notFilter = ((NotFilter) filter);
+      return extractTimeInterval(notFilter.getFilter()).not();
+    } else if (filter instanceof TimeFilter.TimeGt) {
+      TimeFilter.TimeGt timeGt = ((TimeFilter.TimeGt) filter);
+      return new Intervals(((long) timeGt.getValue()) + 1, Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeGtEq) {
+      TimeFilter.TimeGtEq timeGtEq = ((TimeFilter.TimeGtEq) filter);
+      return new Intervals(((long) timeGtEq.getValue()), Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeEq) {
+      TimeFilter.TimeEq timeEq = ((TimeFilter.TimeEq) filter);
+      return new Intervals(((long) timeEq.getValue()), ((long) timeEq.getValue()));
+    } else if (filter instanceof TimeFilter.TimeNotEq) {
+      TimeFilter.TimeNotEq timeNotEq = ((TimeFilter.TimeNotEq) filter);
+      Intervals intervals = new Intervals();
+      intervals.addInterval(Long.MIN_VALUE, (long) timeNotEq.getValue() - 1);
+      intervals.addInterval((long) timeNotEq.getValue() + 1, Long.MAX_VALUE);
+      return intervals;
+    } else if (filter instanceof TimeFilter.TimeLt) {
+      TimeFilter.TimeLt timeLt = ((TimeFilter.TimeLt) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLt.getValue() - 1);
+    } else if (filter instanceof TimeFilter.TimeLtEq) {
+      TimeFilter.TimeLtEq timeLtEq = ((TimeFilter.TimeLtEq) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLtEq.getValue());
+    } else if (filter instanceof TimeFilter.TimeIn) {
+      TimeFilter.TimeIn timeIn = ((TimeFilter.TimeIn) filter);
+      Intervals intervals = new Intervals();
+      for (Object value : timeIn.getValues()) {
+        long time = ((long) value);
+        intervals.addInterval(time, time);
+      }
+      return intervals;
+    } else if (filter instanceof GroupByFilter) {
+      GroupByFilter groupByFilter = ((GroupByFilter) filter);
+      return new Intervals(groupByFilter.getStartTime(), groupByFilter.getEndTime() + 1);
+    }
+    // value filter
+    return Intervals.ALL_INTERVAL;
+  }
+
+  /** All intervals are closed. */
+  public static class Intervals extends ArrayList<Long> {
+
+    static final Intervals ALL_INTERVAL = new Intervals(Long.MIN_VALUE, Long.MAX_VALUE);
+
+    public Intervals() {
+      super();
+    }
+
+    Intervals(long lowerBound, long upperBound) {
+      super();
+      addInterval(lowerBound, upperBound);
+    }
+
+    public int getIntervalSize() {
+      return size() / 2;
+    }
+
+    public long getLowerBound(int index) {
+      return get(index * 2);
+    }
+
+    public long getUpperBound(int index) {
+      return get(index * 2 + 1);
+    }
+
+    void setLowerBound(int index, long lb) {
+      set(index * 2, lb);
+    }
+
+    void setUpperBound(int index, long ub) {
+      set(index * 2 + 1, ub);
+    }
+
+    public void addInterval(long lowerBound, long upperBound) {
+      add(lowerBound);
+      add(upperBound);
+    }
+
+    Intervals intersection(Intervals that) {
+      Intervals result = new Intervals();
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      for (int i = 0; i < thisSize; i++) {
+        for (int j = 0; j < thatSize; j++) {
+          long thisLB = this.getLowerBound(i);
+          long thisUB = this.getUpperBound(i);
+          long thatLB = that.getLowerBound(i);
+          long thatUB = that.getUpperBound(i);
+          if (thisUB >= thatLB) {
+            if (thisUB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thisUB);
+            } else if (thisLB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thatUB);
+            }
+          }
+        }
+      }
+      return result;
+    }
+
+    /**
+     * The union is implemented by merge, so the two intervals must be ordered.
+     *
+     * @param that
+     * @return
+     */
+    Intervals union(Intervals that) {
+      if (this.isEmpty()) {
+        return that;
+      } else if (that.isEmpty()) {
+        return this;
+      }
+      Intervals result = new Intervals();
+
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      int thisIndex = 0;
+      int thatIndex = 0;
+      // merge the heads of the two intervals
+      while (thisIndex < thisSize && thatIndex < thatSize) {
+        long thisLB = this.getLowerBound(thisIndex);
+        long thisUB = this.getUpperBound(thisIndex);
+        long thatLB = that.getLowerBound(thatIndex);
+        long thatUB = that.getUpperBound(thatIndex);
+        if (thisLB <= thatLB) {
+          result.mergeLast(thisLB, thisUB);
+          thisIndex++;
+        } else {
+          result.mergeLast(thatLB, thatUB);
+          thatIndex++;
+        }
+      }
+      // merge the remaining intervals
+      Intervals remainingIntervals = thisIndex < thisSize ? this : that;
+      int remainingIndex = thisIndex < thisSize ? thisIndex : thatIndex;
+      mergeRemainingIntervals(remainingIndex, remainingIntervals, result);
+
+      return result;
+    }
+
+    private void mergeRemainingIntervals(
+        int remainingIndex, Intervals remainingIntervals, Intervals result) {
+      for (int i = remainingIndex; i < remainingIntervals.getIntervalSize(); i++) {
+        long lb = remainingIntervals.getLowerBound(i);
+        long ub = remainingIntervals.getUpperBound(i);
+        result.mergeLast(lb, ub);
+      }
+    }
+
+    /**
+     * Merge an interval of [lowerBound, upperBound] with the last interval if they can be merged,
+     * or just add it as the last interval if its lowerBound is larger than the upperBound of the
+     * last interval. If the upperBound of the new interval is less than the lowerBound of the last
+     * interval, nothing will be done.
+     *
+     * @param lowerBound
+     * @param upperBound
+     */
+    private void mergeLast(long lowerBound, long upperBound) {
+      if (getIntervalSize() == 0) {
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      int lastIndex = getIntervalSize() - 1;
+      long lastLB = getLowerBound(lastIndex);
+      long lastUB = getUpperBound(lastIndex);
+      if (lowerBound > lastUB + 1) {
+        // e.g., last [3, 5], new [7, 10], just add the new interval
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      if (upperBound < lastLB - 1) {
+        // e.g., last [7, 10], new [3, 5], do nothing
+        return;
+      }
+      // merge the new interval into the last one
+      setLowerBound(lastIndex, Math.min(lastLB, lowerBound));
+      setUpperBound(lastIndex, Math.max(lastUB, upperBound));
+    }
+
+    public Intervals not() {
+      if (isEmpty()) {
+        return ALL_INTERVAL;
+      }
+      Intervals result = new Intervals();
+      long firstLB = getLowerBound(0);
+      if (firstLB != Long.MIN_VALUE) {
+        result.addInterval(Long.MIN_VALUE, firstLB - 1);
+      }
+
+      int intervalSize = getIntervalSize();
+      for (int i = 0; i < intervalSize - 1; i++) {
+        long currentUB = getUpperBound(i);
+        long nextLB = getLowerBound(i + 1);
+        if (currentUB + 1 <= nextLB - 1) {
+          result.addInterval(currentUB + 1, nextLB - 1);
+        }
+      }
+
+      long lastUB = getUpperBound(result.getIntervalSize() - 1);
+      if (lastUB != Long.MAX_VALUE) {
+        result.addInterval(lastUB + 1, Long.MAX_VALUE);
+      }
+      return result;
+    }
+  }

Review comment:
       I'll make some improvements




-- 
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] ijihang commented on a change in pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

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



##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+
+  public static Intervals extractTimeInterval(Filter filter) {
+    if (filter == null) {
+      return Intervals.ALL_INTERVAL;
+    }
+    // and, or, not, value, time, group by
+    // eq, neq, gt, gteq, lt, lteq, in
+    if (filter instanceof AndFilter) {
+      AndFilter andFilter = ((AndFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(andFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(andFilter.getRight());
+      return leftIntervals.intersection(rightIntervals);
+    } else if (filter instanceof OrFilter) {
+      OrFilter orFilter = ((OrFilter) filter);
+      Intervals leftIntervals = extractTimeInterval(orFilter.getLeft());
+      Intervals rightIntervals = extractTimeInterval(orFilter.getRight());
+      return leftIntervals.union(rightIntervals);
+    } else if (filter instanceof NotFilter) {
+      NotFilter notFilter = ((NotFilter) filter);
+      return extractTimeInterval(notFilter.getFilter()).not();
+    } else if (filter instanceof TimeFilter.TimeGt) {
+      TimeFilter.TimeGt timeGt = ((TimeFilter.TimeGt) filter);
+      return new Intervals(((long) timeGt.getValue()) + 1, Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeGtEq) {
+      TimeFilter.TimeGtEq timeGtEq = ((TimeFilter.TimeGtEq) filter);
+      return new Intervals(((long) timeGtEq.getValue()), Long.MAX_VALUE);
+    } else if (filter instanceof TimeFilter.TimeEq) {
+      TimeFilter.TimeEq timeEq = ((TimeFilter.TimeEq) filter);
+      return new Intervals(((long) timeEq.getValue()), ((long) timeEq.getValue()));
+    } else if (filter instanceof TimeFilter.TimeNotEq) {
+      TimeFilter.TimeNotEq timeNotEq = ((TimeFilter.TimeNotEq) filter);
+      Intervals intervals = new Intervals();
+      intervals.addInterval(Long.MIN_VALUE, (long) timeNotEq.getValue() - 1);
+      intervals.addInterval((long) timeNotEq.getValue() + 1, Long.MAX_VALUE);
+      return intervals;
+    } else if (filter instanceof TimeFilter.TimeLt) {
+      TimeFilter.TimeLt timeLt = ((TimeFilter.TimeLt) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLt.getValue() - 1);
+    } else if (filter instanceof TimeFilter.TimeLtEq) {
+      TimeFilter.TimeLtEq timeLtEq = ((TimeFilter.TimeLtEq) filter);
+      return new Intervals(Long.MIN_VALUE, (long) timeLtEq.getValue());
+    } else if (filter instanceof TimeFilter.TimeIn) {
+      TimeFilter.TimeIn timeIn = ((TimeFilter.TimeIn) filter);
+      Intervals intervals = new Intervals();
+      for (Object value : timeIn.getValues()) {
+        long time = ((long) value);
+        intervals.addInterval(time, time);
+      }
+      return intervals;
+    } else if (filter instanceof GroupByFilter) {
+      GroupByFilter groupByFilter = ((GroupByFilter) filter);
+      return new Intervals(groupByFilter.getStartTime(), groupByFilter.getEndTime() + 1);
+    }
+    // value filter
+    return Intervals.ALL_INTERVAL;
+  }
+
+  /** All intervals are closed. */
+  public static class Intervals extends ArrayList<Long> {
+
+    static final Intervals ALL_INTERVAL = new Intervals(Long.MIN_VALUE, Long.MAX_VALUE);
+
+    public Intervals() {
+      super();
+    }
+
+    Intervals(long lowerBound, long upperBound) {
+      super();
+      addInterval(lowerBound, upperBound);
+    }
+
+    public int getIntervalSize() {
+      return size() / 2;
+    }
+
+    public long getLowerBound(int index) {
+      return get(index * 2);
+    }
+
+    public long getUpperBound(int index) {
+      return get(index * 2 + 1);
+    }
+
+    void setLowerBound(int index, long lb) {
+      set(index * 2, lb);
+    }
+
+    void setUpperBound(int index, long ub) {
+      set(index * 2 + 1, ub);
+    }
+
+    public void addInterval(long lowerBound, long upperBound) {
+      add(lowerBound);
+      add(upperBound);
+    }
+
+    Intervals intersection(Intervals that) {
+      Intervals result = new Intervals();
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      for (int i = 0; i < thisSize; i++) {
+        for (int j = 0; j < thatSize; j++) {
+          long thisLB = this.getLowerBound(i);
+          long thisUB = this.getUpperBound(i);
+          long thatLB = that.getLowerBound(i);
+          long thatUB = that.getUpperBound(i);
+          if (thisUB >= thatLB) {
+            if (thisUB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thisUB);
+            } else if (thisLB <= thatUB) {
+              result.addInterval(Math.max(thisLB, thatLB), thatUB);
+            }
+          }
+        }
+      }
+      return result;
+    }
+
+    /**
+     * The union is implemented by merge, so the two intervals must be ordered.
+     *
+     * @param that
+     * @return
+     */
+    Intervals union(Intervals that) {
+      if (this.isEmpty()) {
+        return that;
+      } else if (that.isEmpty()) {
+        return this;
+      }
+      Intervals result = new Intervals();
+
+      int thisSize = this.getIntervalSize();
+      int thatSize = that.getIntervalSize();
+      int thisIndex = 0;
+      int thatIndex = 0;
+      // merge the heads of the two intervals
+      while (thisIndex < thisSize && thatIndex < thatSize) {
+        long thisLB = this.getLowerBound(thisIndex);
+        long thisUB = this.getUpperBound(thisIndex);
+        long thatLB = that.getLowerBound(thatIndex);
+        long thatUB = that.getUpperBound(thatIndex);
+        if (thisLB <= thatLB) {
+          result.mergeLast(thisLB, thisUB);
+          thisIndex++;
+        } else {
+          result.mergeLast(thatLB, thatUB);
+          thatIndex++;
+        }
+      }
+      // merge the remaining intervals
+      Intervals remainingIntervals = thisIndex < thisSize ? this : that;
+      int remainingIndex = thisIndex < thisSize ? thisIndex : thatIndex;
+      mergeRemainingIntervals(remainingIndex, remainingIntervals, result);
+
+      return result;
+    }
+
+    private void mergeRemainingIntervals(
+        int remainingIndex, Intervals remainingIntervals, Intervals result) {
+      for (int i = remainingIndex; i < remainingIntervals.getIntervalSize(); i++) {
+        long lb = remainingIntervals.getLowerBound(i);
+        long ub = remainingIntervals.getUpperBound(i);
+        result.mergeLast(lb, ub);
+      }
+    }
+
+    /**
+     * Merge an interval of [lowerBound, upperBound] with the last interval if they can be merged,
+     * or just add it as the last interval if its lowerBound is larger than the upperBound of the
+     * last interval. If the upperBound of the new interval is less than the lowerBound of the last
+     * interval, nothing will be done.
+     *
+     * @param lowerBound
+     * @param upperBound
+     */
+    private void mergeLast(long lowerBound, long upperBound) {
+      if (getIntervalSize() == 0) {
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      int lastIndex = getIntervalSize() - 1;
+      long lastLB = getLowerBound(lastIndex);
+      long lastUB = getUpperBound(lastIndex);
+      if (lowerBound > lastUB + 1) {
+        // e.g., last [3, 5], new [7, 10], just add the new interval
+        addInterval(lowerBound, upperBound);
+        return;
+      }
+      if (upperBound < lastLB - 1) {
+        // e.g., last [7, 10], new [3, 5], do nothing
+        return;
+      }
+      // merge the new interval into the last one
+      setLowerBound(lastIndex, Math.min(lastLB, lowerBound));
+      setUpperBound(lastIndex, Math.max(lastUB, upperBound));
+    }
+
+    public Intervals not() {
+      if (isEmpty()) {
+        return ALL_INTERVAL;
+      }
+      Intervals result = new Intervals();
+      long firstLB = getLowerBound(0);
+      if (firstLB != Long.MIN_VALUE) {
+        result.addInterval(Long.MIN_VALUE, firstLB - 1);
+      }
+
+      int intervalSize = getIntervalSize();
+      for (int i = 0; i < intervalSize - 1; i++) {
+        long currentUB = getUpperBound(i);
+        long nextLB = getLowerBound(i + 1);
+        if (currentUB + 1 <= nextLB - 1) {
+          result.addInterval(currentUB + 1, nextLB - 1);
+        }
+      }
+
+      long lastUB = getUpperBound(result.getIntervalSize() - 1);
+      if (lastUB != Long.MAX_VALUE) {
+        result.addInterval(lastUB + 1, Long.MAX_VALUE);
+      }
+      return result;
+    }
+  }

Review comment:
       I'll make some improvements




-- 
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] jixuan1989 edited a comment on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
jixuan1989 edited a comment on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868291410


   Hi, IMO, the server version is better...  considering you are using a relational database, Oracle or MySQL, and query using the SQL: `select * from table where id >5 and id <3`, will you get an exception?
   
   But, we indeed can optimize the execution logic, to return empty resultset ASAP. 


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



[GitHub] [iotdb] neuyilan commented on a change in pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

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



##########
File path: server/src/main/java/org/apache/iotdb/db/utils/TimeValuePairUtils.java
##########
@@ -107,4 +115,228 @@ public static TimeValuePair getEmptyTimeValuePair(TSDataType dataType) {
         throw new UnsupportedOperationException("Unrecognized datatype: " + dataType);
     }
   }
+

Review comment:
       Is those code duplicated with the `org.apache.iotdb.cluster.utils.PartitionUtils.Intervals` ? if yes, why repeat coding in two places?
   Don't repeat yourself.




-- 
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] coveralls edited a comment on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868301377


   
   [![Coverage Status](https://coveralls.io/builds/40930449/badge)](https://coveralls.io/builds/40930449)
   
   Coverage decreased (-0.4%) to 67.69% when pulling **c2d13692a105da7d567fb21f45a130d82a550832 on ijihang:fixSingleError** into **7ac39ef139e590b566acc60643bd2e29c0cb4bee on apache:master**.
   


-- 
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] ijihang commented on pull request #3452: [IOTDB-1453]Fix result set when the server query time filtered is And

Posted by GitBox <gi...@apache.org>.
ijihang commented on pull request #3452:
URL: https://github.com/apache/iotdb/pull/3452#issuecomment-868330489


   exactly,if Oracle or MySQL ,the  return is empty,now  I tried to return the result set to null


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