Просмотр исходного кода

HPCC-10240 Prevent a TABLE() aggregate being converted to a non aggregate

Switch to using a no_newaggregate much earlier in the tree normalizsation so
it will be treated as an aggregate - even if all fields are removed.

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 11 лет назад
Родитель
Сommit
1e4524f149

+ 0 - 2
ecl/hql/hqlatoms.cpp

@@ -66,7 +66,6 @@ IAtom * activeAtom;
 IAtom * activeFailureAtom;
 IAtom * activeNlpAtom;
 IAtom * afterAtom;
-IAtom * aggregateAtom;
 IAtom * algorithmAtom;
 IAtom * allAtom;
 IAtom * allocatorAtom;
@@ -472,7 +471,6 @@ MODULE_INIT(INIT_PRIORITY_HQLATOM)
     MAKEATOM(activeFailure);
     MAKEATOM(activeNlp);
     MAKEATOM(after);
-    MAKEATOM(aggregate);
     MAKEATOM(algorithm);
     MAKEATOM(all);
     MAKEATOM(allocator);

+ 0 - 1
ecl/hql/hqlatoms.hpp

@@ -68,7 +68,6 @@ extern HQL_API IAtom * activeAtom;
 extern HQL_API IAtom * activeFailureAtom;
 extern HQL_API IAtom * activeNlpAtom;
 extern HQL_API IAtom * afterAtom;
-extern HQL_API IAtom * aggregateAtom;
 extern HQL_API IAtom * allAtom;
 extern HQL_API IAtom * algorithmAtom;
 extern HQL_API IAtom * allocatorAtom;

+ 0 - 11
ecl/hql/hqlexpr.cpp

@@ -2696,20 +2696,9 @@ bool isAggregateDataset(IHqlExpression * expr)
     case no_aggregate:
     case no_newaggregate:
         return true;
-    case no_newusertable:
-        {
-            if (expr->hasAttribute(aggregateAtom))
-                return true;
-            IHqlExpression * grouping = expr->queryChild(3);
-            if (grouping && !grouping->isAttribute())
-                return true;
-            return expr->queryChild(2)->isGroupAggregateFunction();
-        }
     case no_selectfields:
     case no_usertable:
         {
-            if (expr->hasAttribute(aggregateAtom))
-                return true;
             IHqlExpression * grouping = expr->queryChild(2);
             if (grouping && !grouping->isAttribute())
                 return true;

+ 2 - 2
ecl/hql/hqlmeta.cpp

@@ -2604,7 +2604,7 @@ void calculateDatasetMeta(CHqlMetaInfo & meta, IHqlExpression * expr)
             else
             {
                 //Aggregation removes grouping
-                if (op == no_newaggregate || op == no_aggregate || (mapping && mapping->isGroupAggregateFunction()) || expr->hasAttribute(aggregateAtom))
+                if (op == no_newaggregate || op == no_aggregate || (mapping && mapping->isGroupAggregateFunction()))
                     meta.removeGroup();
             }
             //Now map any fields that we can.
@@ -3043,7 +3043,7 @@ ITypeInfo * calculateDatasetType(node_operator op, const HqlExprArray & parms)
             else
             {
                 //Aggregation removes grouping
-                if (op == no_newaggregate || op == no_aggregate || (mapping && mapping->isGroupAggregateFunction()) || hasAttribute(aggregateAtom, parms))
+                if (op == no_newaggregate || op == no_aggregate || (mapping && mapping->isGroupAggregateFunction()))
                     nowGrouped=false;
                 else
                     nowGrouped = isGrouped(datasetType);

+ 1 - 1
ecl/hql/hqlutil.cpp

@@ -4347,7 +4347,7 @@ extern HQL_API IHqlExpression * convertScalarAggregateToDataset(IHqlExpression *
     if (dataset->queryType()->getTypeCode() == type_groupedtable)
         dataset = createDataset(no_group, dataset, NULL);
 
-    IHqlExpression * project = createDataset(no_newusertable, dataset, createComma(aggregateRecord, transform, LINK(keyedAttr), LINK(prefetchAttr)));
+    IHqlExpression * project = createDataset(no_newaggregate, dataset, createComma(aggregateRecord, transform, LINK(keyedAttr), LINK(prefetchAttr)));
     return createRow(no_selectnth, project, createConstantOne());
 }
 

+ 3 - 2
ecl/hqlcpp/hqliproj.cpp

@@ -2392,8 +2392,9 @@ void ImplicitProjectTransformer::calculateFieldsUsed(IHqlExpression * expr)
                                     break;
                                 }
                             }
-                            assertex(match);
-                            processMatchingSelector(extra->outputFields, match, match->queryChild(0));
+                            //There may possibly be no match if it hasn't been normalized yet.
+                            if (match)
+                                processMatchingSelector(extra->outputFields, match, match->queryChild(0));
                         }
                     }
                     break;

+ 11 - 9
ecl/hqlcpp/hqlttcpp.cpp

@@ -1964,11 +1964,13 @@ IHqlExpression * ThorHqlTransformer::createTransformed(IHqlExpression * expr)
         break;
     case no_newusertable:
         normalized = normalizeTableGrouping(transformed);
-        if (!normalized)
-            normalized = normalizeTableToAggregate(expr, true);
         break;
     case no_newaggregate:
-        normalized = normalizePrefetchAggregate(transformed);
+        normalized = normalizeTableGrouping(transformed);
+        if (!normalized)
+            normalized = normalizeTableToAggregate(transformed, true);
+        if (!normalized || (normalized == transformed))
+            normalized = normalizePrefetchAggregate(transformed);
         break;
     case no_dedup:
         normalized = normalizeDedup(transformed);
@@ -2006,7 +2008,7 @@ IHqlExpression * ThorHqlTransformer::createTransformed(IHqlExpression * expr)
         return getDebugValueExpr(translator.wu(), expr);
     }
 
-    if (normalized)
+    if (normalized && (normalized != transformed))
     {
         transformed.setown(transform(normalized));
         normalized->Release();
@@ -3326,7 +3328,6 @@ static IHqlExpression * convertAggregateGroupingToGroupedAggregate(IHqlExpressio
     unwindChildren(args, expr);
     args.replace(*result.getClear(), 0);
     args.remove(3); // no longer grouped.
-    args.append(*createAttribute(aggregateAtom));
     return expr->clone(args);
 }
 
@@ -3525,8 +3526,6 @@ IHqlExpression * ThorHqlTransformer::normalizeTableToAggregate(IHqlExpression *
 
     HqlExprArray aggregateAttrs;
     unwindAttributes(aggregateAttrs, expr);
-    removeAttribute(aggregateAttrs, aggregateAtom);
-    removeAttribute(aggregateAttrs, fewAtom);
     if (!expr->hasAttribute(localAtom) && newGroupBy && !isGrouped(dataset) && isPartitionedForGroup(dataset, newGroupBy, true))
         aggregateAttrs.append(*createLocalAttribute());
 
@@ -3546,6 +3545,7 @@ IHqlExpression * ThorHqlTransformer::normalizeTableToAggregate(IHqlExpression *
         ret.setown(createDataset(no_newusertable, ret.getClear(), createComma(LINK(record), projectTransform)));
         ret.setown(expr->cloneAllAnnotations(ret));
     }
+
     return ret.getClear();
 }
 
@@ -3602,7 +3602,7 @@ IHqlExpression * ThorHqlTransformer::normalizeTableGrouping(IHqlExpression * exp
                 useHashAggregate = true;
         }
 
-        if (!expr->hasAttribute(aggregateAtom) && !useHashAggregate && !expr->hasAttribute(groupedAtom))
+        if (!useHashAggregate && !expr->hasAttribute(groupedAtom))
             return convertAggregateGroupingToGroupedAggregate(expr, group);
     }
     return NULL;
@@ -10562,7 +10562,9 @@ IHqlExpression * HqlTreeNormalizer::convertSelectToProject(IHqlExpression * newR
     unsigned numChildren = expr->numChildren();
     for (unsigned idx = 2; idx < numChildren; idx++)
         args.append(*transform(expr->queryChild(idx)));
-    OwnedHqlExpr project = createDataset(no_newusertable, args);
+
+    node_operator op = isAggregateDataset(expr) ? no_newaggregate : no_newusertable;
+    OwnedHqlExpr project = createDataset(op, args);
     return expr->cloneAllAnnotations(project);
 }
 

+ 3 - 0
ecl/regress/issue10240.ecl

@@ -0,0 +1,3 @@
+ds2 := DATASET(2, TRANSFORM({UNSIGNED line}, SELF.line := COUNTER));
+summary2 := TABLE(ds2, { COUNT(GROUP) }, LOCAL);
+COUNT(summary2) = 1;