rubenada commented on code in PR #3193:
URL: https://github.com/apache/calcite/pull/3193#discussion_r1202473255
##########
core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java:
##########
@@ -866,6 +866,8 @@ private static void matchFilter(SubQueryRemoveRule rule,
LogicVisitor.find(RelOptUtil.Logic.TRUE, ImmutableList.of(c), e);
final Set<CorrelationId> variablesSet =
RelOptUtil.getVariablesUsed(e.rel);
+ /* Only consider the correlated variables which originated from this
sub-query level*/
+ variablesSet.retainAll(filter.getVariablesSet());
Review Comment:
Interestingly, when I apply this fix in my local environment and run
CoreQuidemTest, I get a regression in `misc.iq` :
```
< +-----+---+
< | KEY | C |
< +-----+---+
< | 2 | 1 |
< +-----+---+
< (1 row)
<
---
> java.lang.AssertionError: Correlation variable $cor0 should be defined
> at
org.apache.calcite.adapter.enumerable.EnumerableRelImplementor.getCorrelVariableGetter(EnumerableRelImplementor.java:476)
> at
org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitFieldAccess(RexToLixTranslator.java:1393)
> at
org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitFieldAccess(RexToLixTranslator.java:101)
> at org.apache.calcite.rex.RexFieldAccess.accept(RexFieldAccess.java:94)
...
```
Not sure why this is not shown on the PR checks...
##########
core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java:
##########
@@ -866,6 +866,8 @@ private static void matchFilter(SubQueryRemoveRule rule,
LogicVisitor.find(RelOptUtil.Logic.TRUE, ImmutableList.of(c), e);
final Set<CorrelationId> variablesSet =
RelOptUtil.getVariablesUsed(e.rel);
+ /* Only consider the correlated variables which originated from this
sub-query level*/
+ variablesSet.retainAll(filter.getVariablesSet());
Review Comment:
Interestingly, when I apply this fix in my local environment and run
`CoreQuidemTest`, I get a regression in `misc.iq` :
```
< +-----+---+
< | KEY | C |
< +-----+---+
< | 2 | 1 |
< +-----+---+
< (1 row)
<
---
> java.lang.AssertionError: Correlation variable $cor0 should be defined
> at
org.apache.calcite.adapter.enumerable.EnumerableRelImplementor.getCorrelVariableGetter(EnumerableRelImplementor.java:476)
> at
org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitFieldAccess(RexToLixTranslator.java:1393)
> at
org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitFieldAccess(RexToLixTranslator.java:101)
> at org.apache.calcite.rex.RexFieldAccess.accept(RexFieldAccess.java:94)
...
```
Not sure why this is not shown on the PR checks...
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
