You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/09/28 04:17:57 UTC
[GitHub] [calcite] jamesstarr opened a new pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
jamesstarr opened a new pull request #2549:
URL: https://github.com/apache/calcite/pull/2549
* Adding assertion to RelOptUtil.getVariablesUsed when a variable is
used out of context.
* Changing RelBuilderTest.testFilterWithCorrelationVariables which was
build an illegal relNode.
--
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
[GitHub] [calcite] chunweilei commented on pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
Posted by GitBox <gi...@apache.org>.
chunweilei commented on pull request #2549:
URL: https://github.com/apache/calcite/pull/2549#issuecomment-957035186
Could you merge the commits to one and rebase the PR, @jamesstarr ?
--
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
[GitHub] [calcite] chunweilei commented on pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
Posted by GitBox <gi...@apache.org>.
chunweilei commented on pull request #2549:
URL: https://github.com/apache/calcite/pull/2549#issuecomment-957035186
Could you merge the commits to one and rebase the PR, @jamesstarr ?
--
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
[GitHub] [calcite] jamesstarr commented on pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
Posted by GitBox <gi...@apache.org>.
jamesstarr commented on pull request #2549:
URL: https://github.com/apache/calcite/pull/2549#issuecomment-958147636
I am not sure why the test is not passing. It might be something wrong with the CI.
--
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
[GitHub] [calcite] zinking commented on a change in pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
Posted by GitBox <gi...@apache.org>.
zinking commented on a change in pull request #2549:
URL: https://github.com/apache/calcite/pull/2549#discussion_r732814403
##########
File path: core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
##########
@@ -3732,9 +3732,7 @@ private static RelNode groupIdRel(RelBuilder builder, boolean extra) {
+ "requiredColumns=[{5, 7}])\n"
+ " LogicalTableScan(table=[[scott, EMP]])\n"
+ " LogicalFilter(condition=[=($cor0.SAL, 1000)])\n"
- + " LogicalFilter(condition=[OR("
- + "SEARCH($cor0.DEPTNO, Sarg[(20..30)]), "
- + "IS NULL($2))], variablesSet=[[$cor0]])\n"
+ + " LogicalFilter(condition=[OR(SEARCH($cor0.DEPTNO, Sarg[(20..30)]), IS NULL($2))])\n"
Review comment:
interesting, it seems only Assertion clauses are added, why the plan is impacted after that?
--
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
[GitHub] [calcite] jamesstarr commented on a change in pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
Posted by GitBox <gi...@apache.org>.
jamesstarr commented on a change in pull request #2549:
URL: https://github.com/apache/calcite/pull/2549#discussion_r733131157
##########
File path: core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
##########
@@ -3732,9 +3732,7 @@ private static RelNode groupIdRel(RelBuilder builder, boolean extra) {
+ "requiredColumns=[{5, 7}])\n"
+ " LogicalTableScan(table=[[scott, EMP]])\n"
+ " LogicalFilter(condition=[=($cor0.SAL, 1000)])\n"
- + " LogicalFilter(condition=[OR("
- + "SEARCH($cor0.DEPTNO, Sarg[(20..30)]), "
- + "IS NULL($2))], variablesSet=[[$cor0]])\n"
+ + " LogicalFilter(condition=[OR(SEARCH($cor0.DEPTNO, Sarg[(20..30)]), IS NULL($2))])\n"
Review comment:
This test has changed. It was generating a non-sense plan before where the id was specified multiple times.
--
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
[GitHub] [calcite] jamesstarr commented on pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
Posted by GitBox <gi...@apache.org>.
jamesstarr commented on pull request #2549:
URL: https://github.com/apache/calcite/pull/2549#issuecomment-958147636
I am not sure why the test is not passing. It might be something wrong with the CI.
--
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
[GitHub] [calcite] chunweilei commented on pull request #2549: [CALCITE-4808] Adding assertion to RelOptUtil.getVariablesUsed
Posted by GitBox <gi...@apache.org>.
chunweilei commented on pull request #2549:
URL: https://github.com/apache/calcite/pull/2549#issuecomment-957035186
Could you merge the commits to one and rebase the PR, @jamesstarr ?
--
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