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 2020/05/02 07:48:29 UTC

[GitHub] [incubator-iotdb] JulianFeinauer opened a new pull request #1146: Added workaround when doing Aggregation over boolean Series.

JulianFeinauer opened a new pull request #1146:
URL: https://github.com/apache/incubator-iotdb/pull/1146


   As discussed, please have a look @jixuan1989 


----------------------------------------------------------------
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] [incubator-iotdb] JulianFeinauer commented on pull request #1146: Added workaround when doing Aggregation over boolean Series.

Posted by GitBox <gi...@apache.org>.
JulianFeinauer commented on pull request #1146:
URL: https://github.com/apache/incubator-iotdb/pull/1146#issuecomment-622821366


   Makes sense I guess. Will add that
   
   Holen Sie sich Outlook für Android<https://aka.ms/ghei36>
   
   ________________________________
   From: Xiangdong Huang <no...@github.com>
   Sent: Saturday, May 2, 2020 9:50:48 AM
   To: apache/incubator-iotdb <in...@noreply.github.com>
   Cc: Julian Feinauer <j....@pragmaticminds.de>; Author <au...@noreply.github.com>
   Subject: Re: [apache/incubator-iotdb] Added workaround when doing Aggregation over boolean Series. (#1146)
   
   
   @jixuan1989 commented on this pull request.
   
   ________________________________
   
   In grafana/src/main/java/org/apache/iotdb/web/grafana/dao/impl/BasicDaoImpl.java<https://github.com/apache/incubator-iotdb/pull/1146#discussion_r418927351>:
   
   >    @Override
      public List<TimeValues> querySeries(String s, Pair<ZonedDateTime, ZonedDateTime> timeRange) {
   +    try {
   +      return querySeriesInternal(s, timeRange, function);
   +    } catch (Exception e) {
   +      logger.info("Execution failed, trying now with FIRST Function!");
   +      // Try it with FIRST
   +      try {
   +        return querySeriesInternal(s, timeRange, "FIRST");
   +      } catch (Exception e2) {
   +        logger.warn("Even FIRST query did not succeed, returning NULL now", e2);
   +        return null;
   
   
   how about empty List?
   
   —
   You are receiving this because you authored the thread.
   Reply to this email directly, view it on GitHub<https://github.com/apache/incubator-iotdb/pull/1146#pullrequestreview-404508751>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AECOQCXTEG3FR23WPYTV7J3RPPGFRANCNFSM4MXSM3YQ>.
   


----------------------------------------------------------------
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] [incubator-iotdb] JulianFeinauer commented on a change in pull request #1146: [IOTDB-635]Added workaround when doing Aggregation over boolean Series.

Posted by GitBox <gi...@apache.org>.
JulianFeinauer commented on a change in pull request #1146:
URL: https://github.com/apache/incubator-iotdb/pull/1146#discussion_r418931467



##########
File path: grafana/src/main/java/org/apache/iotdb/web/grafana/dao/impl/BasicDaoImpl.java
##########
@@ -98,8 +98,28 @@ public Object doInConnection(Connection connection) throws SQLException {
     return (List<String>) jdbcTemplate.execute(connectionCallback);
   }
 
+  /**
+   * Note: If the query fails this could be due to AGGREGATIION like AVG on booleayn field.
+   * Thus, we then do a retry with FIRST aggregation.
+   * This should be solved better in the long run.
+   */
   @Override
   public List<TimeValues> querySeries(String s, Pair<ZonedDateTime, ZonedDateTime> timeRange) {
+    try {
+      return querySeriesInternal(s, timeRange, function);
+    } catch (Exception e) {
+      logger.info("Execution failed, trying now with FIRST Function!");
+      // Try it with FIRST
+      try {
+        return querySeriesInternal(s, timeRange, "FIRST");
+      } catch (Exception e2) {
+        logger.warn("Even FIRST query did not succeed, returning NULL now", e2);
+        return null;

Review comment:
       +1, done




----------------------------------------------------------------
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] [incubator-iotdb] jixuan1989 commented on a change in pull request #1146: Added workaround when doing Aggregation over boolean Series.

Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on a change in pull request #1146:
URL: https://github.com/apache/incubator-iotdb/pull/1146#discussion_r418927463



##########
File path: grafana/src/main/resources/application.properties
##########
@@ -32,4 +32,4 @@ isDownSampling=true
 interval=1m
 # aggregation function to use to downsampling the data
 # COUNT, FIRST_VALUE, LAST_VALUE, MAX_TIME, MAX_VALUE, AVG, MIN_TIME, MIN_VALUE, NOW, SUM
-function=avg
\ No newline at end of file
+function=AVG

Review comment:
       add a comment here: if the data type does not support AVG, then FIRST will be used.




----------------------------------------------------------------
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] [incubator-iotdb] JulianFeinauer commented on a change in pull request #1146: [IOTDB-635]Added workaround when doing Aggregation over boolean Series.

Posted by GitBox <gi...@apache.org>.
JulianFeinauer commented on a change in pull request #1146:
URL: https://github.com/apache/incubator-iotdb/pull/1146#discussion_r418931476



##########
File path: grafana/src/main/resources/application.properties
##########
@@ -32,4 +32,4 @@ isDownSampling=true
 interval=1m
 # aggregation function to use to downsampling the data
 # COUNT, FIRST_VALUE, LAST_VALUE, MAX_TIME, MAX_VALUE, AVG, MIN_TIME, MIN_VALUE, NOW, SUM
-function=avg
\ No newline at end of file
+function=AVG

Review comment:
       done.




----------------------------------------------------------------
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] [incubator-iotdb] jixuan1989 commented on a change in pull request #1146: Added workaround when doing Aggregation over boolean Series.

Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on a change in pull request #1146:
URL: https://github.com/apache/incubator-iotdb/pull/1146#discussion_r418927351



##########
File path: grafana/src/main/java/org/apache/iotdb/web/grafana/dao/impl/BasicDaoImpl.java
##########
@@ -98,8 +98,28 @@ public Object doInConnection(Connection connection) throws SQLException {
     return (List<String>) jdbcTemplate.execute(connectionCallback);
   }
 
+  /**
+   * Note: If the query fails this could be due to AGGREGATIION like AVG on booleayn field.
+   * Thus, we then do a retry with FIRST aggregation.
+   * This should be solved better in the long run.
+   */
   @Override
   public List<TimeValues> querySeries(String s, Pair<ZonedDateTime, ZonedDateTime> timeRange) {
+    try {
+      return querySeriesInternal(s, timeRange, function);
+    } catch (Exception e) {
+      logger.info("Execution failed, trying now with FIRST Function!");
+      // Try it with FIRST
+      try {
+        return querySeriesInternal(s, timeRange, "FIRST");
+      } catch (Exception e2) {
+        logger.warn("Even FIRST query did not succeed, returning NULL now", e2);
+        return null;

Review comment:
       how about empty List?




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