You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2023/08/18 10:28:08 UTC
[GitHub] [pinot] xiangfu0 opened a new pull request, #11383: [multistage] Adding rule to skip SINGLE_VALUE aggregation function
xiangfu0 opened a new pull request, #11383:
URL: https://github.com/apache/pinot/pull/11383
(no comment)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11383: [multistage] Adding rule to skip SINGLE_VALUE aggregation function
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11383:
URL: https://github.com/apache/pinot/pull/11383#discussion_r1298906395
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -101,6 +101,23 @@ public void testHardcodedQueries()
super.testHardcodedQueries();
}
+ @Test
+ public void testSingleValueQuery()
+ throws Exception {
+ String query = "select sum(ActualElapsedTime) from mytable WHERE ActualElapsedTime > \n"
+ + "(select avg(ActualElapsedTime) as avg_profit from mytable) \n";
+ JsonNode jsonNode = postQuery(query);
Review Comment:
No,
Integration test result comparison needs to parse the query using v1 parser, which throws exceptions.
```
Caused by: java.lang.ClassCastException: class org.apache.calcite.sql.SqlSelect cannot be cast to class org.apache.calcite.sql.SqlBasicCall (org.apache.calcite.sql.SqlSelect and org.apache.calcite.sql.SqlBasicCall are in unnamed module of loader 'app')
at org.apache.pinot.sql.parsers.CalciteSqlParser.toExpression(CalciteSqlParser.java:748)
at org.apache.pinot.sql.parsers.CalciteSqlParser.compileFunctionExpression(CalciteSqlParser.java:810)
at org.apache.pinot.sql.parsers.CalciteSqlParser.toExpression(CalciteSqlParser.java:748)
at org.apache.pinot.sql.parsers.CalciteSqlParser.compileSqlNodeToPinotQuery(CalciteSqlParser.java:458)
at org.apache.pinot.sql.parsers.CalciteSqlParser.compileToPinotQuery(CalciteSqlParser.java:191)
at org.apache.pinot.sql.parsers.CalciteSqlParser.compileToPinotQuery(CalciteSqlParser.java:186)
at org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils.getQueryContext(QueryContextConverterUtils.java:48)
at org.apache.pinot.integration.tests.ClusterIntegrationTestUtils.testQueryInternal(ClusterIntegrationTestUtils.java:729)
at org.apache.pinot.integration.tests.ClusterIntegrationTestUtils.testQuery(ClusterIntegrationTestUtils.java:677)
... 27 more
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #11383: [multistage] Adding rule to skip SINGLE_VALUE aggregation function
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11383:
URL: https://github.com/apache/pinot/pull/11383#issuecomment-1683754596
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11383?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11383](https://app.codecov.io/gh/apache/pinot/pull/11383?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (768aba9) into [master](https://app.codecov.io/gh/apache/pinot/commit/575398dae0841ba152c31306cb460cfdc776b04b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (575398d) will **decrease** coverage by `46.88%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #11383 +/- ##
=============================================
- Coverage 61.46% 14.59% -46.88%
+ Complexity 6514 494 -6020
=============================================
Files 2233 2159 -74
Lines 120144 116937 -3207
Branches 18234 17782 -452
=============================================
- Hits 73848 17067 -56781
- Misses 40882 98456 +57574
+ Partials 5414 1414 -4000
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-17](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-20](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.59% <ø> (-46.74%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.59% <ø> (-46.88%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/11383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.59% <ø> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
[see 1468 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11383/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 merged pull request #11383: [multistage] Adding rule to skip SINGLE_VALUE aggregation function
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #11383:
URL: https://github.com/apache/pinot/pull/11383
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11383: [multistage] Adding rule to skip SINGLE_VALUE aggregation function
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11383:
URL: https://github.com/apache/pinot/pull/11383#discussion_r1298791276
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -101,6 +101,23 @@ public void testHardcodedQueries()
super.testHardcodedQueries();
}
+ @Test
+ public void testSingleValueQuery()
+ throws Exception {
+ String query = "select sum(ActualElapsedTime) from mytable WHERE ActualElapsedTime > \n"
+ + "(select avg(ActualElapsedTime) as avg_profit from mytable) \n";
Review Comment:
```suggestion
String query = "select sum(ActualElapsedTime) from mytable WHERE ActualElapsedTime > "
+ "(select avg(ActualElapsedTime) as avg_profit from mytable)";
```
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -101,6 +101,23 @@ public void testHardcodedQueries()
super.testHardcodedQueries();
}
+ @Test
+ public void testSingleValueQuery()
+ throws Exception {
+ String query = "select sum(ActualElapsedTime) from mytable WHERE ActualElapsedTime > \n"
+ + "(select avg(ActualElapsedTime) as avg_profit from mytable) \n";
+ JsonNode jsonNode = postQuery(query);
+ long joinResult = jsonNode.get("resultTable").get("rows").get(0).get(0).asLong();
+ System.out.println("jsonNode = " + jsonNode);
+
+ // The query of `SELECT avg(ActualElapsedTime) FROM mytable` is -1412.435033969449
+ query = "select sum(ActualElapsedTime) as profit from mytable WHERE ActualElapsedTime > -1412.435033969449 \n";
Review Comment:
```suggestion
query = "select sum(ActualElapsedTime) as profit from mytable WHERE ActualElapsedTime > -1412.435033969449";
```
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -101,6 +101,23 @@ public void testHardcodedQueries()
super.testHardcodedQueries();
}
+ @Test
+ public void testSingleValueQuery()
+ throws Exception {
+ String query = "select sum(ActualElapsedTime) from mytable WHERE ActualElapsedTime > \n"
+ + "(select avg(ActualElapsedTime) as avg_profit from mytable) \n";
+ JsonNode jsonNode = postQuery(query);
+ long joinResult = jsonNode.get("resultTable").get("rows").get(0).get(0).asLong();
+ System.out.println("jsonNode = " + jsonNode);
+
+ // The query of `SELECT avg(ActualElapsedTime) FROM mytable` is -1412.435033969449
+ query = "select sum(ActualElapsedTime) as profit from mytable WHERE ActualElapsedTime > -1412.435033969449 \n";
+ jsonNode = postQuery(query);
+ System.out.println("jsonNode = " + jsonNode);
Review Comment:
Remove
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -101,6 +101,23 @@ public void testHardcodedQueries()
super.testHardcodedQueries();
}
+ @Test
+ public void testSingleValueQuery()
+ throws Exception {
+ String query = "select sum(ActualElapsedTime) from mytable WHERE ActualElapsedTime > \n"
+ + "(select avg(ActualElapsedTime) as avg_profit from mytable) \n";
+ JsonNode jsonNode = postQuery(query);
+ long joinResult = jsonNode.get("resultTable").get("rows").get(0).get(0).asLong();
+ System.out.println("jsonNode = " + jsonNode);
Review Comment:
Remove
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -101,6 +101,23 @@ public void testHardcodedQueries()
super.testHardcodedQueries();
}
+ @Test
+ public void testSingleValueQuery()
+ throws Exception {
+ String query = "select sum(ActualElapsedTime) from mytable WHERE ActualElapsedTime > \n"
+ + "(select avg(ActualElapsedTime) as avg_profit from mytable) \n";
+ JsonNode jsonNode = postQuery(query);
Review Comment:
Can we compare with H2 result?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org