You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "suibianwanwank (via GitHub)" <gi...@apache.org> on 2024/09/24 12:41:56 UTC
[PR] [CALCITE-6586] Some Rules not firing due to RelMdPredicates returning… [calcite]
suibianwanwank opened a new pull request, #3976:
URL: https://github.com/apache/calcite/pull/3976
This PR supports getPredicates(RelSubset) returning getPredicate(stripped) in place of EMPTY as a temporary solution.
But it exceeds the expected range because some rules are re-fire in VolcanoPlanner.
Major Changes:
In SortRemoveConstantKey the constant key in the sort is removed. before this PR the traitset of the sort needs to be consistent with the collation. It would result in the new sort having a different traitset than the original one, it would not be in the same RelSubSet and would not be selected.
So I tried to modify the traitSet assert in sort so that it can be set to RelComposetrait.
For example : order by a,b,c
If we can make sure that b is constant, then the traitSet of the sort can be [[a,b,c],[a,c]]. Also, when collation is empty and there are only offset and fetch, the original input collation traits can be kept in the sort.
There are also a few more changes that I will add comments on. For the Sort changes, we can probably discuss them in Jira if needed. Thanks for the suggestion and review.
--
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-6586] Some Rules not firing due to RelMdPredicates returning… [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3976:
URL: https://github.com/apache/calcite/pull/3976#issuecomment-2371218546
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3976) **Quality Gate passed**
Issues
 [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3976&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3976&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3976&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [93.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3976&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3976&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3976)
--
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-6586] Some Rules not firing due to RelMdPredicates returning… [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3976:
URL: https://github.com/apache/calcite/pull/3976#issuecomment-2371452186
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3976) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3976&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3976&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3976&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [93.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3976&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3976&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3976)
--
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-6586] Some Rules not firing due to RelMdPredicates returning… [calcite]
Posted by "suibianwanwank (via GitHub)" <gi...@apache.org>.
suibianwanwank commented on code in PR #3976:
URL: https://github.com/apache/calcite/pull/3976#discussion_r1773452097
##########
innodb/src/main/java/org/apache/calcite/adapter/innodb/InnodbFilter.java:
##########
@@ -96,4 +98,9 @@ public static InnodbFilter create(RelOptCluster cluster, RelTraitSet traitSet,
public RelCollation getImplicitCollation() {
return indexCondition.getImplicitCollation();
}
+
+ @Override public RexNode getCondition() {
Review Comment:
InnodbFIlter's condition contains both indexed and non-indexed conditions, but only the indexed filter will be executed. This will cause the FilterReduceExpression to produce incorrect results.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java:
##########
@@ -456,51 +456,23 @@ public Boolean areColumnsUnique(Values rel, RelMetadataQuery mq,
ImmutableBitSet columns, boolean ignoreNulls) {
columns = decorateWithConstantColumnsFromPredicates(columns, rel, mq);
for (RelNode rel2 : rel.getRels()) {
- if (rel2 instanceof Aggregate
Review Comment:
I'm not sure why this restriction was made, but after pullUpPredicate supported RelSubset, columns would union the constant field, And return false due to simplyProject. So I remove the restriction here.
##########
geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeSort.java:
##########
@@ -66,7 +66,7 @@ public class GeodeSort extends Sort implements GeodeRel {
@Override public Sort copy(RelTraitSet traitSet, RelNode input,
RelCollation newCollation, RexNode offset, RexNode fetch) {
- return new GeodeSort(getCluster(), traitSet, input, collation, fetch);
Review Comment:
Use the new traitset in the copy of the sort, and the other sort copies as well.
##########
core/src/test/resources/sql/sub-query.iq:
##########
@@ -922,10 +922,9 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[null:BOOLEAN], expr#5=[IS NOT NULL(
EnumerableCalc(expr#0..7=[{inputs}], EMPNO=[$t0], SAL=[$t5])
EnumerableTableScan(table=[[scott, EMP]])
EnumerableLimit(fetch=[1])
- EnumerableSort(sort0=[$0], dir0=[DESC])
Review Comment:
The changes to sub-query.iq are due to JoinOnUniqueToSemiJoinRule and SortRemoveConstantKeysRule taking effect.
--
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