Merge pull request #11296 from ghalliday/issue19879

HPCC-19879 Convert SEQUENTIAL to ORDERED inside a child query

Reviewed-By: Shamser Ahmed <shamser.ahmed@lexisnexis.co.uk>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/hqlcpp/hqlcerrors.hpp

@@ -272,6 +272,7 @@
 #define HQLWRN_WorkflowDependParameter          4546
 #define HQLWRN_OutputScalarInsideChildQuery     4547
 #define HQLWRN_GlobalDatasetFromChildQuery      4548
+#define HQLWRN_NestedSequentialUseOrdered       4214
 
 //Temporary errors
 #define HQLERR_OrderOnVarlengthStrings          4601
@@ -566,6 +567,7 @@
 #define HQLWRN_WorkflowDependParameter_Text     "Workflow action %s appears to be dependent upon a parameter"
 #define HQLWRN_OutputScalarInsideChildQuery_Text "Output(%s) of single value inside a child query has undefined behaviour"
 #define HQLWRN_GlobalDatasetFromChildQuery_Text "Global dataset expression (%s) is used in a child query"
+#define HQLWRN_NestedSequentialUseOrdered_Text  "Using ORDERED instead of SEQUENTIAL for child actions"
 
 #define HQLERR_DistributionVariableLengthX_Text "DISTRIBUTION does not support variable length field '%s'"
 #define HQLERR_DistributionUnsupportedTypeXX_Text "DISTRIBUTION does not support field '%s' with type %s"

ecl/hqlcpp/hqlcpp.cpp

@@ -1812,6 +1812,7 @@ void HqlCppTranslator::cacheOptions()
         DebugOption(options.implicitKeyedDiskFilter,"implicitKeyedDiskFilter", false),
         DebugOption(options.addDefaultBloom,"addDefaultBloom", true),
         DebugOption(options.newDiskReadMapping, "newDiskReadMapping", true),
+        DebugOption(options.transformNestedSequential, "transformNestedSequential", true),
     };
 
     //get options values from workunit

ecl/hqlcpp/hqlcpp.ipp

@@ -797,6 +797,7 @@ struct HqlCppOptions
     bool                implicitKeyedDiskFilter;
     bool                addDefaultBloom;
     bool                newDiskReadMapping;
+    bool                transformNestedSequential;
 };
 
 //Any information gathered while processing the query should be moved into here, rather than cluttering up the translator class
@@ -1921,6 +1922,7 @@ protected:
     void optimizePersists(HqlExprArray & exprs);
     IHqlExpression * convertSetResultToExtract(IHqlExpression * expr);
     void allocateSequenceNumbers(HqlExprArray & exprs);
+    void transformNestedSequential(HqlExprArray & exprs);
     void convertLogicalToActivities(WorkflowItem & curWorkflow);
     void flattenDatasets(WorkflowArray & array);
 

ecl/hqlcpp/hqlttcpp.cpp

@@ -1453,17 +1453,22 @@ IHqlExpression * SequenceNumberAllocator::createTransformed(IHqlExpression * exp
             HqlExprArray args;
             ForEachChild(i, expr)
             {
-                OwnedHqlExpr next = transform(expr->queryChild(i));
-                if (!next->isAction())
+                LinkedHqlExpr cur = expr->queryChild(i);
+                if (!cur->isConstant())
                 {
-                    if (!next->isConstant())
-                        next.setown(createValue(no_evaluate_stmt, makeVoidType(), next.getClear()));
-                    else
-                        next.clear();
-                }
+                    if (!cur->isAction())
+                    {
+                        //For WHEN(a, b) generate WHEN(EVALUATE(a), b) instead of EVALUATE(WHEN(a,b))
+                        //since the code is generally cleaner and more efficient.
+                        HqlExprArray compoundArgs;
+                        cur->unwindList(compoundArgs, no_compound);
+                        unsigned last = compoundArgs.ordinality()-1;
+                        compoundArgs.replace(*createValue(no_evaluate_stmt, makeVoidType(), &OLINK(compoundArgs.item(last))), last);
+                        cur.setown(createCompound(compoundArgs));
+                    }
 
-                if (next)
-                    args.append(*next.getClear());
+                    args.append(*transform(cur));
+                }
             }
             return expr->clone(args);
         }
@@ -1537,6 +1542,98 @@ void HqlCppTranslator::allocateSequenceNumbers(HqlExprArray & exprs)
 
 //---------------------------------------------------------------------------
 
+class NestedSequentialTransformer : public NewHqlTransformer
+{
+public:
+    NestedSequentialTransformer(HqlCppTranslator & _translator);
+
+    virtual IHqlExpression * createTransformed(IHqlExpression * expr);
+
+protected:
+    virtual IHqlExpression * doTransformRootExpr(IHqlExpression * expr);
+
+protected:
+    HqlCppTranslator & translator; // should really be an error handler - could do with refactoring.
+};
+
+static HqlTransformerInfo nestedSequentialTransformerInfo("NestedSequentialTransformer");
+NestedSequentialTransformer::NestedSequentialTransformer(HqlCppTranslator & _translator) : NewHqlTransformer(nestedSequentialTransformerInfo), translator(_translator)
+{
+}
+
+IHqlExpression * NestedSequentialTransformer::doTransformRootExpr(IHqlExpression * expr)
+{
+    bool specialCase = expr->isAction();
+    switch (expr->getOperator())
+    {
+    case no_apply:
+        specialCase = false;
+        break;
+    case no_comma:
+    case no_stored:
+    case no_checkpoint:
+    case no_persist:
+    case no_critical:
+    case no_independent:
+    case no_once:
+    case no_success:
+    case no_failure:
+    case no_recovery:
+        specialCase = true;
+        break;
+    }
+
+    if (specialCase)
+    {
+        bool same = true;
+        HqlExprArray children;
+        ForEachChild(i, expr)
+        {
+            IHqlExpression * cur = expr->queryChild(i);
+            IHqlExpression * transformed = doTransformRootExpr(cur);
+            children.append(*transformed);
+            if (cur != transformed)
+                same = false;
+        }
+        if (!same)
+            return expr->clone(children);
+        return LINK(expr);
+    }
+    return transform(expr);
+}
+
+IHqlExpression * NestedSequentialTransformer::createTransformed(IHqlExpression * expr)
+{
+    switch (expr->getOperator())
+    {
+    case no_sequential:
+    {
+        translator.WARNINGAT(CategoryUnexpected, expr, HQLWRN_NestedSequentialUseOrdered);
+        HqlExprArray args;
+        transformChildren(expr, args);
+        return createAction(no_orderedactionlist, args);
+    }
+    case no_colon:
+    {
+        HqlExprArray args;
+        args.append(*transform(expr->queryChild(0)));
+        args.append(*doTransformRootExpr(expr->queryChild(1)));
+        return completeTransform(expr, args);
+    }
+    }
+    return NewHqlTransformer::createTransformed(expr);
+}
+
+void HqlCppTranslator::transformNestedSequential(HqlExprArray & exprs)
+{
+    HqlExprArray sequenced;
+    NestedSequentialTransformer transformer(*this);
+    transformer.transformRoot(exprs, sequenced);
+    replaceArray(exprs, sequenced);
+}
+
+//---------------------------------------------------------------------------
+
 static void replaceAssignSelector(HqlExprArray & assigns, IHqlExpression * newSelector)
 {
     ForEachItemIn(idx, assigns)
@@ -13709,6 +13806,13 @@ void HqlCppTranslator::normalizeGraphForGeneration(HqlExprArray & exprs, HqlQuer
     allocateSequenceNumbers(exprs);                                             // Added to all expressions/output statements etc.
 
     traceExpressions("allocate Sequence", exprs);
+
+    if (options.transformNestedSequential)
+    {
+        transformNestedSequential(exprs);
+        traceExpressions("transformNestedSequential", exprs);
+    }
+
     checkNormalized(exprs);
 }
 

testing/regress/ecl/redissynctest.ecl

@@ -177,7 +177,7 @@ END;
 pubDS := DATASET(N2, TRANSFORM({ integer a }, SELF.a := pub('PubSubTest' + (STRING)COUNTER)));
 
 INTEGER pub2(STRING channel) := FUNCTION
-        sl := SEQUENTIAL(
+        sl := ORDERED(
             Std.System.Debug.Sleep(10),
             myRedis.Publish(channel, '3', database)//This pub is the one read by the sub.
             );