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/02/11 17:32:58 UTC

[GitHub] [calcite] aigor opened a new pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence

aigor opened a new pull request #2347:
URL: https://github.com/apache/calcite/pull/2347


   Improve planning performance with:
   - RelSubset check for Rel presence
   - Better strategy for building message when checking for nulls 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] aigor commented on pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence (Igor Lozynskyi)

Posted by GitBox <gi...@apache.org>.
aigor commented on pull request #2347:
URL: https://github.com/apache/calcite/pull/2347#issuecomment-778122725


   > There's one more instance of `getRelList().contains` in `matchRecurse`. Would you please update it to the faster API?
   > 
   > ```java
   >             List<RelNode> inputRels = input.getRelList();
   >             if (!inputRels.contains(previous)) {
   >               continue;
   >             }
   > ```
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] aigor edited a comment on pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence (Igor Lozynskyi)

Posted by GitBox <gi...@apache.org>.
aigor edited a comment on pull request #2347:
URL: https://github.com/apache/calcite/pull/2347#issuecomment-778122725


   > There's one more instance of `getRelList().contains` in `matchRecurse`. Would you please update it to the faster API?
   > 
   > ```java
   >             List<RelNode> inputRels = input.getRelList();
   >             if (!inputRels.contains(previous)) {
   >               continue;
   >             }
   > ```
   
   done


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] zabetak closed pull request #2347: [CALCITE-4494] Improve planning performance with RelSubset check for Rel presence (Igor Lozynskyi)

Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #2347:
URL: https://github.com/apache/calcite/pull/2347


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] aigor closed pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence (Igor Lozynskyi)

Posted by GitBox <gi...@apache.org>.
aigor closed pull request #2347:
URL: https://github.com/apache/calcite/pull/2347


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] vlsi commented on pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence

Posted by GitBox <gi...@apache.org>.
vlsi commented on pull request #2347:
URL: https://github.com/apache/calcite/pull/2347#issuecomment-778114563


   There's one more instance of `getRelList().contains` in `matchRecurse`. Would you please update it to the faster API?
   
   ```java
               List<RelNode> inputRels = input.getRelList();
               if (!inputRels.contains(previous)) {
                 continue;
               }
   ```


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] julianhyde commented on a change in pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence

Posted by GitBox <gi...@apache.org>.
julianhyde commented on a change in pull request #2347:
URL: https://github.com/apache/calcite/pull/2347#discussion_r574724664



##########
File path: core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
##########
@@ -433,6 +433,13 @@ RelNode buildCheapestPlan(VolcanoPlanner planner) {
     return list;
   }
 
+  /**
+   * Checks whether the rel is a part of the current subset.
+   */
+  public boolean containsRel(RelNode node) {

Review comment:
       change the name to `contains` rather than `containsRel`?
   
   Change the javadoc to 'Returns whether' rather than 'Checks whether'. (Often methods that 'check' return void, and throw if the condition is violated.)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] aigor commented on a change in pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence

Posted by GitBox <gi...@apache.org>.
aigor commented on a change in pull request #2347:
URL: https://github.com/apache/calcite/pull/2347#discussion_r574741090



##########
File path: core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
##########
@@ -433,6 +433,13 @@ RelNode buildCheapestPlan(VolcanoPlanner planner) {
     return list;
   }
 
+  /**
+   * Checks whether the rel is a part of the current subset.
+   */
+  public boolean containsRel(RelNode node) {

Review comment:
       done




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] aigor commented on pull request #2347: CALCITE-4494 Improve planning performance with RelSubset check for Rel presence (Igor Lozynskyi)

Posted by GitBox <gi...@apache.org>.
aigor commented on pull request #2347:
URL: https://github.com/apache/calcite/pull/2347#issuecomment-778124395


   Sorry, accidentally closed after adding one more commit


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org