HPCC-21587 Fix problem with transformation of selectors

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

ecl/hql/hqlexpr.cpp

@@ -6343,10 +6343,7 @@ CHqlRow::CHqlRow(node_operator op, ITypeInfo * type, HqlExprArray & _ownedOperan
     switch (op)
     {
     case no_select: 
-        if (!hasAttribute(newAtom))
-        {
-            normalized.setown(calcNormalizedSelector());
-        }
+        normalized.setown(calcNormalizedSelector());
         break;
     }
 
@@ -13132,14 +13129,6 @@ inline IHqlExpression * normalizeSelectLhs(IHqlExpression * lhs, bool & isNew)
             if (isNew && isAlwaysActiveRow(lhs))
                 isNew = false;
             return lhs;
-        case no_selectnth:
-        case no_createrow:
-            assertex(isAlwaysNewRow(lhs));
-            //Ensure selects of these expressions always have new set - it saves problems when an
-            //unnormalized tree is walked or transformed.
-            //IF/PROJECTROW are not includes since they could be folded to an active selector
-            isNew = true;
-            return lhs;
         default:
             return lhs;
         }
@@ -13225,17 +13214,6 @@ IHqlExpression * ensureDataset(IHqlExpression * expr)
 }
 
 
-extern bool isAlwaysNewRow(IHqlExpression * expr)
-{
-    switch (expr->getOperator())
-    {
-    case no_createrow:
-    case no_selectnth:
-        return true;
-    }
-    return false;
-}
-
 extern bool isAlwaysActiveRow(IHqlExpression * expr)
 {
     switch (expr->getOperator())

ecl/hql/hqlexpr.hpp

@@ -1603,7 +1603,6 @@ extern HQL_API IHqlExpression * createSelectorSequence(unsigned __int64 seq);
 extern HQL_API IHqlExpression * createDummySelectorSequence();
 extern HQL_API IHqlExpression * expandBetween(IHqlExpression * expr);
 extern HQL_API bool isAlwaysActiveRow(IHqlExpression * expr);
-extern HQL_API bool isAlwaysNewRow(IHqlExpression * expr);
 extern HQL_API IHqlExpression * ensureActiveRow(IHqlExpression * expr);
 extern HQL_API bool isRedundantGlobalScope(IHqlExpression * expr);
 extern HQL_API bool isIndependentOfScope(IHqlExpression * expr);

ecl/hql/hqltrans.cpp

@@ -1611,29 +1611,28 @@ IHqlExpression * NewHqlTransformer::transformSelector(IHqlExpression * expr)
 #ifdef TRANSFORM_STATS_DETAILS
     stats.numTransformSelects++;
 #endif
-
-    expr = expr->queryNormalizedSelector();
-    IHqlExpression * transformed = queryAlreadyTransformedSelector(expr);
-    if (transformed)
+    IHqlExpression * normalized = expr->queryNormalizedSelector();
+    IHqlExpression * transformedSelector = queryAlreadyTransformedSelector(normalized);
+    if (transformedSelector)
     {
 #ifdef TRANSFORM_STATS_DETAILS
-        if (expr ==  transformed)
+        if (expr ==  transformedSelector)
             stats.numTransformSelectsSame++;
 #endif
-        if (transformed->getOperator() == no_activerow)
-            return LINK(transformed->queryChild(0));
-        return LINK(transformed);
+        if (transformedSelector->getOperator() == no_activerow)
+            return LINK(transformedSelector->queryChild(0));
+        return LINK(transformedSelector);
     }
-    transformed = queryAlreadyTransformed(expr);
+    IHqlExpression * transformed = queryAlreadyTransformed(normalized);
     if (transformed)
         transformed = LINK(transformed->queryNormalizedSelector());
     else
-        transformed = createTransformedSelector(expr);
+        transformed = createTransformedSelector(normalized);
 #ifdef TRANSFORM_STATS_DETAILS
-    if (expr ==  transformed)
+    if (normalized == transformed)
         stats.numTransformSelectsSame++;
 #endif
-    setTransformedSelector(expr, transformed);
+    setTransformedSelector(normalized, transformed);
     return transformed;
 }
 
@@ -1673,9 +1672,7 @@ IHqlExpression * NewHqlTransformer::createTransformedActiveSelect(IHqlExpression
     if (newLeft->getOperator() == no_newrow)
         return createNewSelectExpr(LINK(newLeft->queryChild(0)), LINK(newRight));
 
-    //NOTE: In very obscure situations: ds[1].x.ds<new>.y -> z.ds<new>.y (newLeft != normalizeSelector)
-    //NB: newLeft.get() == newLeft->queryNormalizedSelector() - asserted above
-    return createSelectExpr(LINK(newLeft->queryNormalizedSelector()), LINK(newRight));
+    return createSelectExpr(LINK(newLeft), LINK(newRight));
 }
 
 
@@ -1837,7 +1834,7 @@ void NewHqlTransformer::setTransformedSelector(IHqlExpression * expr, IHqlExpres
 {
     assertex(expr == expr->queryNormalizedSelector());
     //in rare situations a selector could get converted to a non-selector e.g, when replace self-ref with a new dataset.
-    assertex(!transformed || transformed == transformed->queryNormalizedSelector() || transformed->hasAttribute(newAtom));
+    assertex(!transformed || transformed == transformed->queryNormalizedSelector() || (transformed->getOperator() == no_select && isNewSelector(transformed)));
     queryTransformExtra(expr)->setTransformedSelector(transformed);
 }
 
@@ -4178,6 +4175,12 @@ IHqlExpression * ScopedTransformer::createTransformed(IHqlExpression * expr)
             }
             if (expr->isDataset())
                 restoreScope();
+            if (op == no_libraryscopeinstance)
+            {
+                IHqlExpression * oldFunction = expr->queryDefinition();
+                OwnedHqlExpr newFunction = transform(oldFunction);
+                return createLibraryInstance(newFunction.getClear(), children);
+            }
             break;
         }
     default:

ecl/hqlcpp/hqlcpp.cpp

@@ -8463,8 +8463,8 @@ void HqlCppTranslator::optimizeOrderValues(HqlExprArray & leftValues, HqlExprArr
         if (!compareSize)
             continue;
 
-        bool nextLeftIsNew;
-        bool nextRightIsNew;
+        bool nextLeftIsNew = false;
+        bool nextRightIsNew = false;
         IHqlExpression * nextLeft = &leftValues.item(i+1);
         IHqlExpression * nextRight = &rightValues.item(i+1);
         if (querySimpleOrderSelector(nextLeft, nextLeftIsNew) != leftSel || querySimpleOrderSelector(nextRight, nextRightIsNew) != rightSel ||

ecl/hqlcpp/hqlcppds.cpp

@@ -1876,7 +1876,7 @@ IHqlExpression * HqlCppTranslator::getResourcedChildGraph(BuildCtx & ctx, IHqlEx
         resourced.setown(resourceLoopGraph(*this, activeRows, resourced, targetClusterType, graphIdExpr, numResults, isInsideChildQuery, unlimitedResources));
     }
     else
-        resourced.setown(resourceNewChildGraph(*this, activeRows, resourced, targetClusterType, graphIdExpr, numResults));
+        resourced.setown(resourceNewChildGraph(ctx, *this, activeRows, resourced, targetClusterType, graphIdExpr, numResults));
 
     checkNormalized(ctx, resourced);
     traceExpression("AfterResourcing Child", resourced);
@@ -4584,7 +4584,8 @@ void HqlCppTranslator::doBuildRowAssignProjectRow(BuildCtx & ctx, IReferenceSele
     BuildCtx subctx(ctx);
 
     OwnedHqlExpr leftSelect = createSelector(no_left, srcRow, querySelSeq(expr));
-    OwnedHqlExpr newTransform = replaceSelector(transform, leftSelect, srcRow);
+    OwnedHqlExpr newRow = srcRow->getOperator() == no_select ? LINK(srcRow) : createRow(no_newrow, LINK(srcRow));
+    OwnedHqlExpr newTransform = replaceSelector(transform, leftSelect, newRow);
 
     Owned<BoundRow> selfCursor = target->getRow(subctx);
     doTransform(subctx, newTransform, selfCursor);

ecl/hqlcpp/hqlhtcpp.cpp

@@ -6524,11 +6524,15 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
                 result = doBuildActivityKeyedDistribute(ctx, expr);
                 break;
             case no_select:
-                if (isNewSelector(expr))
+            {
+                bool isNew;
+                IHqlExpression * ds = querySelectorDataset(expr, isNew);
+                if (isNew && (ds->getOperator() != no_translated) && !ctx.queryAssociation(ds, AssocRow, nullptr))
                     result = doBuildActivitySelectNew(ctx, expr);
                 else
                     result = doBuildActivityChildDataset(ctx, expr);
                 break;
+            }
                 //Items in this list need to also be in the list inside doBuildActivityChildDataset
             case no_call:
             case no_externalcall:

ecl/hqlcpp/hqlresource.cpp

@@ -1261,6 +1261,8 @@ bool ActivityInvariantHoister::findSplitPoints(IHqlExpression * expr, bool isPro
                 locator.analyseChild(expr->queryChild(0), true);
                 break;
             }
+        case no_translated:
+            break;
         case no_childquery:
             throwUnexpected();
         default:
@@ -3917,6 +3919,16 @@ void EclResourcer::deriveUsageCounts(IHqlExpression * expr)
         if (insideNeverSplit || insideSteppedNeverSplit)
             info->neverSplit = true;
 
+        //Currently code that is executed inline is not resourced at the same time as code that is executed
+        //out of line.  If a row has already been evaluated inline, ensure that this expression is not modified
+        //e.g. by adding splitters - otherwise it will be re-evaluated in a child query, rather than reusing
+        //the expression already evaluated. Fixing HPCC-9318 would make this test redundant.
+        if (ctx && expr->isDatarow())
+        {
+            if ((getNumActivityArguments(expr) != 0) && ctx->queryAssociation(expr, AssocRow, nullptr))
+                insideNeverSplit = true;
+        }
+
         switch (expr->getOperator())
         {
         case no_select:
@@ -6664,7 +6676,7 @@ IHqlExpression * resourceThorGraph(HqlCppTranslator & translator, IHqlExpression
 }
 
 
-static IHqlExpression * doResourceGraph(HqlCppTranslator & translator, HqlExprCopyArray * activeRows, IHqlExpression * _expr,
+static IHqlExpression * doResourceGraph(BuildCtx * ctx, HqlCppTranslator & translator, HqlExprCopyArray * activeRows, IHqlExpression * _expr,
                                         ClusterType targetClusterType, unsigned clusterSize,
                                         IHqlExpression * graphIdExpr, unsigned numResults, bool isChild, bool useGraphResults, bool unlimitedResources)
 {
@@ -6689,6 +6701,7 @@ static IHqlExpression * doResourceGraph(HqlCppTranslator & translator, HqlExprCo
 
     {
         EclResourcer resourcer(translator.queryErrorProcessor(), translator.wu(), translator.queryOptions(), options);
+        resourcer.setContext(ctx);
         resourcer.tagActiveCursors(activeRows);
         resourcer.resourceGraph(expr, transformed);
         totalResults = resourcer.numGraphResults();
@@ -6708,18 +6721,18 @@ static IHqlExpression * doResourceGraph(HqlCppTranslator & translator, HqlExprCo
 
 IHqlExpression * resourceLibraryGraph(HqlCppTranslator & translator, IHqlExpression * expr, ClusterType targetClusterType, unsigned clusterSize, IHqlExpression * graphIdExpr, unsigned numResults)
 {
-    return doResourceGraph(translator, NULL, expr, targetClusterType, clusterSize, graphIdExpr, numResults, false, true, false);       //?? what value for isChild (e.g., thor library call).  Need to gen twice?
+    return doResourceGraph(nullptr, translator, NULL, expr, targetClusterType, clusterSize, graphIdExpr, numResults, false, true, false);       //?? what value for isChild (e.g., thor library call).  Need to gen twice?
 }
 
 
-IHqlExpression * resourceNewChildGraph(HqlCppTranslator & translator, HqlExprCopyArray & activeRows, IHqlExpression * expr, ClusterType targetClusterType, IHqlExpression * graphIdExpr, unsigned numResults)
+IHqlExpression * resourceNewChildGraph(BuildCtx & ctx, HqlCppTranslator & translator, HqlExprCopyArray & activeRows, IHqlExpression * expr, ClusterType targetClusterType, IHqlExpression * graphIdExpr, unsigned numResults)
 {
-    return doResourceGraph(translator, &activeRows, expr, targetClusterType, 0, graphIdExpr, numResults, true, true, false);
+    return doResourceGraph(&ctx, translator, &activeRows, expr, targetClusterType, 0, graphIdExpr, numResults, true, true, false);
 }
 
 IHqlExpression * resourceLoopGraph(HqlCppTranslator & translator, HqlExprCopyArray & activeRows, IHqlExpression * expr, ClusterType targetClusterType, IHqlExpression * graphIdExpr, unsigned numResults, bool insideChildQuery, bool unlimitedResources)
 {
-    return doResourceGraph(translator, &activeRows, expr, targetClusterType, 0, graphIdExpr, numResults, insideChildQuery, true, unlimitedResources);
+    return doResourceGraph(nullptr, translator, &activeRows, expr, targetClusterType, 0, graphIdExpr, numResults, insideChildQuery, true, unlimitedResources);
 }
 
 IHqlExpression * resourceRemoteGraph(HqlCppTranslator & translator, IHqlExpression * _expr, ClusterType targetClusterType, unsigned clusterSize)

ecl/hqlcpp/hqlresource.hpp

@@ -24,7 +24,7 @@
 IHqlExpression * resourceThorGraph(HqlCppTranslator & translator, IHqlExpression * expr, ClusterType targetClusterType, unsigned clusterSize, IHqlExpression * graphIdExpr);
 IHqlExpression * resourceLibraryGraph(HqlCppTranslator & translator, IHqlExpression * expr, ClusterType targetClusterType, unsigned clusterSize, IHqlExpression * graphIdExpr, unsigned numResults);
 IHqlExpression * resourceLoopGraph(HqlCppTranslator & translator, HqlExprCopyArray & activeRows, IHqlExpression * expr, ClusterType targetClusterType, IHqlExpression * graphIdExpr, unsigned numResults, bool insideChildQuery, bool unlimitedResources);
-IHqlExpression * resourceNewChildGraph(HqlCppTranslator & translator, HqlExprCopyArray & activeRows, IHqlExpression * expr, ClusterType targetClusterType, IHqlExpression * graphIdExpr, unsigned numResults);
+IHqlExpression * resourceNewChildGraph(BuildCtx & ctx, HqlCppTranslator & translator, HqlExprCopyArray & activeRows, IHqlExpression * expr, ClusterType targetClusterType, IHqlExpression * graphIdExpr, unsigned numResults);
 IHqlExpression * resourceRemoteGraph(HqlCppTranslator & translator, IHqlExpression * expr, ClusterType targetClusterType, unsigned clusterSize);
 
 IHqlExpression * convertSpillsToActivities(IHqlExpression * expr, bool createGraphResults);

ecl/hqlcpp/hqlresource.ipp

@@ -466,6 +466,8 @@ public:
 
     void resourceGraph(IHqlExpression * expr, HqlExprArray & transformed);
     void resourceRemoteGraph(IHqlExpression * expr, HqlExprArray & transformed);
+    void setContext(BuildCtx * _ctx) { ctx = _ctx; }
+
     void setSequential(bool _sequential) { sequential = _sequential; }
     void tagActiveCursors(HqlExprCopyArray * activeRows);
     inline unsigned numGraphResults() { return options.nextResult; }
@@ -578,6 +580,7 @@ protected:
     CResourceOptions & options;
     HqlExprArray rootConditions;
     HqlExprCopyArray activeSelectors;
+    BuildCtx * ctx = nullptr;
 };
 
 #endif