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/10/26 07:54:43 UTC

[GitHub] [iotdb] RYH61 opened a new pull request, #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

RYH61 opened a new pull request, #7736:
URL: https://github.com/apache/iotdb/pull/7736

   see : [https://issues.apache.org/jira/browse/IOTDB-4695](https://issues.apache.org/jira/browse/IOTDB-4695)
   
   
   ![图片](https://user-images.githubusercontent.com/79885238/197967507-63c71ced-ba34-4cd9-9241-0be5144c0bf2.png)
   


-- 
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] ericpai commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
ericpai commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1007567130


##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java:
##########
@@ -417,8 +424,9 @@ private List<Pair<Expression, String>> analyzeSelect(
           break;
         }
       }
+      outputExpressionMap.put(columnIndex, outputExpressions);

Review Comment:
   I think the columnIndex is very useful in later use, so it's better to set from 0, not 1. If it begins at 0, we can easily get the corresponding ResultColumn by `queryStatement.getSelectComponent().getResultColumns().get(columnIndex)`



-- 
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] RYH61 commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
RYH61 commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1007576671


##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java:
##########
@@ -417,8 +424,9 @@ private List<Pair<Expression, String>> analyzeSelect(
           break;
         }
       }
+      outputExpressionMap.put(columnIndex, outputExpressions);

Review Comment:
   ok~~



-- 
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] RYH61 commented on pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
RYH61 commented on PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#issuecomment-1291711817

   > Better to add an IT case
   
   ok~


-- 
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] ericpai commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
ericpai commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1005475490


##########
integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBAggregationByLevelIT.java:
##########
@@ -674,6 +675,25 @@ public void TestGroupByLevelWithoutAggregationFunc() {
     }
   }
 
+  @Test
+  public void groupByLevelWithSameColumn() throws SQLException {
+    String[] retArray = new String[] {"8", "8"};
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+
+      int cnt = 0;
+      try (ResultSet resultSet =
+          statement.executeQuery(
+              "select count(status),count(status) from root.*.* GROUP BY level=0")) {
+        while (resultSet.next()) {
+          String ans = resultSet.getString(count("root.*.*.status"));

Review Comment:
   Thanks. Maybe the while loop can be removed, and add another assert `assertEquals(count("root.*.*.status"), metadata.getColumn(1)); `



-- 
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] ericpai commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
ericpai commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1005475490


##########
integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBAggregationByLevelIT.java:
##########
@@ -674,6 +675,25 @@ public void TestGroupByLevelWithoutAggregationFunc() {
     }
   }
 
+  @Test
+  public void groupByLevelWithSameColumn() throws SQLException {
+    String[] retArray = new String[] {"8", "8"};
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+
+      int cnt = 0;
+      try (ResultSet resultSet =
+          statement.executeQuery(
+              "select count(status),count(status) from root.*.* GROUP BY level=0")) {
+        while (resultSet.next()) {
+          String ans = resultSet.getString(count("root.*.*.status"));

Review Comment:
   Thanks. Maybe the while loop can be removed, and add another asserts 
   `assertEquals(count("root.*.*.status"), metadata.getColumn(1)); ` and `assertEquals(2, metadata.getColumnCount()); ****



##########
integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBAggregationByLevelIT.java:
##########
@@ -674,6 +675,25 @@ public void TestGroupByLevelWithoutAggregationFunc() {
     }
   }
 
+  @Test
+  public void groupByLevelWithSameColumn() throws SQLException {
+    String[] retArray = new String[] {"8", "8"};
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+
+      int cnt = 0;
+      try (ResultSet resultSet =
+          statement.executeQuery(
+              "select count(status),count(status) from root.*.* GROUP BY level=0")) {
+        while (resultSet.next()) {
+          String ans = resultSet.getString(count("root.*.*.status"));

Review Comment:
   Thanks. Maybe the while loop can be removed, and add another asserts 
   `assertEquals(count("root.*.*.status"), metadata.getColumn(1)); ` and `assertEquals(2, metadata.getColumnCount());`



-- 
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] RYH61 commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
RYH61 commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1005432990


##########
integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBAggregationByLevelIT.java:
##########
@@ -674,6 +675,25 @@ public void TestGroupByLevelWithoutAggregationFunc() {
     }
   }
 
+  @Test
+  public void groupByLevelWithSameColumn() throws SQLException {
+    String[] retArray = new String[] {"8", "8"};
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+
+      int cnt = 0;
+      try (ResultSet resultSet =
+          statement.executeQuery(
+              "select count(status),count(status) from root.*.* GROUP BY level=0")) {
+        while (resultSet.next()) {
+          String ans = resultSet.getString(count("root.*.*.status"));

Review Comment:
   fix~



-- 
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] ericpai commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
ericpai commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1005405901


##########
integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBAggregationByLevelIT.java:
##########
@@ -674,6 +675,25 @@ public void TestGroupByLevelWithoutAggregationFunc() {
     }
   }
 
+  @Test
+  public void groupByLevelWithSameColumn() throws SQLException {
+    String[] retArray = new String[] {"8", "8"};
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+
+      int cnt = 0;
+      try (ResultSet resultSet =
+          statement.executeQuery(
+              "select count(status),count(status) from root.*.* GROUP BY level=0")) {
+        while (resultSet.next()) {
+          String ans = resultSet.getString(count("root.*.*.status"));

Review Comment:
   It seems that this case can't verify whether the output has 2 or 1 columns. You should use `resultSet.getMetadata()` and check
   1. It contains 2 columns.
   2. The 1st and 2nd column name are both `count(root.*.*.status)`.



-- 
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] RYH61 commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
RYH61 commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1005481138


##########
integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBAggregationByLevelIT.java:
##########
@@ -674,6 +675,25 @@ public void TestGroupByLevelWithoutAggregationFunc() {
     }
   }
 
+  @Test
+  public void groupByLevelWithSameColumn() throws SQLException {
+    String[] retArray = new String[] {"8", "8"};
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+
+      int cnt = 0;
+      try (ResultSet resultSet =
+          statement.executeQuery(
+              "select count(status),count(status) from root.*.* GROUP BY level=0")) {
+        while (resultSet.next()) {
+          String ans = resultSet.getString(count("root.*.*.status"));

Review Comment:
   ok~~



-- 
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 merged pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
neuyilan merged PR #7736:
URL: https://github.com/apache/iotdb/pull/7736


-- 
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] ericpai commented on pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
ericpai commented on PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#issuecomment-1292846220

   All ClusterIT tasks are failed. Maybe there're some potential bugs.


-- 
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] RYH61 commented on a diff in pull request #7736: [IOTDB-4695] GROUP LEVEL query de-duplicates result columns unexpected

Posted by GitBox <gi...@apache.org>.
RYH61 commented on code in PR #7736:
URL: https://github.com/apache/iotdb/pull/7736#discussion_r1005409804


##########
integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBAggregationByLevelIT.java:
##########
@@ -674,6 +675,25 @@ public void TestGroupByLevelWithoutAggregationFunc() {
     }
   }
 
+  @Test
+  public void groupByLevelWithSameColumn() throws SQLException {
+    String[] retArray = new String[] {"8", "8"};
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+
+      int cnt = 0;
+      try (ResultSet resultSet =
+          statement.executeQuery(
+              "select count(status),count(status) from root.*.* GROUP BY level=0")) {
+        while (resultSet.next()) {
+          String ans = resultSet.getString(count("root.*.*.status"));

Review Comment:
   `String[] retArray = new String[] {"8", "8"};`
   
   The results that already contain two columns should be changed here



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