Merge pull request #4459 from ghalliday/issue9430

HPCC-9430 Remove incorrect active datasets for unusual cases

Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

common/deftype/deftype.cpp

@@ -3426,6 +3426,9 @@ ITypeInfo * replaceChildType(ITypeInfo * type, ITypeInfo * newChild)
     case type_sortlist:
         newType.setown(makeSortListType(LINK(newChild)));
         break;
+    case type_rule:
+        newType.setown(makeRuleType(LINK(newChild)));
+        break;
     default:
         throwUnexpected();
     }

ecl/hql/hqlexpr.cpp

@@ -4796,6 +4796,32 @@ void CUsedTablesBuilder::cleanupProduction()
     }
 }
 
+void CUsedTablesBuilder::removeParent(IHqlExpression * expr)
+{
+    IHqlExpression * sel = expr->queryNormalizedSelector();
+    removeActive(sel);
+
+    loop
+    {
+        IHqlExpression * root = queryRoot(expr);
+        if (!root || root->getOperator() != no_select)
+            break;
+        if (!root->hasProperty(newAtom))
+            break;
+        expr = root->queryChild(0);
+        removeActive(expr->queryNormalizedSelector());
+    }
+}
+
+void CUsedTablesBuilder::removeActiveRecords()
+{
+    ForEachItemInRev(i, inScopeTables)
+    {
+        if (inScopeTables.item(i).isRecord())
+            inScopeTables.remove(i);
+    }
+}
+
 void CUsedTablesBuilder::removeRows(IHqlExpression * expr, IHqlExpression * left, IHqlExpression * right)
 {
     node_operator rowsSide = queryHasRows(expr);
@@ -4906,7 +4932,7 @@ void CHqlExpressionWithTables::cacheTablesProcessChildScope(CUsedTablesBuilder &
         {
             IHqlExpression * ds = queryChild(0);
             cacheChildrenTablesUsed(used, 1, max);
-            used.removeActive(ds->queryNormalizedSelector());
+            used.removeParent(ds);
             cacheChildrenTablesUsed(used, 0, 1);
         }
         break;
@@ -4917,7 +4943,7 @@ void CHqlExpressionWithTables::cacheTablesProcessChildScope(CUsedTablesBuilder &
             OwnedHqlExpr left = createSelector(no_left, ds, selSeq);
             cacheChildrenTablesUsed(used, 1, max);
             used.removeActive(left);
-            used.removeActive(ds->queryNormalizedSelector());
+            used.removeParent(ds);
             used.removeRows(this, left, NULL);
             cacheChildrenTablesUsed(used, 0, 1);
 #ifdef GATHER_HIDDEN_SELECTORS
@@ -4964,7 +4990,7 @@ void CHqlExpressionWithTables::cacheTablesProcessChildScope(CUsedTablesBuilder &
             OwnedHqlExpr left = createSelector(no_left, ds, selSeq);
             OwnedHqlExpr right = createSelector(no_right, ds, selSeq);
             cacheChildrenTablesUsed(used, 1, max);
-            used.removeActive(ds->queryNormalizedSelector());
+            used.removeParent(ds);
             used.removeActive(left);
             used.removeActive(right);
             used.removeRows(this, left, right);
@@ -5092,7 +5118,7 @@ void CHqlExpressionWithTables::cacheTablesUsed()
                     {
     #ifdef GATHER_HIDDEN_SELECTORS
                         cachePotentialTablesUsed(used);
-                        used.removeActive(queryChild(0)->queryNormalizedSelector());
+                        used.removeParent(queryChild(0));
     #else
                         HqlExprCopyArray childInScopeTables;
                         ForEachChild(idx, this)
@@ -5104,9 +5130,13 @@ void CHqlExpressionWithTables::cacheTablesUsed()
     #endif
                     }
                     break;
+                case no_sizeof:
+                    cachePotentialTablesUsed(used);
+                    used.removeActive(queryActiveTableSelector());
+                    used.removeActiveRecords();
+                    break;
                 case no_externalcall:
                 case no_rowvalue:
-                case no_sizeof:
                 case no_offsetof:
                 case no_eq:
                 case no_ne:
@@ -5124,7 +5154,9 @@ void CHqlExpressionWithTables::cacheTablesUsed()
                 case no_keyindex:
                 case no_newkeyindex:
                     cacheChildrenTablesUsed(used, 1, numChildren());
-                    used.removeActive(queryChild(0)->queryNormalizedSelector());
+                    used.removeParent(queryChild(0));
+                    //Distributed attribute might contain references to the no_activetable
+                    used.removeActive(queryActiveTableSelector());
                     break;
                 case no_evaluate:
                     cacheTableUseage(used, queryChild(0));
@@ -5135,7 +5167,7 @@ void CHqlExpressionWithTables::cacheTablesUsed()
                         cacheChildrenTablesUsed(used, 0, numChildren());
                         IHqlExpression * parent = queryChild(3);
                         if (parent)
-                            used.removeActive(parent->queryNormalizedSelector());
+                            used.removeParent(parent);
                         break;
                     }
                 case no_pat_production:
@@ -5313,13 +5345,13 @@ IHqlExpression * CHqlSelectBaseExpression::makeSelectExpression(HqlExprArray & o
 
 bool CHqlSelectBaseExpression::isIndependentOfScope()
 {
-    IHqlExpression * ds = queryChild(0);
     if (isSelectRootAndActive())
     {
         return false;
     }
     else
     {
+        IHqlExpression * ds = queryChild(0);
         return ds->isIndependentOfScope();
     }
 }
@@ -5994,7 +6026,7 @@ void CHqlDataset::cacheParent()
                     IHqlDataset * parent = pDataset->queryTable();
                     if (parent)
                     {
-                        container = queryExpression(parent->queryRootTable());
+                        container = ::queryExpression(parent->queryRootTable());
                         container->Link();
                     }
                 }
@@ -6051,7 +6083,7 @@ void CHqlDataset::cacheParent()
     if (op != no_select)
     {
         IHqlDataset * table = queryTable();
-        IHqlExpression * tableExpr = queryExpression(table);//->queryNormalizedSelector();
+        IHqlExpression * tableExpr = ::queryExpression(table);
         if (tableExpr != this)
         {
             normalized.set(tableExpr->queryNormalizedSelector());
@@ -15775,7 +15807,7 @@ IHqlExpression * queryExpression(ITypeInfo * type)
 IHqlExpression * queryExpression(IHqlDataset * ds)
 {
     if (!ds) return NULL;
-    return QUERYINTERFACE(ds, IHqlExpression);
+    return ds->queryExpression();
 }
 
 IHqlScope * queryScope(ITypeInfo * type)

ecl/hql/hqlexpr.hpp

@@ -1049,6 +1049,7 @@ interface IHqlRemoteScope : public IInterface
 
 interface IHqlDataset : public IInterface
 {
+    virtual IHqlExpression * queryExpression() = 0;
     virtual IHqlDataset* queryTable() = 0;
     virtual IHqlDataset * queryRootTable() = 0;         // get DATASET definition.
     virtual IHqlExpression * queryContainer() = 0;          // this shouldn't really be used - won't extent to more arbitrary relation trees.

ecl/hql/hqlexpr.ipp

@@ -97,6 +97,8 @@ public:
     void addActiveTable(IHqlExpression * expr);
     void cleanupProduction();
     inline void removeActive(IHqlExpression * expr) { inScopeTables.zap(*expr); }
+    void removeParent(IHqlExpression * expr);
+    void removeActiveRecords();
     void removeRows(IHqlExpression * expr, IHqlExpression * left, IHqlExpression * right);
     void set(CUsedTables & tables) { tables.set(inScopeTables, newScopeTables); }
 
@@ -783,6 +785,7 @@ class CHqlExternalDatasetCall: public CHqlExternalCall, implements IHqlDataset
 
     virtual IHqlExpression *clone(HqlExprArray &newkids);
     virtual IHqlDataset *queryDataset() { return this; }
+    virtual IHqlExpression * queryExpression() { return this; }
 
 //interface IHqlDataset
     virtual IHqlDataset* queryTable()               { return this; }
@@ -917,6 +920,7 @@ class CHqlDelayedDatasetCall: public CHqlDelayedCall, implements IHqlDataset
     IMPLEMENT_IINTERFACE_USING(CHqlDelayedCall)
 
     virtual IHqlDataset *queryDataset() { return this; }
+    virtual IHqlExpression * queryExpression() { return this; }
 
 //interface IHqlDataset
     virtual IHqlDataset* queryTable()               { return this; }
@@ -1304,6 +1308,7 @@ public:
 //IHqlExpression
     virtual bool assignableFrom(ITypeInfo * source) { type_t tc = source->getTypeCode(); return tc==type_table; }
     virtual IHqlDataset *queryDataset() { return this; }
+    virtual IHqlExpression * queryExpression() { return this; }
 
 //CHqlParameter
 
@@ -1331,6 +1336,7 @@ public:
 //IHqlExpression
     virtual bool assignableFrom(ITypeInfo * source) { type_t tc = source->getTypeCode(); return tc==type_dictionary; }
     virtual IHqlDataset *queryDataset() { return this; }
+    virtual IHqlExpression * queryExpression() { return this; }
 
 //CHqlParameter
 
@@ -1627,6 +1633,7 @@ public:
     static CHqlDataset *makeDataset(node_operator op, ITypeInfo *type, HqlExprArray &operands);
 
     virtual IHqlDataset *queryDataset() { return this; };
+    virtual IHqlExpression * queryExpression() { return this; }
     virtual IHqlSimpleScope *querySimpleScope();
     virtual IHqlDataset* queryTable();
     virtual IHqlExpression *queryNormalizedSelector(bool skipIndex);

ecl/hqlcpp/hqlcerrors.hpp

@@ -251,6 +251,7 @@
 #define HQLWRN_GroupedGlobalFew                 4540
 #define HQLWRN_AmbiguousRollupCondition         4541
 #define HQLWRN_AmbiguousRollupNoGroup           4542
+#define HQLWRN_GlobalActionDependendOnScope     4543
 
 //Temporary errors
 #define HQLERR_OrderOnVarlengthStrings          4601
@@ -523,6 +524,7 @@
 #define HQLWRN_GroupedGlobalFew_Text            "Global few expression is grouped"
 #define HQLWRN_AmbiguousRollupCondition_Text    "ROLLUP condition on '%s' is also modified in the transform"
 #define HQLWRN_AmbiguousRollupNoGroup_Text      "ROLLUP condition - no fields are preserved in the transform - not converted to GROUPed ROLLUP"
+#define HQLWRN_GlobalActionDependendOnScope_Text "Global action appears to be context dependent - this may cause a dataset not active error"
 
 #define HQLERR_OrderOnVarlengthStrings_Text     "Rank/Ranked not supported on variable length strings"
 #define HQLERR_DistributionNoSequence_Text      "DISTRIBUTION() only supported at the outer level"

ecl/hqlcpp/hqlttcpp.cpp

@@ -7143,23 +7143,6 @@ void ExplicitGlobalTransformer::doAnalyseExpr(IHqlExpression * expr)
     node_operator op = expr->getOperator();
     switch (op)
     {
-    case no_output:
-        if (!isIndependentOfScope(expr))
-        {
-            IHqlExpression * filename = queryRealChild(expr, 2);
-            if (!filename)
-                filename = expr->queryProperty(namedAtom);
-
-            StringBuffer s;
-            if (filename)
-                getExprECL(filename, s);
-            translator.WARNINGAT1(queryActiveLocation(expr), HQLWRN_OutputDependendOnScope, s.str());
-
-#if 0
-            checkIndependentOfScope(expr);
-#endif
-        }
-        break;
     case no_nothor:
         if (expr->isAction())
             break;
@@ -7256,6 +7239,69 @@ IHqlExpression * ExplicitGlobalTransformer::createTransformed(IHqlExpression * e
 
 //------------------------------------------------------------------------
 
+static HqlTransformerInfo scopeIndependentActionCheckerInfo("ScopeIndependentActionChecker");
+class ScopeIndependentActionChecker : public NewHqlTransformer
+{
+public:
+    ScopeIndependentActionChecker(HqlCppTranslator & _translator) : NewHqlTransformer(scopeIndependentActionCheckerInfo), translator(_translator)
+    {
+    }
+
+protected:
+    void analyseExpr(IHqlExpression * expr)
+    {
+        switch (expr->getOperator())
+        {
+        case no_parallel:
+        case no_if:
+        case no_sequential:
+        case no_compound:
+        case no_actionlist:
+            NewHqlTransformer::analyseExpr(expr);
+            break;
+        case no_output:
+            if (!isIndependentOfScope(expr))
+            {
+                IHqlExpression * filename = queryRealChild(expr, 2);
+                if (!filename)
+                    filename = expr->queryProperty(namedAtom);
+
+                StringBuffer s;
+                if (filename)
+                    getExprECL(filename, s);
+                translator.WARNINGAT1(queryActiveLocation(expr), HQLWRN_OutputDependendOnScope, s.str());
+
+    #if 0
+                checkIndependentOfScope(expr);
+    #endif
+            }
+            break;
+        default:
+            if (!isIndependentOfScope(expr))
+            {
+                translator.WARNINGAT(queryActiveLocation(expr), HQLWRN_GlobalActionDependendOnScope);
+
+    #if 0
+                checkIndependentOfScope(expr);
+    #endif
+            }
+            break;
+        }
+    }
+
+private:
+    HqlCppTranslator & translator;
+};
+
+void checkGlobalActionsIndependentOfScope(HqlCppTranslator & translator, const HqlExprArray & exprs)
+{
+    ScopeIndependentActionChecker checker(translator);
+    checker.analyseArray(exprs, 0);
+}
+
+
+//------------------------------------------------------------------------
+
 
 IHqlDataset * queryRootDataset(IHqlExpression * dataset)
 {
@@ -7459,6 +7505,8 @@ void migrateExprToNaturalLevel(WorkflowItem & cur, IWorkUnit * wu, HqlCppTransla
 
     translator.traceExpressions("m0", exprs);
 
+    checkGlobalActionsIndependentOfScope(translator, exprs);
+
     if (options.workunitTemporaries)
     {
         ExplicitGlobalTransformer transformer(wu, translator);
@@ -10522,7 +10570,7 @@ IHqlExpression * HqlTreeNormalizer::queryTransformPatternDefine(IHqlExpression *
             HqlExprArray args;
             args.append(*createValue(no_define, base->getType(), base, makeRecursiveName(module, name)));
             unwindChildren(args, expr, 1);
-            return expr->clone(args);
+            return createValue(expr->getOperator(), transformType(expr->queryType()), args);
         }
     }
     return NULL;
@@ -10845,7 +10893,8 @@ IHqlExpression * HqlTreeNormalizer::transformPatNamedUse(IHqlExpression * expr)
             args.append(*LINK(cur));
     }
     args.append(*define.getClear());
-    return expr->clone(args);
+    ITypeInfo * type = expr->queryType();
+    return createValue(no_pat_use, transformType(type), args);
 }
 
 IHqlExpression * HqlTreeNormalizer::transformPatCheckIn(IHqlExpression * expr)
@@ -11235,7 +11284,10 @@ IHqlExpression * HqlTreeNormalizer::transformChildrenNoAnnotations(IHqlExpressio
 {
     HqlExprArray args;
     ForEachChild(i, expr)
-        args.append(*transform(expr->queryChild(i)->queryBody()));
+    {
+        OwnedHqlExpr newChild = transform(expr->queryChild(i)->queryBody());
+        args.append(*LINK(newChild->queryBody()));
+    }
     return completeTransform(expr, args);
 }
 
@@ -11522,14 +11574,15 @@ IHqlExpression * HqlTreeNormalizer::createTransformedBody(IHqlExpression * expr)
     case no_top:
     case no_self:
         {
-            HqlExprArray children;
             IHqlExpression * record = expr->queryChild(0);
+            //If no record argument then make sure the type is transformed
+            if (!record)
+                break;
+
+            HqlExprArray children;
             //Ensure that the first parameter to one of these nodes is the body of the record, not a named symbol
-            if (record)
-            {
-                OwnedHqlExpr transformedRecord = transform(record);
-                children.append(*LINK(transformedRecord->queryBody()));
-            }
+            OwnedHqlExpr transformedRecord = transform(record);
+            children.append(*LINK(transformedRecord->queryBody()));
             return completeTransform(expr, children);
         }
     case no_field:
@@ -11917,14 +11970,14 @@ IHqlExpression * HqlTreeNormalizer::createTransformedBody(IHqlExpression * expr)
             HqlExprArray children;
             transformChildren(expr, children);
             OwnedITypeInfo setType = makeSetType(children.item(0).getType());
-            return createValue(expr->getOperator(), setType.getClear(), children);
+            return createValue(op, setType.getClear(), children);
         }
     case no_rowsetrange:
         {
             HqlExprArray children;
             transformChildren(expr, children);
             OwnedITypeInfo setType = children.item(0).getType();
-            return createValue(expr->getOperator(), setType.getClear(), children);
+            return createValue(op, setType.getClear(), children);
         }
     case no_buildindex:
         {
@@ -12098,6 +12151,13 @@ IHqlExpression * HqlTreeNormalizer::createTransformedBody(IHqlExpression * expr)
             }
             break;
         }
+    case no_typedef:
+        {
+            HqlExprArray children;
+            transformChildren(expr, children);
+            OwnedITypeInfo newType = transformType(expr->queryType());
+            return createValue(op, newType.getClear(), children);
+        }
     case no_assertkeyed:
         {
             //Ensure assertkeyed is tagged with the selectors of each of the fields that are keyed, otherwise
@@ -12141,7 +12201,7 @@ IHqlExpression * HqlTreeNormalizer::createTransformedBody(IHqlExpression * expr)
                 IHqlExpression * filter = queryRealChild(transformed, 2);
                 IHqlExpression * rowsid = transformed->queryProperty(_rowsid_Atom);
                 IHqlExpression * selSeq = querySelSeq(transformed);
-                IHqlExpression * counter = queryPropertyChild(expr, _countProject_Atom, 0);
+                IHqlExpression * counter = queryPropertyChild(transformed, _countProject_Atom, 0);
                 OwnedHqlExpr left = createSelector(no_left, dataset, selSeq);
                 OwnedHqlExpr rowsExpr = createDataset(no_rows, LINK(left), LINK(rowsid));
                 OwnedHqlExpr initialLoopDataset = LINK(dataset);
@@ -12165,8 +12225,21 @@ IHqlExpression * HqlTreeNormalizer::createTransformedBody(IHqlExpression * expr)
         break;
     }
 
+    ITypeInfo * type = expr->queryType();
+    bool checkType = false;
+    if (type)
+    {
+        switch (type->getTypeCode())
+        {
+        case type_pattern:
+        case type_rule:
+            checkType = true;
+            break;
+        }
+    }
+
     unsigned max = expr->numChildren();
-    if (max == 0)
+    if ((max == 0) && !checkType)
         return LINK(expr);
 
     bool same = true;
@@ -12180,7 +12253,14 @@ IHqlExpression * HqlTreeNormalizer::createTransformedBody(IHqlExpression * expr)
         if (child != tchild)
             same = false;
     }
-//MORE: Test for pattern type here!
+
+    if (checkType)
+    {
+        OwnedITypeInfo newType = transformType(type);
+        if (type != newType)
+            return createWrapper(op, newType, children);
+    }
+
     if (!same)
         return expr->clone(children);
     return LINK(expr);