[CALCITE-2927] The javadoc and implement of RuleQueue.computeImportance() is inconsistent

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 1.18.0
• Fix Version/s: 1.20.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Unassigned
• Reporter: Matt Wang
• Created: 18/Mar/19 02:13
• Updated: 24/Jun/19 15:01
• Resolved: 28/Mar/19 04:56

Description

In the javadoc of computeImportance(), it shows the importance of node is Sum{computeImportanceOfChild()}, but in the implementation it shows Max{computeImportanceOfChild()}, this is inconsistent. The Javadoc has some errors, it will cause some confusion.

 

// /**
 * Computes the <dfn>importance</dfn> of a node. Importance is defined as
 * follows:
 *
 * <ul>
 * <li>the root {@link RelSubset} has an importance of 1</li>
 * <li>the importance of any other subset is the sum of its importance to
 * its parents</li>
 * <li>The importance of children is pro-rated according to the cost of the
 * children. Consider a node which has a cost of 3, and children with costs
 * of 2 and 5. The total cost is 10. If the node has an importance of .5,
 * then the children will have importance of .1 and .25. The retains .15
 * importance points, to reflect the fact that work needs to be done on the
 * node's algorithm.</li>
 * </ul>
 *
 * <p>The formula for the importance <i>I</i> of node n is:
 *
 * <blockquote>I<sub>n</sub> = Sum<sub>parents p of n</sub>{I<sub>p</sub> .
 * W <sub>n, p</sub>}</blockquote>
 *
 * <p>where W<sub>n, p</sub>, the weight of n within its parent p, is
 *
 * <blockquote>W<sub>n, p</sub> = Cost<sub>n</sub> / (SelfCost<sub>p</sub> +
 * Cost<sub>n0</sub> + ... + Cost<sub>nk</sub>)
 * </blockquote>
 */
double computeImportance(RelSubset subset) {
  double importance;
  if (subset == planner.root) {
    // The root always has importance = 1
    importance = 1.0;
  } else {
    final RelMetadataQuery mq = subset.getCluster().getMetadataQuery();

    // The importance of a subset is the max of its importance to its
    // parents
    importance = 0.0;
    for (RelSubset parent : subset.getParentSubsets(planner)) {
      final double childImportance =
          computeImportanceOfChild(mq, subset, parent);
      importance = Math.max(importance, childImportance);
    }
  }
  LOGGER.trace("Importance of [{}] is {}", subset, importance);
  return importance;
}

 

 

Issue Links

• links to: GitHub Pull Request #1111