You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "xuzifu666 (via GitHub)" <gi...@apache.org> on 2026/04/20 12:19:40 UTC
[PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
xuzifu666 opened a new pull request, #4896:
URL: https://github.com/apache/calcite/pull/4896
jira: https://issues.apache.org/jira/browse/CALCITE-6757
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4896:
URL: https://github.com/apache/calcite/pull/4896#issuecomment-4293196490
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4896:
URL: https://github.com/apache/calcite/pull/4896#discussion_r3112894634
##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java:
##########
@@ -263,8 +265,42 @@ protected ElasticsearchAggregateRule(Config config) {
super(config);
}
+ /**
+ * Checks if the relational expression or any of its inputs
+ * is an Aggregate (LogicalAggregate or ElasticsearchAggregate).
+ * This prevents nested aggregations from being pushed down to Elasticsearch.
Review Comment:
The second part of this comment should be where the function is called.
##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java:
##########
@@ -263,8 +265,42 @@ protected ElasticsearchAggregateRule(Config config) {
super(config);
}
+ /**
+ * Checks if the relational expression or any of its inputs
+ * is an Aggregate (LogicalAggregate or ElasticsearchAggregate).
+ * This prevents nested aggregations from being pushed down to Elasticsearch.
+ */
+ private static boolean containsAggregate(RelNode node) {
+ // Handle RelSubset by checking its best or original node
+ if (node instanceof RelSubset) {
Review Comment:
I don't really understand what happens with RelSubset, maybe you can explain a bit 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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4896:
URL: https://github.com/apache/calcite/pull/4896#issuecomment-4280906134
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4896:
URL: https://github.com/apache/calcite/pull/4896#discussion_r3114621671
##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java:
##########
@@ -263,8 +265,42 @@ protected ElasticsearchAggregateRule(Config config) {
super(config);
}
+ /**
+ * Checks if the relational expression or any of its inputs
+ * is an Aggregate (LogicalAggregate or ElasticsearchAggregate).
+ * This prevents nested aggregations from being pushed down to Elasticsearch.
Review Comment:
Had added comment on called function.
##########
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java:
##########
@@ -263,8 +265,42 @@ protected ElasticsearchAggregateRule(Config config) {
super(config);
}
+ /**
+ * Checks if the relational expression or any of its inputs
+ * is an Aggregate (LogicalAggregate or ElasticsearchAggregate).
+ * This prevents nested aggregations from being pushed down to Elasticsearch.
+ */
+ private static boolean containsAggregate(RelNode node) {
+ // Handle RelSubset by checking its best or original node
+ if (node instanceof RelSubset) {
Review Comment:
Let me explain, When the optimizer processes a subquery:
1. The subquery ```SELECT deptno FROM dept``` is optimized to ```RelSubset```.
2. The main query's filter input is this ```RelSubset```.
3. If ```RelSubset``` is not processed, ```containsAggregate()``` will incorrectly return false.
4. This causes nested aggregations to be incorrectly pushed down to Elasticsearch.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4896:
URL: https://github.com/apache/calcite/pull/4896#issuecomment-4280869526
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 merged PR #4896:
URL: https://github.com/apache/calcite/pull/4896
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4896:
URL: https://github.com/apache/calcite/pull/4896#issuecomment-4294094597
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6757] Elasticsearch adapter returns wrong result when aggregating sub-query with aggregation [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4896:
URL: https://github.com/apache/calcite/pull/4896#issuecomment-4285574008
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4896&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4896&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4896&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4896)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org