drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From amansinha100 <...@git.apache.org>
Subject [GitHub] drill pull request #889: DRILL-5691: enhance scalar sub queries checking for...
Date Wed, 06 Sep 2017 15:06:42 GMT
Github user amansinha100 commented on a diff in the pull request:

    https://github.com/apache/drill/pull/889#discussion_r137291711
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
---
    @@ -203,35 +203,27 @@ public static void addLeastRestrictiveCasts(LogicalExpression[]
leftExpressions,
       }
     
       /**
    -   * Utility method to check if a subquery (represented by its root RelNode) is provably
scalar. Currently
    -   * only aggregates with no group-by are considered scalar. In the future, this method
should be generalized
    -   * to include more cases and reconciled with Calcite's notion of scalar.
    +   * Utility method to check if a subquery (represented by its root RelNode) is provably
scalar.
        * @param root The root RelNode to be examined
        * @return True if the root rel or its descendant is scalar, False otherwise
        */
       public static boolean isScalarSubquery(RelNode root) {
    -    DrillAggregateRel agg = null;
    -    RelNode currentrel = root;
    -    while (agg == null && currentrel != null) {
    -      if (currentrel instanceof DrillAggregateRel) {
    -        agg = (DrillAggregateRel)currentrel;
    -      } else if (currentrel instanceof RelSubset) {
    -        currentrel = ((RelSubset)currentrel).getBest() ;
    -      } else if (currentrel.getInputs().size() == 1) {
    -        // If the rel is not an aggregate or RelSubset, but is a single-input rel (could
be Project,
    -        // Filter, Sort etc.), check its input
    -        currentrel = currentrel.getInput(0);
    -      } else {
    -        break;
    -      }
    -    }
    -
    -    if (agg != null) {
    -      if (agg.getGroupSet().isEmpty()) {
    -        return true;
    +    RelMetadataQuery relMetadataQuery = RelMetadataQuery.instance();
    +    RelNode currentRel = root;
    +    for (; ; ) {
    +      if (currentRel instanceof RelSubset) {
    +        currentRel = ((RelSubset) currentRel).getBest();
    +      } else if (currentRel != null) {
    +        Double rowCount = relMetadataQuery.getRowCount(currentRel);
    --- End diff --
    
    getRowCount() is not correct.  Pls see my prior comment on using the RelMdMaxRowCount.getMaxRowCount()
 APIs.   The reason is getRowCount() will give an estimate which may not match the actual
run-time value, whereas the getMaxRowCount() is an assertion by the optimizer that row count
cannot exceed a number N (in your case 1).  


---

Mime
View raw message