Merge pull request #6626 from ghalliday/issue12515

HPCC-12515 no_datasetfromrow should always traverse input

Reviewed-By: Jamie Noss <james.noss@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/hql/hqlutil.cpp

@@ -1861,32 +1861,7 @@ unsigned getNumActivityArguments(IHqlExpression * expr)
             return 1;
         return 0;
     case no_datasetfromrow:
-        {
-            IHqlExpression * row = expr->queryChild(0);
-            //Is this special casing really the best way to handle this?  I'm not completely convinced.
-            loop
-            {
-                switch (row->getOperator())
-                {
-                case no_selectnth:
-                case no_split:
-                case no_spill:
-                    return 1;
-                case no_alias:
-                case no_alias_scope:
-                case no_nofold:
-                case no_nohoist:
-                case no_section:
-                case no_sectioninput:
-                case no_globalscope:
-                case no_thisnode:
-                    break;
-                default:
-                    return 0;
-                }
-                row = row->queryChild(0);
-            }
-        }
+    case no_projectrow:
     case no_fetch:
     case no_mapto:
     case no_evaluate:
@@ -2025,8 +2000,6 @@ bool isSourceActivity(IHqlExpression * expr, bool ignoreCompound)
     case no_compound_childgroupaggregate:
     case no_compound_inline:
         return !ignoreCompound;
-    case no_datasetfromrow:
-        return (getNumActivityArguments(expr) == 0);
     }
     return false;
 }
@@ -5274,7 +5247,7 @@ IHqlExpression * queryUncastExpr(IHqlExpression * expr)
     }
 }
 
-bool isSimpleTransformToMergeWith(IHqlExpression * expr, int & varSizeCount)
+static bool isSimpleTransformToMergeWith(IHqlExpression * expr, int & varSizeCount)
 {
     ForEachChild(i, expr)
     {
@@ -5319,6 +5292,11 @@ bool isSimpleTransformToMergeWith(IHqlExpression * expr)
     return isSimpleTransformToMergeWith(expr, varSizeCount) && varSizeCount < 3;
 }
 
+bool isSimpleTransform(IHqlExpression * expr)
+{
+    int varSizeCount = 0;
+    return isSimpleTransformToMergeWith(expr, varSizeCount);
+}
 
 
 bool isConstantDataset(IHqlExpression * expr)

ecl/hql/hqlutil.hpp

@@ -91,6 +91,7 @@ extern HQL_API unsigned getFieldCount(IHqlExpression * expr);
 extern HQL_API unsigned getFlatFieldCount(IHqlExpression * expr);
 extern HQL_API unsigned isEmptyRecord(IHqlExpression * record);
 extern HQL_API bool isTrivialSelectN(IHqlExpression * expr);
+
 extern HQL_API IHqlExpression * queryConvertChoosenNSort(IHqlExpression * expr, unsigned __int64 topNlimit);
 
 extern HQL_API IHqlExpression * queryAttributeChild(IHqlExpression * expr, IAtom * name, unsigned idx);
@@ -186,6 +187,7 @@ extern HQL_API bool isConstantTransform(IHqlExpression * transform);
 extern HQL_API bool isConstantDataset(IHqlExpression * expr);
 extern HQL_API bool isConstantDictionary(IHqlExpression * expr);
 extern HQL_API bool isSimpleTransformToMergeWith(IHqlExpression * expr);
+extern HQL_API bool isSimpleTransform(IHqlExpression * expr);
 extern HQL_API IHqlExpression * queryUncastExpr(IHqlExpression * expr);
 extern HQL_API bool areConstant(const HqlExprArray & args);
 extern HQL_API bool getFoldedConstantText(StringBuffer& ret, IHqlExpression * expr);

ecl/hqlcpp/hqlcpp.ipp

@@ -1235,6 +1235,7 @@ public:
     void doBuildDatasetLimit(BuildCtx & ctx, IHqlExpression * expr, CHqlBoundExpr & tgt, ExpressionFormat format);
     bool doBuildDatasetInlineTable(BuildCtx & ctx, IHqlExpression * expr, CHqlBoundExpr & tgt, ExpressionFormat format);
     bool doBuildDictionaryInlineTable(BuildCtx & ctx, IHqlExpression * expr, CHqlBoundExpr & tgt, ExpressionFormat format);
+    void doBuildDatasetNull(IHqlExpression * expr, CHqlBoundExpr & tgt, ExpressionFormat format);
 
     void doBuildCheckDatasetLimit(BuildCtx & ctx, IHqlExpression * expr, const CHqlBoundExpr & bound);
 

ecl/hqlcpp/hqlcppds.cpp

@@ -2066,20 +2066,8 @@ void HqlCppTranslator::doBuildDataset(BuildCtx & ctx, IHqlExpression * expr, CHq
         doBuildStmtFail(ctx, expr->queryChild(1));
         //fallthrough
     case no_null:
-        {
-            tgt.count.setown(getSizetConstant(0));
-            tgt.length.setown(getSizetConstant(0));
-            IHqlExpression * record = expr->queryRecord();
-            Owned<ITypeInfo> type;
-            if (expr->isDictionary())
-                type.setown(makeDictionaryType(makeRowType(record->getType())));
-            else
-                type.setown(makeTableType(makeRowType(record->getType())));
-            if ((format == FormatLinkedDataset) || (format == FormatArrayDataset) || expr->isDictionary())
-                type.setown(setLinkCountedAttr(type, true));
-            tgt.expr.setown(createValue(no_nullptr, makeReferenceModifier(type.getClear())));
-            return;
-        }
+        doBuildDatasetNull(expr, tgt, format);
+        return;
     case no_translated:
         expandTranslated(expr, tgt);
         return;
@@ -2761,6 +2749,22 @@ void HqlCppTranslator::doBuildDatasetLimit(BuildCtx & ctx, IHqlExpression * expr
     doBuildCheckDatasetLimit(ctx, expr, tgt);
 }
 
+void HqlCppTranslator::doBuildDatasetNull(IHqlExpression * expr, CHqlBoundExpr & tgt, ExpressionFormat format)
+{
+     tgt.count.setown(getSizetConstant(0));
+     tgt.length.setown(getSizetConstant(0));
+     IHqlExpression * record = expr->queryRecord();
+     Owned<ITypeInfo> type;
+     if (expr->isDictionary())
+         type.setown(makeDictionaryType(makeRowType(record->getType())));
+     else
+         type.setown(makeTableType(makeRowType(record->getType())));
+     if ((format == FormatLinkedDataset) || (format == FormatArrayDataset) || expr->isDictionary())
+         type.setown(setLinkCountedAttr(type, true));
+     tgt.expr.setown(createValue(no_nullptr, makeReferenceModifier(type.getClear())));
+}
+
+
 class ConstantRow : public CInterface
 {
 public:

ecl/hqlcpp/hqlcpputil.cpp

@@ -227,6 +227,8 @@ bool isSelectSortedTop(IHqlExpression * selectExpr)
     return false;
 }
 
+//---------------------------------------------------------------------------
+
 ITypeInfo * makeRowReferenceType(IHqlExpression * ds)
 {
     ITypeInfo * recordType = ds ? LINK(ds->queryRecordType()) : NULL;
@@ -453,3 +455,32 @@ bool mustInitializeField(IHqlExpression * field)
         return true;
     return false;
 }
+
+bool worthGeneratingRowAsSingleActivity(IHqlExpression * expr)
+{
+    loop
+    {
+        switch (expr->getOperator())
+        {
+        case no_left:
+        case no_right:
+        case no_activerow:
+        case no_createrow:
+        case no_getresult:
+            return true;
+        case no_select:
+            if (!isNewSelector(expr))
+                return true;
+            break;
+        case no_alias:
+        case no_projectrow:
+            break;
+        case no_if:
+            return worthGeneratingRowAsSingleActivity(expr->queryChild(1)) && worthGeneratingRowAsSingleActivity(expr->queryChild(2));
+        default:
+            //Do not generate no_getgraph result - better as separate activities
+            return false;
+        }
+        expr = expr->queryChild(0);
+    }
+}

ecl/hqlcpp/hqlcpputil.hpp

@@ -52,6 +52,7 @@ extern IHqlExpression * projectCreateSetDataset(IHqlExpression * createsetExpr);
 extern IHqlExpression * mapInternalFunctionParameters(IHqlExpression * expr);
 
 extern bool mustInitializeField(IHqlExpression * field);
+extern bool worthGeneratingRowAsSingleActivity(IHqlExpression * expr);
 
 //Common types and expressions...
 extern ITypeInfo * boolType;

ecl/hqlcpp/hqlhtcpp.cpp

@@ -6336,6 +6336,8 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
             case no_externalcall:
                 if (expr->isAction())
                     result = doBuildActivityAction(ctx, expr, isRoot);
+                else if (expr->isDatarow())
+                    result = doBuildActivityCreateRow(ctx, expr, false);
                 else if (hasStreamedModifier(expr->queryType()))
                 {
                     result = doBuildActivityStreamedCall(ctx, expr);
@@ -6350,14 +6352,17 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
             case no_left:
             case no_right:
             case no_top:
+            case no_activetable:
             case no_id2blob:
             case no_typetransfer:
             case no_rows:
             case no_xmlproject:
             case no_libraryinput:
-            case no_activetable:
             case no_translated:
-                result = doBuildActivityChildDataset(ctx, expr);
+                if (expr->isDatarow())
+                    result = doBuildActivityCreateRow(ctx, expr, false);
+                else
+                    result = doBuildActivityChildDataset(ctx, expr);
                 break;
             case no_compound_inline:
                 result = doBuildActivityChildDataset(ctx, expr->queryChild(0));
@@ -6402,7 +6407,7 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
             case no_datasetfromrow:
                 {
                     OwnedHqlExpr row = expr->cloneAllAnnotations(expr->queryChild(0));  // preserve any position information....
-                    if ((getNumActivityArguments(expr) == 0) && canProcessInline(&ctx, row) && (row->getOperator() != no_getgraphresult))
+                    if (worthGeneratingRowAsSingleActivity(row))
                         result = doBuildActivityCreateRow(ctx, row, false);
                     else
                         result = buildCachedActivity(ctx, row);
@@ -6735,10 +6740,8 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
                 result = doBuildActivitySequentialParallel(ctx, expr, isRoot);
                 break;
             case no_activerow:
-                {
-                    OwnedHqlExpr row = createDatasetFromRow(LINK(expr));
-                    return buildCachedActivity(ctx, row);
-                }
+                result = doBuildActivityCreateRow(ctx, expr, false);
+                break;
             case no_assert_ds:
                 result = doBuildActivityAssert(ctx, expr);
                 break;
@@ -6782,8 +6785,7 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
                     return doBuildActivityAction(ctx, expr, isRoot);
                 if (expr->isDatarow())
                 {
-                    OwnedHqlExpr row = createDatasetFromRow(LINK(expr));
-                    return buildCachedActivity(ctx, row);
+                    return doBuildActivityCreateRow(ctx, expr, false);
                 }
                 else
                 {
@@ -16779,7 +16781,6 @@ ABoundActivity * HqlCppTranslator::doBuildActivityTempTable(BuildCtx & ctx, IHql
     return instance->getBoundActivity();
 }
 
-
 //NB: Also used to create row no_null as an activity.
 ABoundActivity * HqlCppTranslator::doBuildActivityCreateRow(BuildCtx & ctx, IHqlExpression * expr, bool isDataset)
 {
@@ -16837,7 +16838,11 @@ ABoundActivity * HqlCppTranslator::doBuildActivityCreateRow(BuildCtx & ctx, IHql
     if (isDataset)
         associateSkipReturnMarker(funcctx, queryBoolExpr(false), selfCursor);
 
-    buildAssign(funcctx, self, expr);
+    LinkedHqlExpr cseExpr = expr;
+    if (options.spotCSE)
+        cseExpr.setown(spotScalarCSE(cseExpr, NULL, options.spotCseInIfDatasetConditions));
+
+    buildAssign(funcctx, self, cseExpr);
     buildReturnRecordSize(funcctx, selfCursor);
 
     doBuildTempTableFlags(instance->startctx, expr, valuesAreConstant);

ecl/hqlcpp/hqlinline.cpp

@@ -320,9 +320,15 @@ static unsigned calcInlineFlags(BuildCtx * ctx, IHqlExpression * expr)
         return RETevaluate;
     case no_translated:
         return RETassign|RETiterate|RETevaluate;
+    case no_projectrow:
+        {
+            unsigned childFlags = getInlineFlags(ctx, expr->queryChild(0));
+            if (childFlags == 0)
+                return 0;
+            return RETevaluate;
+        }
     case no_null:
     case no_temprow:
-    case no_projectrow:
     case no_left:
     case no_right:
     case no_top:
@@ -741,19 +747,7 @@ GraphLocalisation queryActivityLocalisation(IHqlExpression * expr, bool optimize
         ///    return GraphNoAccess;
         return GraphCoLocal;
     case no_datasetfromrow:
-        {
-            if (getNumActivityArguments(expr) != 0)
-                return GraphNeverAccess;
-
-            IHqlExpression * row = expr->queryChild(0);
-            switch (row->getOperator())
-            {
-            case no_createrow:
-            case no_null:
-                return queryActivityLocalisation(row, optimizeParentAccess);
-            }
-            break;
-        }
+        return GraphNeverAccess;
     case no_workunit_dataset:
         return GraphCoLocal; // weird exception in roxie
     case no_getgraphresult:
@@ -867,6 +861,12 @@ static GraphLocalisation doGetGraphLocalisation(IHqlExpression * expr, bool opti
     return ret;
 }
 
+static GraphLocalisation getGraphLocalisation(IHqlExpression * expr, bool optimizeParentAccess)
+{
+    TransformMutexBlock lock;
+    return doGetGraphLocalisation(expr, optimizeParentAccess);
+}
+
 bool HqlCppTranslator::isAlwaysCoLocal()
 {
     return targetHThor();
@@ -878,8 +878,7 @@ GraphLocalisation HqlCppTranslator::getGraphLocalisation(IHqlExpression * expr,
     if (targetThor() && !isInsideChildQuery)
         return GraphNonLocal;
 
-    TransformMutexBlock lock;
-    return doGetGraphLocalisation(expr, options.optimizeParentAccess);
+    return ::getGraphLocalisation(expr, options.optimizeParentAccess);
 }
 
 bool HqlCppTranslator::isNeverDistributed(IHqlExpression * expr)

ecl/hqlcpp/hqliproj.cpp

@@ -1354,13 +1354,13 @@ void ImplicitProjectTransformer::analyseExpr(IHqlExpression * expr)
             gatherFieldsUsed(expr, extra);
             return;
         case no_activerow:
-            assertex(extra->activityKind() == SimpleActivity);
-            allowActivity = false;
-            Parent::analyseExpr(expr);
-            allowActivity = true;
-            activities.append(*LINK(expr));
-            gatherFieldsUsed(expr, extra);
-            return;
+            {
+                assertex(extra->activityKind() == SourceActivity);
+                allowActivity = false;
+                Parent::analyseExpr(expr);
+                allowActivity = true;
+                break;
+            }
         case no_attr:
         case no_attr_expr:
         case no_attr_link:
@@ -1940,7 +1940,7 @@ ProjectExprKind ImplicitProjectTransformer::getProjectExprKind(IHqlExpression *
             return ScalarSelectActivity;
         return NonActivity;
     case no_activerow:
-        return SimpleActivity;
+        return SourceActivity;
     case no_attr:
     case no_attr_expr:
     case no_attr_link:
@@ -2137,8 +2137,6 @@ ProjectExprKind ImplicitProjectTransformer::getProjectExprKind(IHqlExpression *
     case no_keypatch:
         return NonActivity;
     case no_datasetfromrow:
-        if (getNumActivityArguments(expr) == 0)
-            return SourceActivity;
         return SimpleActivity;
     case no_newtransform:
     case no_transform:

ecl/hqlcpp/hqlresource.cpp

@@ -2648,6 +2648,7 @@ void EclResourcer::gatherChildSplitPoints(IHqlExpression * expr, ResourcerInfo *
 
     //If child queries are supported then don't hoist the expressions if they might only be evaluated once
     //because they may be conditional
+    bool alwaysOnce = false;
     switch (expr->getOperator())
     {
     case no_setresult:
@@ -2655,6 +2656,11 @@ void EclResourcer::gatherChildSplitPoints(IHqlExpression * expr, ResourcerInfo *
         //set results, only done once=>don't hoist conditionals
         locator.findSplitPoints(expr, last, max, true, true);
         return;
+    case no_createrow:
+    case no_datasetfromrow:
+    case no_projectrow:
+        alwaysOnce = true;
+        break;
     case no_loop:
         if ((options.targetClusterType == ThorLCRCluster) && !options.isChildQuery)
         {
@@ -2670,7 +2676,7 @@ void EclResourcer::gatherChildSplitPoints(IHqlExpression * expr, ResourcerInfo *
         break;
     }
     locator.findSplitPoints(expr, 0, first, true, true);          // IF() conditions only evaluated once... => don't force
-    locator.findSplitPoints(expr, last, max, true, false);
+    locator.findSplitPoints(expr, last, max, true, alwaysOnce);
 }
 
 
@@ -4890,7 +4896,7 @@ bool EclResourcer::optimizeAggregate(IHqlExpression * expr)
         return false;
 
     ResourcerInfo * selectNthInfo = queryResourceInfo(selectNth);
-    if (selectNthInfo->numExternalUses)
+    if (!selectNthInfo || selectNthInfo->numExternalUses)
         return false;
 
     IHqlExpression * aggregate = selectNth->queryChild(0);      // no_newaggregate