hsyuan commented on a change in pull request #1840: [CALCITE-3753] Remove rule 
queue importance
URL: https://github.com/apache/calcite/pull/1840#discussion_r387938813
 
 

 ##########
 File path: 
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
 ##########
 @@ -87,36 +85,11 @@
  * according to a dynamic programming algorithm.
  */
 public class VolcanoPlanner extends AbstractRelOptPlanner {
-  protected static final double COST_IMPROVEMENT = .5;
 
   //~ Instance fields --------------------------------------------------------
 
   protected RelSubset root;
 
-  /**
-   * If true, the planner keeps applying rules as long as they continue to
-   * reduce the cost. If false, the planner terminates as soon as it has found
-   * any implementation, no matter how expensive.
-   */
-  protected boolean ambitious = true;
-
-  /**
-   * If true, and if {@link #ambitious} is true, the planner waits a finite
-   * number of iterations for the cost to improve.
-   *
-   * <p>The number of iterations K is equal to the number of iterations
-   * required to get the first finite plan. After the first finite plan, it
-   * continues to fire rules to try to improve it. The planner sets a target
-   * cost of the current best cost multiplied by {@link #COST_IMPROVEMENT}. If
-   * it does not meet that cost target within K steps, it quits, and uses the
-   * current best plan. If it meets the cost, it sets a new, lower target, and
-   * has another K iterations to meet it. And so forth.
-   *
-   * <p>If false, the planner continues to fire rules until the rule queue is
-   * empty.
-   */
-  protected boolean impatient = false;
 
 Review comment:
   It is easy to fix. Don't follow Calcite's way, it is doing wrong. 
   - Transformation rule matches only logical nodes, not physical nodes.
   - Create your rule to match AbstractConverter to convert to your own 
physical sort/distribute nodes.
   - Customize your own convention to use abstract converter only when both 
from and to are physical convention.
   
   If your system is not a federated system, limit your convention to 2: NONE 
and Physical one.
   
   But since you already modified the code, you can still add it back in your 
own codebase even I removed this.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to