Bug:#85204 Removes exponential searches from eclcc

Replaces a few situations where an expression was being walked as
a tree rather than a DAG.  This caused eclcc to take a very long time
to process  complicated grouping conditions.  The new code ensures
children of a common graph node are only visited once.

Not high priority fix since it is a very unusual condition to hit
(this was hit in unusual auto-generated code.)

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ecl/hql/hqlgram2.cpp

@@ -6976,54 +6976,96 @@ void HqlGram::inheritRecordMaxLength(IHqlExpression * dataset, OwnedHqlExpr & re
 }
 
 
-bool checkGroupExpression(HqlExprArray &groups, IHqlExpression *field)
+static HqlTransformerInfo groupExpressionCheckerInfo("GroupExpressionChecker");
+class GroupExpressionChecker : public QuickHqlTransformer
 {
-    if (field->isAggregate() || field->isConstant()) 
-        return true;
-
-    if (groups.find(*field) != NotFound)
-        return true;
-    
-    // No dice - check kids
-    switch (field->getOperator())
+public:
+    GroupExpressionChecker(const HqlExprArray & _groups)
+        : QuickHqlTransformer(groupExpressionCheckerInfo, NULL)
     {
-    case no_select:
-        return false;
+        ok = true;
+        ForEachItemIn(i, _groups)
+            groups.append(*_groups.item(i).queryBody());
     }
-    
-    ForEachChild(idx, field)
+
+    virtual void doAnalyseBody(IHqlExpression * expr)
     {
-        if (!checkGroupExpression(groups, field->queryChild(idx)))
-            return false;
+        if (!ok)
+            return;
+
+        if (expr->isAggregate() || expr->isConstant())
+            return;
+
+        if (groups.contains(*expr))
+            return;
+
+        // No dice - check kids
+        switch (expr->getOperator())
+        {
+        case no_select:
+            ok = false;
+            return;
+        }
+
+        QuickHqlTransformer::doAnalyseBody(expr);
     }
-    return true;
+
+    bool isOk() const { return ok; }
+
+protected:
+    HqlExprCopyArray groups;
+    bool ok;
+};
+
+
+
+bool checkGroupExpression(HqlExprArray &groups, IHqlExpression *field)
+{
+    GroupExpressionChecker checker(groups);
+    checker.analyse(field);
+    return checker.isOk();
 }
 
-static IHqlExpression * normalizeSelects(IHqlExpression * expr)
+
+static HqlTransformerInfo quickSelectNormalizerInfo("QuickSelectNormalizer");
+class HQL_API QuickSelectNormalizer : public QuickHqlTransformer
 {
-    switch (expr->getOperator())
+public:
+    QuickSelectNormalizer() : QuickHqlTransformer(quickSelectNormalizerInfo, NULL)
     {
-    case no_constant:
-    case no_colon:
-    case no_cluster:
-        return LINK(expr);
-    case no_globalscope:
-        if (!expr->hasProperty(optAtom))
+    }
+
+    virtual IHqlExpression * createTransformed(IHqlExpression * expr)
+    {
+        switch (expr->getOperator())
+        {
+        case no_constant:
+        case no_colon:
+        case no_cluster:
+            return LINK(expr);
+        case no_globalscope:
+            if (!expr->hasProperty(optAtom))
+                return LINK(expr);
+            break;
+        case NO_AGGREGATE:
+            //These aren't strictly correct, but will only go wrong if the same
+            //obscure expression is written out twice differently.
+            //you really should be projecting or using the same attribute
             return LINK(expr);
-        break;
-    case NO_AGGREGATE:
-        //These aren't strictly correct, but will only go wrong if the same
-        //obscure expression is written out twice differently.
-        //you really should be projecting or using the same attribute
-        return LINK(expr);
 
-    case no_select:
-        return LINK(expr->queryNormalizedSelector());
+        case no_select:
+            return LINK(expr->queryNormalizedSelector());
+        }
+        return QuickHqlTransformer::createTransformed(expr);
     }
-    HqlExprArray args;
-    ForEachChild(i, expr)
-        args.append(*normalizeSelects(expr->queryChild(i)));
-    return cloneOrLink(expr, args);
+};
+
+
+
+static IHqlExpression * normalizeSelects(IHqlExpression * expr)
+{
+    QuickSelectNormalizer transformer;
+    return transformer.transform(expr);
 }
 
 

ecl/hqlcpp/hqlttcpp.cpp

@@ -1421,7 +1421,9 @@ static IHqlExpression * createArith(node_operator op, ITypeInfo * type, IHqlExpr
 
 
 
-IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpression * expr, HqlExprArray & fields, HqlExprArray & assigns, bool & extraSelectNeeded, bool canOptimizeCasts)
+//MORE: This might be better as a class, it would reduce the number of parameters
+IHqlExpression * doNormalizeAggregateExpr(IHqlExpression * selector, IHqlExpression * expr, HqlExprArray & fields, HqlExprArray & assigns, bool & extraSelectNeeded, bool canOptimizeCasts);
+IHqlExpression * evalNormalizeAggregateExpr(IHqlExpression * selector, IHqlExpression * expr, HqlExprArray & fields, HqlExprArray & assigns, bool & extraSelectNeeded, bool canOptimizeCasts)
 {
     switch (expr->getOperator())
     {
@@ -1438,7 +1440,7 @@ IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpressio
 
             //average should be done as a real operation I think, possibly decimal, if argument is decimal
             OwnedHqlExpr avg = createArith(no_div, exprType, sum, count);
-            return normalizeAggregateExpr(selector, avg, fields, assigns, extraSelectNeeded, false);
+            return doNormalizeAggregateExpr(selector, avg, fields, assigns, extraSelectNeeded, false);
         }
     case no_vargroup:
         //Map this to (sum(x^2)-sum(x)^2/count())/count()
@@ -1457,7 +1459,7 @@ IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpressio
             OwnedHqlExpr n2 = createArith(no_div, exprType, n1, count);
             OwnedHqlExpr n3 = createArith(no_sub, exprType, sumxx, n2);
             OwnedHqlExpr n4 = createArith(no_div, exprType, n3, count);
-            return normalizeAggregateExpr(selector, n4, fields, assigns, extraSelectNeeded, false);
+            return doNormalizeAggregateExpr(selector, n4, fields, assigns, extraSelectNeeded, false);
         }
     case no_covargroup:
         //Map this to (sum(x.y)-sum(x).sum(y)/count())/count()
@@ -1478,7 +1480,7 @@ IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpressio
             OwnedHqlExpr n2 = createArith(no_div, exprType, n1, count);
             OwnedHqlExpr n3 = createArith(no_sub, exprType, sumxy, n2);
             OwnedHqlExpr n4 = createArith(no_div, exprType, n3, count);
-            return normalizeAggregateExpr(selector, n4, fields, assigns, extraSelectNeeded, false);
+            return doNormalizeAggregateExpr(selector, n4, fields, assigns, extraSelectNeeded, false);
         }
     case no_corrgroup:
         //Map this to (covar(x,y)/(var(x).var(y)))
@@ -1514,7 +1516,7 @@ IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpressio
             OwnedHqlExpr n10 = createArith(no_mul, exprType, n6, n9);
             OwnedHqlExpr n11 = createValue(no_sqrt, LINK(exprType), LINK(n10));
             OwnedHqlExpr n12 = createArith(no_div, exprType, n3, n11);
-            return normalizeAggregateExpr(selector, n12, fields, assigns, extraSelectNeeded, false);
+            return doNormalizeAggregateExpr(selector, n12, fields, assigns, extraSelectNeeded, false);
         }
         throwUnexpected();
 
@@ -1576,7 +1578,7 @@ IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpressio
             IHqlExpression * child = expr->queryChild(0);
             if (expr->queryType()->getTypeCode() == child->queryType()->getTypeCode())
             {
-                IHqlExpression * ret = normalizeAggregateExpr(selector, child, fields, assigns, extraSelectNeeded, false);
+                IHqlExpression * ret = doNormalizeAggregateExpr(selector, child, fields, assigns, extraSelectNeeded, false);
                 //This should be ret==child
                 if (ret == selector)
                     return ret;
@@ -1596,7 +1598,7 @@ IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpressio
             for (idx = 0; idx < max; idx++)
             {
                 IHqlExpression * child = expr->queryChild(idx);
-                IHqlExpression * changed = normalizeAggregateExpr(NULL, child, fields, assigns, extraSelectNeeded, false);
+                IHqlExpression * changed = doNormalizeAggregateExpr(NULL, child, fields, assigns, extraSelectNeeded, false);
                 args.append(*changed);
                 if (child != changed)
                     diff = true;
@@ -1608,6 +1610,23 @@ IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpressio
     }
 }
 
+IHqlExpression * doNormalizeAggregateExpr(IHqlExpression * selector, IHqlExpression * expr, HqlExprArray & fields, HqlExprArray & assigns, bool & extraSelectNeeded, bool canOptimizeCasts)
+{
+    IHqlExpression * match = static_cast<IHqlExpression *>(expr->queryTransformExtra());
+    if (match)
+        return LINK(match);
+    IHqlExpression * ret = evalNormalizeAggregateExpr(selector, expr, fields, assigns, extraSelectNeeded, canOptimizeCasts);
+    expr->setTransformExtra(ret);
+    return ret;
+}
+
+
+IHqlExpression * normalizeAggregateExpr(IHqlExpression * selector, IHqlExpression * expr, HqlExprArray & fields, HqlExprArray & assigns, bool & extraSelectNeeded, bool canOptimizeCasts)
+{
+    TransformMutexBlock block;
+    return doNormalizeAggregateExpr(selector, expr, fields, assigns, extraSelectNeeded, canOptimizeCasts);
+}
+
 
 //---------------------------------------------------------------------------