Merge pull request #10499 from ghalliday/issue18516

HPCC-18516 Fix various problems with the graph scope iterator

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

common/workunit/workunit.cpp

@@ -1179,16 +1179,24 @@ private:
         SGraphFirst,
         SGraphEnd,
         SGraphNext,
+
         SSubGraphFirstEdge,
         SSubGraphFirstActivity,
-        SSubGraphFirstSubGraph,
         SSubGraphEnd,
         SSubGraphNext,
+
         SEdgeNext,
         SEdgeEnd,
-        SActivityFirstSubGraph,
+
         SActivityNext,
         SActivityEnd,
+
+        SChildGraphFirstEdge,
+        SChildGraphFirstSubGraph,
+        SChildGraphFirst,
+        SChildGraphNext,
+        SChildGraphEnd,
+
         SDone
     };
     State state = SDone;
@@ -1357,9 +1365,16 @@ private:
             switch (state)
             {
             case SGraph:
-                graphIter->query().getName(StringBufferAdaptor(curScopeName.clear()));
+            {
+                IConstWUGraph & graph = graphIter->query();
+                unsigned wfid = graph.getWfid();
+                curScopeName.clear();
+                if (wfid != 0)
+                    curScopeName.append(WorkflowScopePrefix).append(wfid).append(':');
+                graph.getName(StringBufferAdaptor(curScopeName));
                 scopeType = SSTgraph;
                 return true;
+            }
             case SSubGraph:
                 scopeId.set(SubGraphScopePrefix).append(treeIters.tos().query().getPropInt("@id"));
                 pushScope(scopeId);
@@ -1371,10 +1386,36 @@ private:
                 scopeType = SSTedge;
                 return true;
             case SActivity:
+                if (treeIters.tos().query().getPropInt("att[@name='_kind']/@value") == TAKsubgraph)
+                {
+                    state = SActivityNext;
+                    break;
+                }
                 scopeId.set(ActivityScopePrefix).append(treeIters.tos().query().getPropInt("@id"));
                 pushScope(scopeId);
                 scopeType = SSTactivity;
                 return true;
+            case SChildGraph:
+            {
+                unsigned numIters = treeIters.ordinality();
+                //This should really be implemented by a filter on the node - but it would require _kind/_parentActivity to move to the node tag
+                if (treeIters.tos().query().getPropInt("att[@name='_kind']/@value") != TAKsubgraph)
+                {
+                    state = SChildGraphNext;
+                    break;
+                }
+                unsigned parentActivityId = treeIters.item(numIters-2).query().getPropInt("@id");
+                unsigned parentId = treeIters.tos().query().getPropInt("att[@name='_parentActivity']/@value");
+                if (parentId != parentActivityId)
+                {
+                    state = SChildGraphNext;
+                    break;
+                }
+                scopeId.set(ChildGraphScopePrefix).append(treeIters.tos().query().getPropInt("@id"));
+                pushScope(scopeId);
+                scopeType = SSTchildgraph;
+                return true;
+            }
             //Graph iteration
             case SGraphFirst:
                 if (!graphIter->first())
@@ -1409,6 +1450,19 @@ private:
                     state = SGraphFirstSubGraph;
                 break;
             }
+            case SChildGraphFirstEdge:
+            {
+                Owned<IPropertyTreeIterator> treeIter = treeIters.tos().query().getElements("att/graph/edge");
+                if (treeIter && treeIter->first())
+                {
+                    treeIter.setown(createSortedIterator(*treeIter, compareEdgeNode));
+                    pushIterator(treeIter, SChildGraphFirstSubGraph);
+                    state = SEdge;
+                }
+                else
+                    state = SChildGraphFirstSubGraph;
+                break;
+            }
             case SEdgeEnd:
                 popScope();
                 state = SEdgeNext;
@@ -1434,6 +1488,19 @@ private:
                     state = SGraphNext;
                 break;
             }
+            case SChildGraphFirstSubGraph:
+            {
+                Owned<IPropertyTreeIterator> treeIter = treeIters.tos().query().getElements("att/graph/node");
+                if (treeIter && treeIter->first())
+                {
+                    treeIter.setown(createSortedIterator(*treeIter, compareSubGraphNode));
+                    pushIterator(treeIter, SChildGraphEnd);
+                    state = SSubGraph;
+                }
+                else
+                    state = SChildGraphEnd;
+                break;
+            }
             case SSubGraphFirstEdge:
             {
                 Owned<IPropertyTreeIterator> treeIter = treeIters.tos().query().getElements("att/graph/edge");
@@ -1464,6 +1531,10 @@ private:
                 popScope();
                 state = SSubGraphNext;
                 break;
+            case SChildGraphEnd:
+                popScope();
+                state = SChildGraphNext;
+                break;
             case SSubGraphNext:
                 if (treeIters.tos().next())
                     state = SSubGraph;
@@ -1480,14 +1551,23 @@ private:
                 else
                     state = popIterator();
                 break;
-            case SActivityFirstSubGraph:
+            case SChildGraphNext:
+                if (treeIters.tos().next())
+                    state = SChildGraph;
+                else
+                    state = popIterator();
+                break;
+            case SChildGraphFirst:
             {
-                Owned<IPropertyTreeIterator> treeIter = treeIters.tos().query().getElements("att/graph/node");
+                unsigned numIters = treeIters.ordinality();
+                IPropertyTreeIterator & graphIter = treeIters.item(numIters-2);
+                Owned<IPropertyTreeIterator> treeIter = graphIter.query().getElements("att/graph/node");
+                //Really want to filter by <att name="_parentActivity" value="<parentid>">
                 if (treeIter && treeIter->first())
                 {
                     treeIter.setown(createSortedIterator(*treeIter, compareSubGraphNode));
-                    pushIterator(treeIter, SActivityNext);
-                    state = SSubGraph;
+                    pushIterator(treeIter, SActivityEnd);
+                    state = SChildGraph;
                 }
                 else
                     state = SActivityEnd;
@@ -1526,6 +1606,8 @@ private:
             state = SGraphFirstEdge;
             break;
         case SChildGraph:
+            state = SChildGraphFirstEdge;
+            break;
         case SSubGraph:
             state = SSubGraphFirstEdge;
             break;
@@ -1533,7 +1615,7 @@ private:
             state = SEdgeEnd;
             break;
         case SActivity:
-            state = SActivityFirstSubGraph;
+            state = SChildGraphFirst;
             break;
         case SDone:
             return false;
@@ -1552,7 +1634,8 @@ private:
             state = SGraphEnd;
             break;
         case SChildGraph:
-            throwUnexpected(); // MORE!
+            state = SChildGraphEnd;
+            break;
         case SSubGraph:
             state = SSubGraphEnd;
             break;
@@ -3498,8 +3581,8 @@ public:
             { return c->addLocalFileUpload(type, source, destination, eventTag); }
     virtual IWUResult * updateGlobalByName(const char * name)
             { return c->updateGlobalByName(name); }
-    virtual void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml)
-            { c->createGraph(name, label, type, xgmml); }
+    virtual void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml, unsigned wfid)
+            { c->createGraph(name, label, type, xgmml, wfid); }
     virtual IWUQuery * updateQuery()
             { return c->updateQuery(); }
     virtual IWUWebServicesInfo * updateWebServicesInfo(bool create)
@@ -9079,7 +9162,7 @@ IConstWUGraph* CLocalWorkUnit::getGraph(const char *qname) const
     return NULL;
 }
 
-void CLocalWorkUnit::createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml)
+void CLocalWorkUnit::createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml, unsigned wfid)
 {
     CriticalBlock block(crit);
     if (!graphs.length())
@@ -9090,6 +9173,7 @@ void CLocalWorkUnit::createGraph(const char * name, const char *label, WUGraphTy
     q->setName(name);
     q->setLabel(label);
     q->setType(type);
+    q->setWfid(wfid);
     q->setXGMMLTree(xgmml);
     graphs.append(*q);
 }
@@ -9137,6 +9221,11 @@ void CLocalWUGraph::setLabel(const char *str)
     p->setProp("@label", str);
 }
 
+void CLocalWUGraph::setWfid(unsigned wfid)
+{
+    p->setPropInt("@wfid", wfid);
+}
+
 void CLocalWUGraph::setXGMML(const char *str)
 {
     setXGMMLTree(createPTreeFromXMLString(str,ipt_lowmem));
@@ -9282,6 +9371,11 @@ IStringVal & CLocalWUGraph::getTypeName(IStringVal &str) const
     return str;
 }
 
+unsigned CLocalWUGraph::getWfid() const
+{
+    return p->getPropInt("@wfid", 0);
+}
+
 void CLocalWUGraph::setType(WUGraphType _type)
 {
     setEnum(p, "@type", _type, graphTypes);

common/workunit/workunit.hpp

@@ -198,6 +198,7 @@ interface IConstWUGraphMeta : extends IInterface
     virtual IStringVal & getTypeName(IStringVal & ret) const = 0;
     virtual WUGraphType getType() const = 0;
     virtual WUGraphState getState() const = 0;
+    virtual unsigned getWfid() const = 0;
 };
 
 interface IConstWUGraph : extends IConstWUGraphMeta
@@ -1317,7 +1318,7 @@ interface IWorkUnit : extends IConstWorkUnit
     virtual void deschedule() = 0;
     virtual unsigned addLocalFileUpload(LocalFileUploadType type, const char * source, const char * destination, const char * eventTag) = 0;
     virtual IWUResult * updateGlobalByName(const char * name) = 0;
-    virtual void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml) = 0;
+    virtual void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml, unsigned wfid) = 0;
     virtual IWUQuery * updateQuery() = 0;
     virtual IWUWebServicesInfo * updateWebServicesInfo(bool create) = 0;
     virtual IWUPlugin * updatePluginByName(const char * name) = 0;

common/workunit/workunit.ipp

@@ -394,7 +394,7 @@ public:
     void deschedule();
     unsigned addLocalFileUpload(LocalFileUploadType type, char const * source, char const * destination, char const * eventTag);
     IWUResult * updateGlobalByName(const char * name);
-    void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml);
+    void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml, unsigned wfid);
     IWUQuery * updateQuery();
     IWUWebServicesInfo* updateWebServicesInfo(bool create);
 
@@ -673,18 +673,20 @@ public:
     IMPLEMENT_IINTERFACE;
     CLocalWUGraph(const CLocalWorkUnit &owner, IPropertyTree *p);
 
-    virtual IStringVal & getXGMML(IStringVal & ret, bool mergeProgress) const;
-    virtual IStringVal & getName(IStringVal & ret) const;
-    virtual IStringVal & getLabel(IStringVal & ret) const;
-    virtual IStringVal & getTypeName(IStringVal & ret) const;
-    virtual WUGraphType getType() const;
-    virtual WUGraphState getState() const;
-    virtual IPropertyTree * getXGMMLTree(bool mergeProgress) const;
-    virtual IPropertyTree * getXGMMLTreeRaw() const;
+    virtual IStringVal & getXGMML(IStringVal & ret, bool mergeProgress) const override;
+    virtual IStringVal & getName(IStringVal & ret) const override;
+    virtual IStringVal & getLabel(IStringVal & ret) const override;
+    virtual IStringVal & getTypeName(IStringVal & ret) const override;
+    virtual WUGraphType getType() const override;
+    virtual WUGraphState getState() const override;
+    virtual unsigned getWfid() const override;
+    virtual IPropertyTree * getXGMMLTree(bool mergeProgress) const override;
+    virtual IPropertyTree * getXGMMLTreeRaw() const override;
 
     void setName(const char *str);
     void setLabel(const char *str);
     void setType(WUGraphType type);
+    void setWfid(unsigned wfid);
     void setXGMML(const char *str);
     void setXGMMLTree(IPropertyTree * tree);
 };

ecl/hqlcpp/hqlgraph.cpp

@@ -259,7 +259,7 @@ void LogicalGraphCreator::createLogicalGraph(HqlExprArray & exprs)
     ForEachItemIn(i, exprs)
         createRootGraphActivity(&exprs.item(i));
 //  endSubGraph();
-    wu->createGraph("Logical", NULL, GraphTypeEcl, graph.getClear());
+    wu->createGraph("Logical", NULL, GraphTypeEcl, graph.getClear(), 0);
 }
 
 

ecl/hqlcpp/hqlhtcpp.cpp

@@ -5914,7 +5914,8 @@ bool HqlCppTranslator::buildCpp(IHqlCppInstance & _code, HqlQueryContext & query
         ForEachItemIn(i2, graphs)
         {
             GeneratedGraphInfo & cur = graphs.item(i2);
-            wu()->createGraph(cur.name, cur.label, GraphTypeActivities, cur.xgmml.getClear());
+            unsigned wfid = cur.xgmml->getPropInt("@wfid");
+            wu()->createGraph(cur.name, cur.label, GraphTypeActivities, cur.xgmml.getClear(), wfid);
         }
 
         code->processIncludes();

plugins/cassandra/cassandrawu.cpp

@@ -2454,9 +2454,9 @@ public:
         }
     }
 
-    virtual void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml)
+    virtual void createGraph(const char * name, const char *label, WUGraphType type, IPropertyTree *xgmml, unsigned wfid)
     {
-        CPersistedWorkUnit::createGraph(name, label, type, xgmml);
+        CPersistedWorkUnit::createGraph(name, label, type, xgmml, wfid);
         VStringBuffer xpath("Graphs/Graph[@name='%s']", name);
         noteDirty(xpath, wuGraphsMappings);
     }

tools/wutool/wutool.cpp

@@ -1076,9 +1076,9 @@ protected:
         Owned<IWorkUnitFactory> factory = getWorkUnitFactory();
         Owned<IWorkUnit> createWu = factory->createWorkUnit("WuTest", NULL, NULL, NULL);
         StringBuffer wuid(createWu->queryWuid());
-        createWu->createGraph("Graph1", "graphLabel", GraphTypeActivities, createPTreeFromXMLString("<graph/>"));
-        createWu->createGraph("Graph2", "graphLabel", GraphTypeActivities, createPTreeFromXMLString("<graph/>"));
-        createWu->createGraph("Graph3", "graphLabel", GraphTypeEcl, createPTreeFromXMLString("<graph/>"));
+        createWu->createGraph("Graph1", "graphLabel", GraphTypeActivities, createPTreeFromXMLString("<graph/>"), 0);
+        createWu->createGraph("Graph2", "graphLabel", GraphTypeActivities, createPTreeFromXMLString("<graph/>"), 1);
+        createWu->createGraph("Graph3", "graphLabel", GraphTypeEcl, createPTreeFromXMLString("<graph/>"), 2);
         createWu->setState(WUStateCompleted);
         createWu->commit();
         createWu.clear();
@@ -1198,7 +1198,7 @@ protected:
         Owned<IWorkUnitFactory> factory = getWorkUnitFactory();
         Owned<IWorkUnit> createWu = factory->createWorkUnit("WuTest", NULL, NULL, NULL);
         StringBuffer wuid(createWu->queryWuid());
-        createWu->createGraph("graph1", "graphLabel", GraphTypeActivities, createPTreeFromXMLString("<graph><node id='1'/></graph>"));
+        createWu->createGraph("graph1", "graphLabel", GraphTypeActivities, createPTreeFromXMLString("<graph><node id='1'/></graph>"), 1);
         createWu->setState(WUStateCompleted);
         createWu->commit();
         createWu.clear();