Fix gh-634 ROWS(LEFT) handling for LOOP in thor

Also removes some unused code for a never implemented varient of LOOP

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

ecl/hql/hqlatoms.cpp

@@ -86,6 +86,7 @@ _ATOM deprecatedAtom;
 _ATOM descAtom;
 _ATOM diskAtom;
 _ATOM distributedAtom;
+_ATOM _distributed_Atom;
 _ATOM _dot_Atom;
 _ATOM dynamicAtom;
 _ATOM ebcdicAtom;
@@ -457,6 +458,7 @@ MODULE_INIT(INIT_PRIORITY_HQLATOM)
     MAKEATOM(desc);
     MAKEATOM(disk);
     MAKEATOM(distributed);
+    MAKESYSATOM(distributed);
     MAKESYSATOM(dot);
     MAKEATOM(dynamic);
     MAKEATOM(ebcdic);

ecl/hql/hqlatoms.hpp

@@ -89,6 +89,7 @@ extern HQL_API _ATOM deprecatedAtom;
 extern HQL_API _ATOM descAtom;
 extern HQL_API _ATOM diskAtom;
 extern HQL_API _ATOM distributedAtom;
+extern HQL_API _ATOM _distributed_Atom;
 extern HQL_API _ATOM _dot_Atom;
 extern HQL_API _ATOM dynamicAtom;
 extern HQL_API _ATOM ebcdicAtom;

ecl/hql/hqlattr.cpp

@@ -350,7 +350,6 @@ unsigned getOperatorMetaFlags(node_operator op)
     case no_loop:
     case no_forcenolocal:
     case no_allnodes:
-    case no_loop2:
     case no_selfjoin:
     case no_process:
     case no_thisnode:
@@ -606,7 +605,7 @@ case no_unused53:
     case no_mix:
     case no_persist_check:
 
-    case no_unused1: case no_unused2: case no_unused3: case no_unused4: case no_unused5:
+    case no_unused1: case no_unused2: case no_unused3: case no_unused4: case no_unused5: case no_unused6:
     case no_unused13: case no_unused14: case no_unused15: case no_unused16: case no_unused17: case no_unused18: case no_unused19:
     case no_unused20: case no_unused21: case no_unused22: case no_unused23: case no_unused24: case no_unused25: case no_unused26: case no_unused27: case no_unused28: case no_unused29:
     case no_unused30: case no_unused31: case no_unused32: case no_unused33: case no_unused34: case no_unused35: case no_unused36: case no_unused37: case no_unused38: case no_unused39:
@@ -1591,7 +1590,6 @@ bool isLocalActivity(IHqlExpression * expr)
     case no_assertgrouped:
     case no_nonempty:
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_aggregate:
     case no_combine:
@@ -1687,7 +1685,6 @@ bool isGroupedActivity(IHqlExpression * expr)
     case no_case:
     case no_map:
     case no_loop:
-    case no_loop2:
     case no_choosen:
     case no_process:
     case no_nonempty:
@@ -1783,7 +1780,6 @@ bool localChangesActivityAction(IHqlExpression * expr)
     case no_assertgrouped:
     case no_nonempty:
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_combine:
         return true;
@@ -1869,9 +1865,10 @@ bool isInlineTrivialDataset(IHqlExpression * expr)
             break;
         case no_workunit_dataset:
         case no_getresult:
-        case no_getgraphresult:
         case no_null:
             return true;
+        case no_getgraphresult:
+            return !expr->hasProperty(_distributed_Atom);
         default:
             return false;
         }
@@ -2488,7 +2485,10 @@ IHqlExpression * calcRowInformation(IHqlExpression * expr)
             }
             else
             {
-                info.setUnknown(RCMfew);
+                if (expr->hasProperty(_distributed_Atom))
+                    info.setUnknown(RCMdisk);
+                else
+                    info.setUnknown(RCMfew);
             }
             break;
         }
@@ -2718,7 +2718,6 @@ IHqlExpression * calcRowInformation(IHqlExpression * expr)
         info.setUnknown(RCMdisk);
         break;
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_libraryselect:
     case no_libraryinput:

ecl/hql/hqlexpr.cpp

@@ -984,7 +984,6 @@ const char *getOpString(node_operator op)
     case no_enum: return "ENUM";
     case no_pat_or: return "|";
     case no_loop: return "LOOP";
-    case no_loop2: return "LOOP";
     case no_loopbody: return "no_loopbody";
     case no_cluster: return "CLUSTER";
     case no_forcenolocal: return "NOLOCAL";
@@ -1081,7 +1080,7 @@ const char *getOpString(node_operator op)
     case no_assign_addfiles: return "+=";
     case no_debug_option_value: return "__DEBUG__";
 
-    case no_unused1: case no_unused2: case no_unused3: case no_unused4: case no_unused5:
+    case no_unused1: case no_unused2: case no_unused3: case no_unused4: case no_unused5: case no_unused6:
     case no_unused13: case no_unused14: case no_unused15: case no_unused16: case no_unused17: case no_unused18: case no_unused19:
     case no_unused20: case no_unused21: case no_unused22: case no_unused23: case no_unused24: case no_unused25: case no_unused26: case no_unused27: case no_unused28: case no_unused29:
     case no_unused30: case no_unused31: case no_unused32: case no_unused33: case no_unused34: case no_unused35: case no_unused36: case no_unused37: case no_unused38: case no_unused39:
@@ -1607,7 +1606,6 @@ childDatasetType getChildDatasetType(IHqlExpression * expr)
     case no_joincount:
     case no_combine:
     case no_combinegroup:   // Hmm really LEFT,rows but that's a bit too nasty.  RIGHT can be the first rhs record
-    case no_loop2:          // right is actually a row, but shouldn't cause issues.
     case no_process:
     case no_aggregate:
         return childdataset_leftright;
@@ -1789,8 +1787,6 @@ inline unsigned doGetNumChildTables(IHqlExpression * dataset)
     case no_normalizegroup:
     case no_owned_ds:
         return 1;
-    case no_loop2:
-        return 2;//hmm
     case no_childdataset:
     case no_left:
     case no_right:
@@ -1955,7 +1951,6 @@ node_operator queryHasRows(IHqlExpression * expr)
     case no_aggregate:
         return no_right;
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_rollupgroup:
     case no_nwayjoin:
@@ -2053,7 +2048,6 @@ bool definesColumnList(IHqlExpression * dataset)
     case no_iterate:
     case no_rollup:
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_newrow:         //only used while transforming
     case no_newaggregate:
@@ -10641,7 +10635,6 @@ IHqlExpression *createDataset(node_operator op, HqlExprArray & parms)
     case no_assert_ds:
     case no_spillgraphresult:
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_cluster:
     case no_forcenolocal:

ecl/hql/hqlexpr.hpp

@@ -657,7 +657,7 @@ enum _node_operator {
         no_cluster,
         no_forcenolocal,
         no_allnodes,
-        no_loop2,
+        no_unused6,
         no_matchrow,
         no_sequence,
         no_selfjoin,

ecl/hql/hqlfold.cpp

@@ -4965,7 +4965,6 @@ IHqlExpression * CExprFolderTransformer::percolateConstants(IHqlExpression * exp
             break;
         }
     case no_loop:
-    case no_loop2:
     case no_graphloop:
         //Safer to do nothing...
         break;
@@ -5339,7 +5338,6 @@ HqlConstantPercolator * CExprFolderTransformer::gatherConstants(IHqlExpression *
     case no_activerow:
     case no_newrow:
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_rowsetindex:
     case no_rowsetrange:

ecl/hql/hqlgram.y

@@ -7399,55 +7399,6 @@ simpleDataSet
                             $$.setExpr(loopExpr);
                             $$.setPosition($1);
                         }
-    | LOOP '(' startLeftRowsSeqFilter beginCounterScope ',' startRightRowUpdateSeq ',' expression ',' dataSet ',' transform endCounterScope loopOptions ')' endRightFilter endRowsGroup endLeftFilter endSelectorSequence
-                        {
-                            parser->reportWarning(WRN_UNSUPPORTED_FEATURE, $1.pos, "LOOP with loop variables not yet supported");
-
-                            //New experimental syntax - which would allow RIGHT to specify a row used to store temporaries for each loop iteration
-                            parser->normalizeExpression($8);
-                            parser->ensureDatasetTypeMatch($10, $3.queryExpr());
-                            parser->checkBooleanOrNumeric($8);
-                            IHqlExpression * left = $3.getExpr();
-                            IHqlExpression * right = $6.getExpr();
-                            IHqlExpression * body = createValue(no_loopbody, makeNullType(), $10.getExpr());
-                            IHqlExpression * counter = $13.getExpr();
-                            if (counter)
-                                body = createComma(body, createAttribute(_countProject_Atom, counter));
-                            IHqlExpression * loopCondition = parser->createLoopCondition(left, $8.getExpr(), NULL, $19.queryExpr());
-                            IHqlExpression * loopExpr = createDataset(no_loop2, left, createComma(
-                                                            createComma(right, loopCondition),
-                                                            createComma(body, $12.getExpr(), $14.getExpr(), $17.getExpr()),
-                                                            $19.getExpr()
-                                                            ));
-                            parser->checkLoopFlags($1, loopExpr);
-                            $$.setExpr(loopExpr);
-                            $$.setPosition($1);
-                        }
-    | LOOP '(' startLeftRowsSeqFilter beginCounterScope ',' startRightRowUpdateSeq ',' expression ',' expression ',' dataSet ',' transform endCounterScope loopOptions ')' endRightFilter endRowsGroup endLeftFilter endSelectorSequence
-                        {
-                            parser->reportWarning(WRN_UNSUPPORTED_FEATURE, $1.pos, "LOOP with loop variables not yet supported");
-
-                            //New experimental syntax - which would allow RIGHT to specify a row used to store temporaries for each loop iteration
-                            parser->ensureDatasetTypeMatch($12, $3.queryExpr());
-                            parser->normalizeExpression($8);
-                            parser->checkBooleanOrNumeric($8);
-                            parser->normalizeExpression($10, type_boolean, false);
-                            IHqlExpression * left = $3.getExpr();
-                            IHqlExpression * right = $6.getExpr();
-                            IHqlExpression * body = createValue(no_loopbody, makeNullType(), $12.getExpr());
-                            IHqlExpression * counter = $15.getExpr();
-                            if (counter)
-                                body = createComma(body, createAttribute(_countProject_Atom, counter));
-                            IHqlExpression * loopCondition = parser->createLoopCondition(left, $8.getExpr(), $10.getExpr(), $21.queryExpr());
-                            IHqlExpression * loopExpr = createDataset(no_loop2, left, createComma(
-                                                            createComma(right, loopCondition),
-                                                            createComma(body, $14.getExpr(), $16.getExpr(), $19.getExpr()),
-                                                            $21.getExpr()
-                                                            ));
-                            parser->checkLoopFlags($1, loopExpr);
-                            $$.setExpr(loopExpr);
-                            $$.setPosition($1);
-                        }
     | GRAPH '(' startLeftRowsSeqFilter beginCounterScope ',' expression ',' dataSet endCounterScope graphOptions ')' endRowsGroup endLeftFilter endSelectorSequence
                         {
                             parser->ensureDatasetTypeMatch($8, $3.queryExpr());

ecl/hql/hqltrans.cpp

@@ -329,7 +329,6 @@ bool activityHidesRows(IHqlExpression * expr, IHqlExpression * selector)
     {
     case no_rollupgroup:
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_filtergroup:
         return (selectOp == no_left);

ecl/hql/hqlutil.cpp

@@ -1555,8 +1555,6 @@ unsigned getNumActivityArguments(IHqlExpression * expr)
         return expr->numChildren()-1;
     case no_forcelocal:
         return 0;
-    case no_loop2:
-        return 1;
     default:
         return getNumChildTables(expr);
     }
@@ -1581,8 +1579,9 @@ bool isDistributedSourceActivity(IHqlExpression * expr)
     case no_compound_indexcount:
     case no_compound_indexgroupaggregate:
         return true;
-    case no_workunit_dataset:
     case no_getgraphresult:
+        return expr->hasProperty(_distributed_Atom);
+    case no_workunit_dataset:
     case no_getgraphloopresult:
     case no_temptable:
     case no_inlinetable:
@@ -4220,24 +4219,32 @@ bool SplitDatasetAttributeTransformer::split(OwnedHqlExpr & dataset, OwnedHqlExp
             IHqlExpression * cur = &datasets.item(i);
             while ((cur->getOperator() == no_selectnth) || (cur->getOperator() == no_preservemeta))
                 cur = cur->queryChild(0);
+
+            bool remove = false;
             switch (cur->getOperator())
             {
-            case no_workunit_dataset:
             case no_getgraphresult:
+                remove = !expr->hasProperty(_distributed_Atom);
+                break;
+            case no_workunit_dataset:
             case no_getgraphloopresult:
             case no_left:
             case no_right:
             case no_colon:
             case no_globalscope:
             case no_nothor:
+                remove = true;
+                break;
+            }
+
+            if (remove)
+            {
                 datasets.remove(i);
                 newDatasets.remove(i);
                 num--;
-                break;
-            default:
-                i++;
-                break;
             }
+            else
+                i++;
         }
     }
 
@@ -5351,7 +5358,8 @@ int compareLibraryParameterOrder(IInterface * * pleft, IInterface * * pright)
 
 
 
-LibraryInputMapper::LibraryInputMapper(IHqlExpression * _libraryInterface) : libraryInterface(_libraryInterface)
+LibraryInputMapper::LibraryInputMapper(IHqlExpression * _libraryInterface)
+: libraryInterface(_libraryInterface)
 {
     assertex(libraryInterface->getOperator() == no_funcdef);
     scopeExpr.set(libraryInterface->queryChild(0));
@@ -5416,7 +5424,7 @@ IHqlExpression * LibraryInputMapper::resolveParameter(_ATOM search)
 }
 
 
-void LibraryInputMapper::mapRealToLogical(HqlExprArray & inputExprs, HqlExprArray & logicalParams, IHqlExpression * libraryId, bool canStream)
+void LibraryInputMapper::mapRealToLogical(HqlExprArray & inputExprs, HqlExprArray & logicalParams, IHqlExpression * libraryId, bool canStream, bool distributed)
 {
     //Create a list of expressions representing each of the inputs...
     ForEachItemIn(i1, realParameters)
@@ -5435,6 +5443,8 @@ void LibraryInputMapper::mapRealToLogical(HqlExprArray & inputExprs, HqlExprArra
                 args.append(*createAttribute(_streaming_Atom));
                 if (isGrouped(cur))
                     args.append(*createAttribute(groupedAtom));
+                if (distributed)
+                    args.append(*createAttribute(_distributed_Atom));
                 result = createDataset(no_getgraphresult, args);
             }
             else

ecl/hql/hqlutil.hpp

@@ -451,7 +451,7 @@ public:
     unsigned findParameter(_ATOM search);
 
     void mapLogicalToReal(HqlExprArray & mapped, HqlExprArray & params);
-    void mapRealToLogical(HqlExprArray & inputExprs, HqlExprArray & logicalParams, IHqlExpression * libraryId, bool canStream);
+    void mapRealToLogical(HqlExprArray & inputExprs, HqlExprArray & logicalParams, IHqlExpression * libraryId, bool canStream, bool distributed);
     inline unsigned numParameters() const { return realParameters.ordinality(); }
     inline unsigned numStreamedInputs() const { return streamingAllowed ? numDatasets : 0; }
 

ecl/hqlcpp/hqlcppds.cpp

@@ -1312,6 +1312,8 @@ unique_id_t ChildGraphBuilder::buildLoopBody(BuildCtx & ctx, IHqlExpression * da
     args.append(*createAttribute(_loop_Atom));
     if (multiInstance)
         args.append(*createAttribute(_streaming_Atom));
+    if (translator.targetThor())    // MORE: && !isChildQuery(ctx)..
+        args.append(*createAttribute(_distributed_Atom));
     OwnedHqlExpr inputResult= createDataset(no_getgraphresult, args);
 
     //Result 2 is the counter - if present

ecl/hqlcpp/hqlhtcpp.cpp

@@ -6235,7 +6235,6 @@ ABoundActivity * HqlCppTranslator::buildActivity(BuildCtx & ctx, IHqlExpression
                 result = doBuildActivityAssert(ctx, expr);
                 break;
             case no_loop:
-            case no_loop2:
                 result = doBuildActivityLoop(ctx, expr);
                 break;
             case no_graphloop:
@@ -8217,19 +8216,12 @@ bool HqlCppTranslator::isCurrentActiveGraph(BuildCtx & ctx, IHqlExpression * gra
 
 ABoundActivity * HqlCppTranslator::doBuildActivityLoop(BuildCtx & ctx, IHqlExpression * expr)
 {
-    unsigned curArg = 0;
-    IHqlExpression * dataset = expr->queryChild(curArg++);
-    IHqlExpression * rhs = NULL;
-    if (expr->getOperator() == no_loop2)
-        rhs = expr->queryChild(curArg++);
-    IHqlExpression * count = queryRealChild(expr, curArg++);
-    IHqlExpression * filter = queryRealChild(expr, curArg++);
-    IHqlExpression * loopCond = queryRealChild(expr, curArg++);
-    IHqlExpression * body = expr->queryChild(curArg++);
+    IHqlExpression * dataset = expr->queryChild(0);
+    IHqlExpression * count = queryRealChild(expr, 1);
+    IHqlExpression * filter = queryRealChild(expr, 2);
+    IHqlExpression * loopCond = queryRealChild(expr, 3);
+    IHqlExpression * body = expr->queryChild(4);
     assertex(body->getOperator() == no_loopbody);
-    IHqlExpression * rhsTransform = NULL;
-    if (rhs)
-        rhsTransform = expr->queryChild(curArg++);
 
     IHqlExpression * counter = queryPropertyChild(expr, _countProject_Atom, 0);
     IHqlExpression * rowsid = expr->queryProperty(_rowsid_Atom);
@@ -8343,10 +8335,9 @@ ABoundActivity * HqlCppTranslator::doBuildActivityLoop(BuildCtx & ctx, IHqlExpre
 
 ABoundActivity * HqlCppTranslator::doBuildActivityGraphLoop(BuildCtx & ctx, IHqlExpression * expr)
 {
-    unsigned curArg = 0;
-    IHqlExpression * dataset = expr->queryChild(curArg++);
-    IHqlExpression * count = expr->queryChild(curArg++);
-    IHqlExpression * body = expr->queryChild(curArg++);
+    IHqlExpression * dataset = expr->queryChild(0);
+    IHqlExpression * count = expr->queryChild(1);
+    IHqlExpression * body = expr->queryChild(2);
     assertex(body->getOperator() == no_loopbody);
     IHqlExpression * counter = queryPropertyChild(expr, _countProject_Atom, 0);
     IHqlExpression * rowsid = expr->queryProperty(_rowsid_Atom);

ecl/hqlcpp/hqlinline.cpp

@@ -268,7 +268,10 @@ static unsigned calcInlineFlags(BuildCtx * ctx, IHqlExpression * expr)
     case no_call:               
     case no_externalcall:               // no so sure about this - should possibly be assignable only. (also no_call above)
     case no_getresult:
+        return expr->isDatarow() ? RETevaluate : RETassign;
     case no_getgraphresult:
+        if (expr->hasProperty(_distributed_Atom))
+            return 0;
         return expr->isDatarow() ? RETevaluate : RETassign;
     case no_temptable:
         return RETassign;

ecl/hqlcpp/hqliproj.cpp

@@ -1536,7 +1536,6 @@ ProjectExprKind ImplicitProjectTransformer::getProjectExprKind(IHqlExpression *
     case no_rollupgroup:
     case no_regroup:
     case no_loop:
-    case no_loop2:
     case no_graphloop:
     case no_filtergroup:            //anything else would be tricky...
     case no_normalizegroup:

ecl/hqlcpp/hqllib.cpp

@@ -154,7 +154,7 @@ unsigned HqlCppLibrary::queryOutputIndex(_ATOM name) const
 HqlCppLibraryImplementation::HqlCppLibraryImplementation(HqlCppTranslator & _translator, IHqlExpression * libraryInterface, IHqlExpression * _libraryId, ClusterType _clusterType)
 : HqlCppLibrary(_translator, libraryInterface, _clusterType), libraryId(_libraryId)
 {
-    inputMapper.mapRealToLogical(inputExprs, logicalParams, libraryId, (clusterType != HThorCluster));
+    inputMapper.mapRealToLogical(inputExprs, logicalParams, libraryId, (clusterType != HThorCluster), translator.targetThor());
 }
 
 

ecl/hqlcpp/hqlresource.cpp

@@ -1184,6 +1184,8 @@ IHqlExpression * ResourcerInfo::createSpilledRead(IHqlExpression * spillReason)
         IHqlExpression * recordCountAttr = queryRecordCountInfo(original);
         if (recordCountAttr)
             args.append(*LINK(recordCountAttr));
+        if (options->targetThor() && original->isDataset() && !options->isChildQuery)
+            args.append(*createAttribute(_distributed_Atom));
         dataset.setown(createDataset(no_getgraphresult, args));
     }
     else if (useGlobalResult())
@@ -2179,7 +2181,7 @@ protected:
         case no_thisnode:
             throwUnexpected();
         case no_getgraphresult:
-            if (expr->hasProperty(_streaming_Atom))
+            if (expr->hasProperty(_streaming_Atom) || expr->hasProperty(_distributed_Atom))
             {
                 noteDataset(expr, expr, true);
                 return;
@@ -2195,7 +2197,7 @@ protected:
                 switch (ds->getOperator())
                 {
                 case no_getgraphresult:
-                    if (!expr->hasProperty(_streaming_Atom))
+                    if (!expr->hasProperty(_streaming_Atom) && !expr->hasProperty(_distributed_Atom))
                         break;
                     //fallthrough
                 case no_getgraphloopresult:

ecl/hqlcpp/hqlresource.ipp

@@ -73,6 +73,7 @@ public:
     inline bool canSplit() const            { return targetClusterType != HThorCluster; }
     inline bool checkResources() const      { return isThorCluster(targetClusterType) && !isChildQuery; }
     inline bool targetRoxie() const         { return targetClusterType == RoxieCluster; }
+    inline bool targetThor() const          { return targetClusterType == ThorCluster || targetClusterType == ThorLCRCluster; }
 };
 
 struct CResources : public CInterface