You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "NobiGo (via GitHub)" <gi...@apache.org> on 2024/12/20 12:28:40 UTC
[PR] [CALCITE-5036] Optimize the pulledPredicates list from Filter condition [calcite]
NobiGo opened a new pull request, #4104:
URL: https://github.com/apache/calcite/pull/4104
(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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Revert [CALCITE-5036] RelMetadataQuery#getPulledUpPredicates support to analyze constant key for the operator of IS_NOT_DISTINCT_FROM [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on PR #4104:
URL: https://github.com/apache/calcite/pull/4104#issuecomment-2558887677
I will merge this PR in the next 36 hours if there are no more comments.
--
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-5036] Optimize the pulledPredicates list from Filter condition [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4104:
URL: https://github.com/apache/calcite/pull/4104#discussion_r1894422561
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java:
##########
@@ -308,18 +308,11 @@ public RelOptPredicateList getPredicates(Filter filter, RelMetadataQuery mq) {
final RexBuilder rexBuilder = filter.getCluster().getRexBuilder();
final RelOptPredicateList inputInfo = mq.getPulledUpPredicates(input);
- // Simplify condition using RexSimplify.
- final RexNode condition = filter.getCondition();
- final RexExecutor executor =
- Util.first(filter.getCluster().getPlanner().getExecutor(), RexUtil.EXECUTOR);
- final RexSimplify simplify = new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor);
- final RexNode simplifiedCondition = simplify.simplify(condition);
-
return Util.first(inputInfo, RelOptPredicateList.EMPTY)
.union(rexBuilder,
RelOptPredicateList.of(rexBuilder,
RexUtil.retainDeterministic(
- RelOptUtil.conjunctions(simplifiedCondition))));
+ RelOptUtil.conjunctions(filter.getCondition()))));
Review Comment:
Yes. I revert the changes in the previous PR. Compared to the previous implementation, it can be considered an optimization. Or I will change the commit info to `[CALCITE-5036] Revert the changes`?
--
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-5036] Optimize the pulledPredicates list from Filter condition [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4104:
URL: https://github.com/apache/calcite/pull/4104#discussion_r1894239043
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java:
##########
@@ -308,18 +308,11 @@ public RelOptPredicateList getPredicates(Filter filter, RelMetadataQuery mq) {
final RexBuilder rexBuilder = filter.getCluster().getRexBuilder();
final RelOptPredicateList inputInfo = mq.getPulledUpPredicates(input);
- // Simplify condition using RexSimplify.
- final RexNode condition = filter.getCondition();
- final RexExecutor executor =
- Util.first(filter.getCluster().getPlanner().getExecutor(), RexUtil.EXECUTOR);
- final RexSimplify simplify = new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor);
- final RexNode simplifiedCondition = simplify.simplify(condition);
-
return Util.first(inputInfo, RelOptPredicateList.EMPTY)
.union(rexBuilder,
RelOptPredicateList.of(rexBuilder,
RexUtil.retainDeterministic(
- RelOptUtil.conjunctions(simplifiedCondition))));
+ RelOptUtil.conjunctions(filter.getCondition()))));
Review Comment:
it looks to me that the title is the opposite of what happens here: you *don't* optimize the list anymore.
--
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] Revert [CALCITE-5036] RelMetadataQuery#getPulledUpPredicates support to analyze constant key for the operator of IS_NOT_DISTINCT_FROM [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4104:
URL: https://github.com/apache/calcite/pull/4104#issuecomment-2558905884
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4104) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4104&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4104&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4104&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4104&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4104&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4104)
--
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-5036] Optimize the pulledPredicates list from Filter condition [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4104:
URL: https://github.com/apache/calcite/pull/4104#discussion_r1894422561
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java:
##########
@@ -308,18 +308,11 @@ public RelOptPredicateList getPredicates(Filter filter, RelMetadataQuery mq) {
final RexBuilder rexBuilder = filter.getCluster().getRexBuilder();
final RelOptPredicateList inputInfo = mq.getPulledUpPredicates(input);
- // Simplify condition using RexSimplify.
- final RexNode condition = filter.getCondition();
- final RexExecutor executor =
- Util.first(filter.getCluster().getPlanner().getExecutor(), RexUtil.EXECUTOR);
- final RexSimplify simplify = new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor);
- final RexNode simplifiedCondition = simplify.simplify(condition);
-
return Util.first(inputInfo, RelOptPredicateList.EMPTY)
.union(rexBuilder,
RelOptPredicateList.of(rexBuilder,
RexUtil.retainDeterministic(
- RelOptUtil.conjunctions(simplifiedCondition))));
+ RelOptUtil.conjunctions(filter.getCondition()))));
Review Comment:
Yes. I reverted the changes in the previous PR. I will change the commit info to `Revert [CALCITE-5036] XXXX` to keep the same style as before.
--
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-5036] Optimize the pulledPredicates list from Filter condition [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4104:
URL: https://github.com/apache/calcite/pull/4104#discussion_r1894555143
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java:
##########
@@ -308,18 +308,11 @@ public RelOptPredicateList getPredicates(Filter filter, RelMetadataQuery mq) {
final RexBuilder rexBuilder = filter.getCluster().getRexBuilder();
final RelOptPredicateList inputInfo = mq.getPulledUpPredicates(input);
- // Simplify condition using RexSimplify.
- final RexNode condition = filter.getCondition();
- final RexExecutor executor =
- Util.first(filter.getCluster().getPlanner().getExecutor(), RexUtil.EXECUTOR);
- final RexSimplify simplify = new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor);
- final RexNode simplifiedCondition = simplify.simplify(condition);
-
return Util.first(inputInfo, RelOptPredicateList.EMPTY)
.union(rexBuilder,
RelOptPredicateList.of(rexBuilder,
RexUtil.retainDeterministic(
- RelOptUtil.conjunctions(simplifiedCondition))));
+ RelOptUtil.conjunctions(filter.getCondition()))));
Review Comment:
yes, please change the PR and commit title, and perhaps link to the actual JIRA issue that is being solved.
Otherwise this looks fine.
--
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-5036] Optimize the pulledPredicates list from Filter condition [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4104:
URL: https://github.com/apache/calcite/pull/4104#issuecomment-2556950606
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4104) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4104&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4104&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4104&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4104&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4104&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4104)
--
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] Revert [CALCITE-5036] RelMetadataQuery#getPulledUpPredicates support to analyze constant key for the operator of IS_NOT_DISTINCT_FROM [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo merged PR #4104:
URL: https://github.com/apache/calcite/pull/4104
--
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