Browse Source

HPCC-18262 Improve the syntax for filtering workunit scopes

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 7 years ago
parent
commit
2a2062b622

+ 199 - 39
common/workunit/workunit.cpp

@@ -793,7 +793,7 @@ protected:
     {
         IPropertyTree & curSubGraph = subgraphIter->query();
 
-        StatisticCreatorType creatorType = queryCreatorType(curSubGraph.queryProp("@c"));
+        StatisticCreatorType creatorType = queryCreatorType(curSubGraph.queryProp("@c"), SCTnone);
         const char * creator = curSubGraph.queryProp("@creator");
 
         //MORE: Check minVersion and allow early filtering
@@ -2037,7 +2037,7 @@ protected:
 
 
 //Extract an argument of the format abc[def[ghi...,]],... as (abc,def[...])
-static bool extractOption(const char * & finger, StringAttr & option, StringAttr & arg)
+static bool extractOption(const char * & finger, StringBuffer & option, StringBuffer & arg)
 {
     const char * start = finger;
     if (!*start)
@@ -2072,7 +2072,7 @@ static bool extractOption(const char * & finger, StringAttr & option, StringAttr
                 if (--braDepth == 0)
                 {
                     end = cur;
-                    arg.set(bra+1, cur - (bra+1));
+                    arg.append(cur - (bra+1), bra+1);
                 }
             }
             break;
@@ -2084,14 +2084,15 @@ static bool extractOption(const char * & finger, StringAttr & option, StringAttr
     if (braDepth != 0)
         throw makeStringExceptionV(0, "Mismatched ] in filter : %s", start);
 
+    option.clear();
     if (bra)
     {
         if (cur != end+1)
             throw makeStringExceptionV(0, "Text follows closing bracket: %s", end);
-        option.set(start, bra-start);
+        option.append(bra-start, start);
     }
     else
-        option.set(start, cur-start);
+        option.append(cur-start, start);
 
     if (next)
         finger = cur+1;
@@ -2100,10 +2101,68 @@ static bool extractOption(const char * & finger, StringAttr & option, StringAttr
     return true;
 }
 
-enum { FOscope, FOstype, FOid, FOdepth, FOsource, FOwhere, FOmatched, FOnested, FOinclude, FOproperties, FOstatistic, FOattribute, FOhint, FOmeasure, FOversion }; // Enum to clarify the swicth statement below
-static constexpr const char * filterOptions[] = { "scope", "stype", "id", "depth", "source", "where", "matched", "nested", "include", "prop", "stat", "attr", "hint", "measure", "version", nullptr };
-static constexpr EnumMapping sourceMappings[] = { { SSFsearchGlobalStats, "global" }, { SSFsearchGraphStats, "stats" }, { SSFsearchGraph, "graph" }, { SSFsearchExceptions, "exception" }, { (int)SSFsearchAll, "all" }, { 0, nullptr } };
-static constexpr EnumMapping propertyMappings[] = { { PTnone, "none" }, { PTstatistics, "stat" }, { PTattributes, "attr" }, { PThints, "hint" }, { PTscope, "scope" }, { PTall, "all" }, { 0, nullptr } };
+static unsigned readOptValue(const char * start, const char * end, unsigned dft, const char * type)
+{
+    if (start == end)
+        return dft;
+    char * next;
+    unsigned value = strtoll(start, &next, 10);
+    if (next != end)
+        throw makeStringExceptionV(0, "Unexpected characters in %s option '%s'", type, next);
+    return value;
+}
+
+static unsigned readValue(const char * start, const char * type)
+{
+    if (*start == '\0')
+        throw makeStringExceptionV(0, "Expected a value for the %s option", type);
+
+    char * next;
+    unsigned value = strtoll(start, &next, 10);
+    if (*next != '\0')
+        throw makeStringExceptionV(0, "Unexpected characters in %s option '%s'", type, next);
+    return value;
+}
+
+/*
+Scope service matching Syntax:  * indicates
+Which items are matched:
+    scope[<scope-id>]* | stype[<scope-type>]* | id[<id>]* - which scopes should be matched?
+    depth[n | low..high] - range of depths to search for a match
+    source[global|stats|graph|all]* - which sources to search within the workunit
+    where[<statistickind> | <statistickind> (=|<|<=|>|>=) value | <statistickind>=low..high] - a statistic filter
+Which items are include in the results:
+    matched[true|false] - are the matched scopes returned?
+    nested[<depth>|all] - how deep within a scope should be matched (default = 0 if matched[true], all if matched[false])
+    includetype[<scope-type>] - which scope types should be included?
+Which properties of the items are returned:
+    properties[statistics|hints|attributes|scope|all]
+    statistic[<statistic-kind>|none|all] - include statistic
+    attribute[<attribute-name>|none|all] - include attribute
+    hint[<hint-name>] - include hint
+    property[<statistic-kind>|<attribute-name>|<hint-name>] - include property
+    measure[<measure>] - all statistics with a particular measure
+    version[<version>] - minimum version to return
+*/
+
+enum { FOscope, FOstype, FOid, FOdepth, FOsource, FOwhere, FOmatched, FOnested, FOinclude, FOproperties, FOstatistic, FOattribute, FOhint, FOproperty, FOmeasure, FOversion, FOunknown };
+//Some of the following contains aliases for the same option e.g. stat and statistic
+static constexpr EnumMapping filterOptions[] = {
+        { FOscope, "scope" }, { FOstype, "stype" }, { FOid, "id" },
+        { FOdepth, "depth" }, { FOsource, "source" }, { FOwhere, "where" },
+        { FOmatched, "matched" }, { FOnested, "nested" },
+        { FOinclude, "include" }, { FOinclude, "includetype" },
+        { FOproperties, "props" }, { FOproperties, "properties" }, // some aliases
+        { FOstatistic, "stat" }, { FOstatistic, "statistic" }, { FOattribute, "attr" }, { FOattribute, "attribute" }, { FOhint, "hint" },
+        { FOproperty, "prop" }, { FOproperty, "property" },
+        { FOmeasure, "measure" }, { FOversion, "version" }, { 0, nullptr} };
+static constexpr EnumMapping sourceMappings[] = {
+        { SSFsearchGlobalStats, "global" }, { SSFsearchGraphStats, "stats" }, { SSFsearchGraphStats, "statistics" }, { SSFsearchGraph, "graph" }, { SSFsearchExceptions, "exception" },
+        { (int)SSFsearchAll, "all" }, { 0, nullptr } };
+static constexpr EnumMapping propertyMappings[] = {
+        { PTstatistics, "stat" }, { PTstatistics, "statistic" }, { PTattributes, "attr" }, { PTattributes, "attribute" }, { PThints, "hint" },
+        { PTstatistics, "stats" }, { PTstatistics, "statistics" }, { PTattributes, "attrs" }, { PTattributes, "attributes" }, { PThints, "hints" },
+        { PTnone, "none" }, { PTscope, "scope" }, { PTall, "all" }, { 0, nullptr } };
 WuScopeFilter::WuScopeFilter(const char * filter)
 {
     addFilter(filter);
@@ -2115,11 +2174,11 @@ WuScopeFilter & WuScopeFilter::addFilter(const char * filter)
     if (!filter)
         return *this;
 
-    StringAttr option;
-    StringAttr arg;
+    StringBuffer option;
+    StringBuffer arg;
     while (extractOption(filter, option, arg))
     {
-        switch (matchString(option, filterOptions))
+        switch (getEnum(option, filterOptions, FOunknown))
         {
         case FOscope:
             addScope(arg);
@@ -2132,17 +2191,31 @@ WuScopeFilter & WuScopeFilter::addFilter(const char * filter)
             break;
         case FOdepth:
         {
+            //Allow depth[n], depth[a,b] or depth[a..b]
             const char * comma = strchr(arg, ',');
+            const char * dotdot = strstr(arg, "..");
             if (comma)
             {
-                unsigned low = atoi(arg); // note: =0 if number omitted
+                unsigned low = readOptValue(arg, comma, 0, "depth");
                 if (comma[1])
-                    scopeFilter.setDepth(low, atoi(comma+1));
+                {
+                    scopeFilter.setDepth(low, readValue(comma+1, "depth"));
+                }
+                else
+                    scopeFilter.setDepth(low, UINT_MAX);
+            }
+            else if (dotdot)
+            {
+                unsigned low = readOptValue(arg, dotdot, 0, "depth");
+                if (dotdot[2])
+                    scopeFilter.setDepth(low, readValue(dotdot+2, "depth"));
                 else
                     scopeFilter.setDepth(low, UINT_MAX);
             }
             else
-                scopeFilter.setDepth(atoi(arg));
+            {
+                scopeFilter.setDepth(readValue(arg, "depth"));
+            }
             break;
         }
         case FOsource:
@@ -2155,14 +2228,24 @@ WuScopeFilter & WuScopeFilter::addFilter(const char * filter)
             setIncludeMatch(strToBool(arg));
             break;
         case FOnested:
-            setIncludeNesting(atoi(arg));
+            if (strieq(arg, "all"))
+                setIncludeNesting(UINT_MAX);
+            else if (isdigit(*arg))
+                setIncludeNesting(atoi(arg));
+            else
+                throw makeStringExceptionV(0, "Expected a value for the nesting depth: %s", arg.str());
             break;
         case FOinclude:
             setIncludeScopeType(arg);
             break;
         case FOproperties:
-            addOutputProperties((WuPropertyTypes)getEnum(arg, propertyMappings));
+        {
+            WuPropertyTypes prop = (WuPropertyTypes)getEnum(arg, propertyMappings, PTunknown);
+            if (prop == PTunknown)
+                throw makeStringExceptionV(0, "Unexpected properties '%s'", arg.str());
+            addOutputProperties(prop);
             break;
+        }
         case FOstatistic:
             addOutputStatistic(arg);
             break;
@@ -2172,11 +2255,17 @@ WuScopeFilter & WuScopeFilter::addFilter(const char * filter)
         case FOhint:
             addOutputHint(arg);
             break;
+        case FOproperty:
+            addOutput(arg);
+            break;
         case FOmeasure:
             setMeasure(arg);
             break;
         case FOversion:
-            minVersion = atoi64(arg);
+            if (isdigit(*arg))
+                minVersion = atoi64(arg);
+            else
+                throw makeStringExceptionV(0, "Expected a value for the version: %s", arg.str());
             break;
         default:
             throw makeStringExceptionV(0, "Unrecognised filter option: %s", option.str());
@@ -2187,27 +2276,36 @@ WuScopeFilter & WuScopeFilter::addFilter(const char * filter)
 
 WuScopeFilter & WuScopeFilter::addScope(const char * scope)
 {
+    validateScope(scope);
     scopeFilter.addScope(scope);
     return *this;
 }
 
 WuScopeFilter & WuScopeFilter::addScopeType(const char * scopeType)
 {
-    scopeFilter.addScopeType(queryScopeType(scopeType));
+    if (scopeType)
+    {
+        StatisticScopeType sst = queryScopeType(scopeType, SSTmax);
+        if (sst == SSTmax)
+            throw makeStringExceptionV(0, "Unrecognised scope type '%s'", scopeType);
+
+        scopeFilter.addScopeType(sst);
+    }
     return *this;
 }
 
 WuScopeFilter & WuScopeFilter::addId(const char * id)
 {
+    validateScopeId(id);
     scopeFilter.addId(id);
     return *this;
 }
 
 WuScopeFilter & WuScopeFilter::addOutput(const char * prop)
 {
-    if (queryStatisticKind(prop) != StKindNone)
+    if (queryStatisticKind(prop, StMax) != StMax)
         addOutputStatistic(prop);
-    else if (queryWuAttribute(prop) != WANone)
+    else if (queryWuAttribute(prop, WAMax) != WAMax)
         addOutputAttribute(prop);
     else
         addOutputHint(prop);
@@ -2216,7 +2314,14 @@ WuScopeFilter & WuScopeFilter::addOutput(const char * prop)
 
 WuScopeFilter & WuScopeFilter::addOutputStatistic(const char * prop)
 {
-    return addOutputStatistic(queryStatisticKind(prop));
+    if (!prop)
+        return *this;
+
+    StatisticKind kind = queryStatisticKind(prop, StMax);
+    if (kind == StMax)
+        throw makeStringExceptionV(0, "Unrecognised statistic '%s'", prop);
+
+    return addOutputStatistic(kind);
 }
 
 WuScopeFilter & WuScopeFilter::addOutputStatistic(StatisticKind stat)
@@ -2236,7 +2341,14 @@ WuScopeFilter & WuScopeFilter::addOutputStatistic(StatisticKind stat)
 
 WuScopeFilter & WuScopeFilter::addOutputAttribute(const char * prop)
 {
-    return addOutputAttribute(queryWuAttribute(prop));
+    if (!prop)
+        return *this;
+
+    WuAttr attr = queryWuAttribute(prop, WAMax);
+    if (attr == WAMax)
+        throw makeStringExceptionV(0, "Unrecognised attribute '%s'", prop);
+
+    return addOutputAttribute(attr);
 }
 
 WuScopeFilter & WuScopeFilter::addOutputAttribute(WuAttr attr)
@@ -2287,14 +2399,26 @@ WuScopeFilter & WuScopeFilter::setIncludeNesting(unsigned depth)
 
 WuScopeFilter & WuScopeFilter::setIncludeScopeType(const char * scopeType)
 {
-    include.scopeTypes.append(queryScopeType(scopeType));
+    if (scopeType)
+    {
+        StatisticScopeType sst = queryScopeType(scopeType, SSTmax);
+        if (sst == SSTmax)
+            throw makeStringExceptionV(0, "Unrecognised scope type '%s'", scopeType);
+
+        include.scopeTypes.append(sst);
+    }
     return *this;
 }
 
 WuScopeFilter & WuScopeFilter::setMeasure(const char * measure)
 {
-    desiredMeasure = queryMeasure(measure);
-    properties |= PTstatistics;
+    if (measure)
+    {
+        desiredMeasure = queryMeasure(measure, SMeasureNone);
+        if (desiredMeasure == SMeasureNone)
+            throw makeStringExceptionV(0, "Unrecognised measure '%s'", measure);
+        properties |= PTstatistics;
+    }
     return *this;
 }
 
@@ -2319,8 +2443,10 @@ WuScopeFilter & WuScopeFilter::addRequiredStat(StatisticKind statKind)
     return *this;
 }
 
-//This does not strictly validate - invalid filters may be accepted
-//process a filter of the form <statistic-name> [=|<|<=|>|>= <value>] or <statistic-name>=[<low>],[<high>]
+//process a filter in one of the following forms:
+//  <statistic-name>
+//  <statistic-name> (=|<|<=|>|>=) <value>
+//  <statistic-name>=[<low>]..[<high>]
 
 void WuScopeFilter::addRequiredStat(const char * filter)
 {
@@ -2330,13 +2456,33 @@ void WuScopeFilter::addRequiredStat(const char * filter)
         cur++;
 
     StringBuffer statisticName(cur-stat, stat);
-    StatisticKind statKind = queryStatisticKind(statisticName);
+    StatisticKind statKind = queryStatisticKind(statisticName, StKindNone);
     if (statKind == StKindNone)
         throw makeStringExceptionV(0, "Unknown statistic name '%s'", statisticName.str());
 
+    //Skip any spaces before a comparison operator.
+    while (*cur && isspace(*cur))
+        cur++;
+
+    //Save the operator, and skip over any non digits.
     const char * op = cur;
-    while (*cur && !isdigit(*cur))
+    switch (*op)
+    {
+    case '=':
         cur++;
+        break;
+    case '<':
+    case '>':
+        if (op[1] == '=')
+            cur += 2;
+        else
+            cur++;
+        break;
+    case '\0':
+        break;
+    default:
+        throw makeStringExceptionV(0, "Unknown comparison '%s'", op);
+    }
 
     const char * next;
     stat_type value = readStatisticValue(cur, &next, queryMeasure(statKind));
@@ -2344,15 +2490,22 @@ void WuScopeFilter::addRequiredStat(const char * filter)
     stat_type highValue = MaxStatisticValue;
     switch (op[0])
     {
-    case ']':
-        break;
     case '=':
     {
+        //Allow a,b or a..b to specify a range - either bound may be omitted.
         if (next[0] == ',')
         {
             lowValue = value;
-            if (next[1] != '\0')
-                highValue = readStatisticValue(next+1, nullptr, queryMeasure(statKind));
+            next++;
+            if (*next != '\0')
+                highValue = readStatisticValue(next, &next, queryMeasure(statKind));
+        }
+        else if (strncmp(next, "..", 2) == 0)
+        {
+            lowValue = value;
+            next += 2;
+            if (*next != '\0')
+                highValue = readStatisticValue(next, &next, queryMeasure(statKind));
         }
         else
         {
@@ -2375,12 +2528,17 @@ void WuScopeFilter::addRequiredStat(const char * filter)
         break;
     }
 
+    if (*next)
+        throw makeStringExceptionV(0, "Trailing characters in where '%s'", next);
+
     requiredStats.emplace_back(statKind, lowValue, highValue);
 }
 
 WuScopeFilter & WuScopeFilter::addSource(const char * source)
 {
-    WuScopeSourceFlags mask = (WuScopeSourceFlags)getEnum(source, sourceMappings);
+    WuScopeSourceFlags mask = (WuScopeSourceFlags)getEnum(source, sourceMappings, SSFunknown);
+    if (mask == SSFunknown)
+        throw makeStringExceptionV(0, "Unexpected source '%s'", source);
     if (!mask)
         sourceFlags = mask;
     else
@@ -2416,6 +2574,8 @@ void WuScopeFilter::finishedFilter()
     assertex(!optimized);
     optimized = true;
 
+    if ((include.nestedDepth == 0) && !include.matchedScope)
+        include.nestedDepth = UINT_MAX;
     preFilterScope = include.matchedScope && (include.nestedDepth == 0);
     if (scopeFilter.canAlwaysPreFilter())
         preFilterScope = true;
@@ -10729,17 +10889,17 @@ IStringVal & CLocalWUStatistic::getFormattedValue(IStringVal & str) const
 
 StatisticCreatorType CLocalWUStatistic::getCreatorType() const
 {
-    return queryCreatorType(p->queryProp("@c"));
+    return queryCreatorType(p->queryProp("@c"), SCTnone);
 }
 
 StatisticScopeType CLocalWUStatistic::getScopeType() const
 {
-    return queryScopeType(p->queryProp("@s"));
+    return queryScopeType(p->queryProp("@s"), SSTnone);
 }
 
 StatisticKind CLocalWUStatistic::getKind() const
 {
-    return queryStatisticKind(p->queryProp("@kind"));
+    return queryStatisticKind(p->queryProp("@kind"), StKindNone);
 }
 
 const char * CLocalWUStatistic::queryScope() const
@@ -10752,7 +10912,7 @@ const char * CLocalWUStatistic::queryScope() const
 
 StatisticMeasure CLocalWUStatistic::getMeasure() const
 {
-    return queryMeasure(p->queryProp("@unit"));
+    return queryMeasure(p->queryProp("@unit"), SMeasureNone);
 }
 
 unsigned __int64 CLocalWUStatistic::getValue() const

+ 4 - 2
common/workunit/workunit.hpp

@@ -1014,6 +1014,7 @@ enum WuPropertyTypes : unsigned
     PThints                 = 0x04,
     PTscope                 = 0x08, // Just the existence of the scope is interesting
     PTall                   = 0xFF,
+    PTunknown               = 0x80000000,
 };
 BITMASK_ENUM(WuPropertyTypes);
 
@@ -1024,7 +1025,8 @@ enum WuScopeSourceFlags : unsigned
     SSFsearchGraphStats     = 0x0002,
     SSFsearchGraph          = 0x0004,
     SSFsearchExceptions     = 0x0008,
-    SSFsearchAll            = UINT_MAX,
+    SSFsearchAll            = 0x7fffffff,
+    SSFunknown              = 0x80000000,
 };
 BITMASK_ENUM(WuScopeSourceFlags);
 
@@ -1093,7 +1095,7 @@ public:
     struct
     {
         bool matchedScope = true;
-        unsigned nestedDepth = UINT_MAX;
+        unsigned nestedDepth = 0;
         UnsignedArray scopeTypes;
     } include;
 

+ 2 - 2
common/workunit/wuattr.cpp

@@ -71,7 +71,7 @@ const char * queryWuAttributeName(WuAttr kind)
     return nullptr;
 }
 
-WuAttr queryWuAttribute(const char * kind)
+WuAttr queryWuAttribute(const char * kind, WuAttr dft)
 {
     //MORE: This needs to use a hash table
     for (unsigned i=WANone; i < WAMax; i++)
@@ -79,7 +79,7 @@ WuAttr queryWuAttribute(const char * kind)
         if (strieq(kind, attrInfo[i-WANone].name))
             return (WuAttr)i;
     }
-    return WANone;
+    return dft;
 }
 
 extern WORKUNIT_API const char * queryAttributeValue(IPropertyTree & src, WuAttr kind)

+ 1 - 1
common/workunit/wuattr.hpp

@@ -46,7 +46,7 @@ enum WuAttr : unsigned
 };
 
 extern WORKUNIT_API const char * queryWuAttributeName(WuAttr kind);
-extern WORKUNIT_API WuAttr queryWuAttribute(const char * kind);
+extern WORKUNIT_API WuAttr queryWuAttribute(const char * kind, WuAttr dft);
 extern WORKUNIT_API const char * queryAttributeValue(IPropertyTree & src, WuAttr kind);
 extern WORKUNIT_API WuAttr queryGraphAttrToWuAttr(const char * name);
 extern WORKUNIT_API WuAttr queryGraphChildAttToWuAttr(const char * name);

+ 1 - 1
ecl/eclccserver/eclccserver.cpp

@@ -201,7 +201,7 @@ class EclccCompileThread : implements IPooledThread, implements IErrorReporter,
                 unsigned __int64 cnt = timing->getPropInt64("@count");
                 const char * scope = timing->queryProp("@scope");
                 StatisticScopeType scopeType = (StatisticScopeType)timing->getPropInt("@scopeType");
-                StatisticKind kind = queryStatisticKind(timing->queryProp("@kind"));
+                StatisticKind kind = queryStatisticKind(timing->queryProp("@kind"), StKindNone);
                 workunit->setStatistic(queryStatisticsComponentType(), queryStatisticsComponentName(), scopeType, scope, kind, NULL, nval, cnt, nmax, StatsMergeReplace);
             }
             else

+ 74 - 43
ecl/regress/testwudetail

@@ -19,12 +19,8 @@ function daliadmin {
     echo
 }
 
-function xdaliadmin {
-    daliadmin $1
-    donothing=x
-}
-
 daliadmincmd=/home/gavin/buildr/RelWithDebInfo/bin/daliadmin
+# daliadmincmd=/home/gavin/build/Debug/bin/daliadmin
 #The following should be TimeLocalExecute, but thor needs to start publishing it.
 timeattr=TimeMaxLocalExecute
 
@@ -32,89 +28,124 @@ searchsg='graph1:sg1'
 searchid='sg1'
 searchsg2='graph1:sg30'
 
+### The Tests #####################################
+# Check invalid filters
+daliadmin magic[true]
+daliadmin source[blah]
+daliadmin source[global,stats]      # lists not yet supported - possibly in the future
+daliadmin scope[rubbish:xyz]
+daliadmin scope[graph1:xyz]
+daliadmin id[xyz]
+daliadmin stype[graph1]
+daliadmin depth[]
+daliadmin depth[x]
+daliadmin depth[x..]
+daliadmin depth[..x]
+daliadmin depth[100..1]
+
+daliadmin where[]
+daliadmin where[rubbish]
+daliadmin where[NumStarts!=3]
+daliadmin where[numstarts=x]
+
+daliadmin matched[maybe]            # not caught
+daliadmin nested[allofit]
+daliadmin include[supergraph]
+daliadmin includetype[graph1]
+
+daliadmin properties[magic]
+daliadmin STAT[fish]
+daliadmin attribute[fish]
+
+daliadmin measure[quantum]
+daliadmin measure[]
+daliadmin version[]
+
+#Check various legal queries
 #Only the scope lists - filter by the different sources
-xdaliadmin prop[scope],source[all]
-xdaliadmin prop[scope],source[global]
-xdaliadmin prop[scope],source[stats]
-xdaliadmin prop[scope],source[graph]
-xdaliadmin prop[scope],source[exception]
+daliadmin props[scope],source[all]
+daliadmin props[scope],source[global]
+daliadmin props[scope],source[stats]
+daliadmin props[scope],source[graph]
+daliadmin props[scope],source[exception]
 
 #Filter which attributes are returned
 #All attributes
-xdaliadmin stat[all]
+daliadmin stat[all]
 #Only elapsed time attributes
-xdaliadmin stat[TimeElapsed]
+daliadmin stat[TimeElapsed]
 #Only hints
-xdaliadmin hint[all]
+daliadmin hint[all]
 #Only attributes
-xdaliadmin attr[all]
+daliadmin attr[all]
 
 # Provide a list of top level scopes/activities
-xdaliadmin depth[1],nested[0],source[global]
+daliadmin depth[1],source[global]
 
 
 # check extracting attributes at a fixed depth
-xdaliadmin depth[2],nested[0],source[global]
-xdaliadmin depth[2],nested[0],source[stats]
-xdaliadmin depth[2],nested[0],source[graph]
+daliadmin depth[2],source[global]
+daliadmin depth[2],source[stats]
+daliadmin depth[2],source[graph]
 
 # provide the values for [TimeLocalExecute] for all top level scopes { Top level heat map }
-xdaliadmin depth[1],nested[0],stat[$timeattr]
+daliadmin depth[1],stat[$timeattr]
 
 # Single root global scope (blank)
-xdaliadmin depth[0],nested[0],prop[scope]
+daliadmin depth[0],props[scope]
 
 # first level of scopes - both forms should be equivalent, but implemented differently
-xdaliadmin depth[1],nested[0],prop[scope]
-xdaliadmin depth[0],nested[1],matched[false],prop[scope]
+daliadmin depth[1],props[scope]
+daliadmin depth[0],nested[1],matched[false],props[scope]
 
 # second level of scopes - both forms should be equivalent, but implemented differently
-xdaliadmin depth[2],nested[0],prop[scope]
-xdaliadmin depth[1],nested[1],matched[false],prop[scope]
+daliadmin depth[2],props[scope]
+daliadmin depth[1],nested[1],matched[false],props[scope]
 
 # Provide all the children of element [n] in the global element [n] { Expand subgraphs within a graph }
-xdaliadmin scope[$searchsg],matched[false],nested[1],prop[scope]
+daliadmin scope[$searchsg],matched[false],nested[1],props[scope]
 
 # Provide the scope information for a particular activity { To map errors to graph locations }
-xdaliadmin id[$searchid],nested[0],prop[scope]
+daliadmin id[$searchid],props[scope]
 
 # Provide an entire heirarchy starting from a particular subgraph. { quick sub-graph view }
-xdaliadmin scope[$searchsg],prop[scope]
+daliadmin scope[$searchsg],nested[all],props[scope]
 
 # For all activities that read data, return the $timeattr. { A filtered heat map }
-xdaliadmin where[NumMinDiskReads],nested[0],stat[$timeattr],stat[NumMinDiskReads]
+daliadmin where[NumMinDiskReads],stat[$timeattr],stat[NumMinDiskReads]
 
 # Return children for 2 items - which nest inside each other
-xdaliadmin id[sg30],id[sg33],nested[1],matched[false],prop[scope]
+daliadmin id[sg30],id[sg33],nested[1],matched[false],props[scope]
 
 # For all activities in a particular subgraph return all time attributes { A multiple series bar chart }
-xdaliadmin id[sg1],include[activity],matched[false],measure[Time]
-xdaliadmin id[sg30],include[activity],measure[Time]
+daliadmin id[sg1],include[activity],matched[false],measure[Time],nested[all]
+daliadmin id[sg30],include[activity],measure[Time],nested[all]
 
 #Check matches within a range of depths.
-daliadmin include[activity],depth[5,7],nested[0],prop[scope]
+daliadmin include[activity],depth[5..7],props[scope]
 
 # All attributes for all activities within a subgraph (but not within a child subgraph) { a table of attributes for the current subgraph }
-xdaliadmin scope[$searchsg],nested[1],include[activity],matched[false],prop[all]
+daliadmin scope[$searchsg],nested[1],include[activity],matched[false],props[all]
 
 # For all activities return WhenFirstRow and TimeElapsed { gantt chart of activities within a subgraph }
-xdaliadmin scope[$searchsg],nested[1],include[activity],matched[false],stat[WhenMinFirstRow],stat[TimeMaxLocalExecute]
+daliadmin scope[$searchsg],nested[1],include[activity],matched[false],stat[WhenMinFirstRow],stat[TimeMaxLocalExecute]
 
 #MORE: Does the filter apply to the match criteria or the child values?  May also need having?
-xdaliadmin scope[$searchsg2],include[activity],matched[false],where[WhenMinFirstRow],measure[When],stat[WhenMinFirstRow],stat[TimeMaxLocalExecute]
+daliadmin scope[$searchsg2],include[activity],matched[false],where[WhenMinFirstRow],measure[When],stat[WhenMinFirstRow],stat[TimeMaxLocalExecute],nested[all]
 
 # Update all properties for a subgraph { e.g., for updating graph progress }.  version[1] implies no static values
-xdaliadmin scope[$searchsg],version[1],prop[all]
+daliadmin scope[$searchsg],version[1],props[all],nested[all]
 
 # Full dump of all statistics - to provide raw material for tables/client side analysis { stats tab, user xml output }
-xdaliadmin prop[stat],version[1]
+daliadmin props[stat],version[1],nested[all]
 
 # Find all activities which spent more than 100us processing.
-xdaliadmin stype[activity],nested[0],where[$timeattr],stat[$timeattr]
-xdaliadmin "stype[activity],nested[0],where[$timeattr>=1000000],stat[$timeattr]"
-xdaliadmin stype[activity],nested[0],where[$timeattr=1000000,],stat[$timeattr]
-xdaliadmin stype[activity],nested[0],where[$timeattr=1000000,2000000],stat[$timeattr]
-xdaliadmin stype[activity],nested[0],where[$timeattr=1000us,2ms],stat[$timeattr]
+daliadmin stype[activity],where[$timeattr],stat[$timeattr]
+daliadmin "stype[activity],where[$timeattr>=1000000],stat[$timeattr]"
+daliadmin stype[activity],where[$timeattr=1000000..],stat[$timeattr]
+daliadmin stype[activity],where[$timeattr=1000000..2000000],stat[$timeattr]
+daliadmin stype[activity],where[$timeattr=..2000000],stat[$timeattr]
+daliadmin stype[activity],where[$timeattr=1000us,2ms],stat[$timeattr]
 
 # Find all activities which spent more than 1 minute sorting { anomaly detection }
-xdaliadmin "stype[activity],nested[0],where[TimeSortElapsed>=60s],prop[scope]"
+daliadmin "stype[activity],where[TimeSortElapsed>=60s],props[scope]"

+ 1 - 0
system/jlib/jerror.hpp

@@ -32,6 +32,7 @@
 #define JLIBERR_BadUtf8InArguments              6001
 #define JLIBERR_InternalError                   6002
 #define JLIBERR_CppCompileError                 6003
+#define JLIBERR_UnexpectedValue                 6004
 
 //---- Text for all errors (make it easy to internationalise) ---------------------------
 

+ 82 - 54
system/jlib/jstats.cpp

@@ -23,6 +23,7 @@
 #include "jlog.hpp"
 #include "jregexp.hpp"
 #include "jfile.hpp"
+#include "jerror.hpp"
 #include <math.h>
 
 #ifdef _WIN32
@@ -69,10 +70,10 @@ static constexpr const char * const measureNames[] = { "", "all", "ns", "ts", "c
 static constexpr const char * const creatorTypeNames[]= { "", "all", "unknown", "hthor", "roxie", "roxie:s", "thor", "thor:m", "thor:s", "eclcc", "esp", "summary", NULL };
 static constexpr const char * const scopeTypeNames[] = { "", "all", "global", "graph", "subgraph", "activity", "allocator", "section", "compile", "dfu", "edge", "function", "workflow", "child", "unknown", nullptr };
 
-static unsigned matchString(const char * const * names, const char * search)
+static unsigned matchString(const char * const * names, const char * search, unsigned dft)
 {
     if (!search)
-        return 0;
+        return dft;
 
     if (streq(search, "*"))
         search = "all";
@@ -82,7 +83,7 @@ static unsigned matchString(const char * const * names, const char * search)
     {
         const char * next = names[i];
         if (!next)
-            return 0;
+            return dft;
         if (strieq(next, search))
             return i;
         i++;
@@ -487,6 +488,31 @@ stat_type readStatisticValue(const char * cur, const char * * end, StatisticMeas
     return value;
 }
 
+
+void validateScopeId(const char * idText)
+{
+    StatsScopeId id;
+    if (!id.setScopeText(idText))
+        throw makeStringExceptionV(JLIBERR_UnexpectedValue, "'%s' does not appear to be a valid scope id", idText);
+}
+
+
+void validateScope(const char * scopeText)
+{
+    StatsScopeId id;
+    const char * cur = scopeText;
+    for(;;)
+    {
+        if (!id.setScopeText(cur, &cur))
+            throw makeStringExceptionV(JLIBERR_UnexpectedValue, "'%s' does not appear to be a valid scope id", cur);
+        cur = strchr(cur, ':');
+        if (!cur)
+            return;
+        cur++;
+    }
+}
+
+
 //--------------------------------------------------------------------------------------------------------------------
 
 unsigned queryScopeDepth(const char * text)
@@ -576,28 +602,25 @@ const char * queryMeasureName(StatisticMeasure measure)
     return measureNames[measure];
 }
 
-StatisticMeasure queryMeasure(const char * measure)
+StatisticMeasure queryMeasure(const char * measure, StatisticMeasure dft)
 {
     //MORE: Use a hash table??
-    StatisticMeasure ret = (StatisticMeasure)matchString(measureNames, measure);
+    StatisticMeasure ret = (StatisticMeasure)matchString(measureNames, measure, SMeasureMax);
+    if (ret != SMeasureMax)
+        return ret;
+
     //Legacy support for an unusual statistic - pretend the sizes are in bytes instead of kb.
-    if ((ret == SMeasureNone) && measure)
+    if (streq(measure, "kb"))
+        return SMeasureSize;
+
+    for (unsigned i1=SMeasureAll+1; i1 < SMeasureMax; i1++)
     {
-        if (streq(measure, "kb"))
-        {
-            ret = SMeasureSize;
-        }
-        else
-        {
-            for (unsigned i1=SMeasureAll+1; i1 < SMeasureMax; i1++)
-            {
-                const char * prefix = queryMeasurePrefix((StatisticMeasure)i1);
-                if (strieq(measure, prefix))
-                    return (StatisticMeasure)i1;
-            }
-        }
+        const char * prefix = queryMeasurePrefix((StatisticMeasure)i1);
+        if (strieq(measure, prefix))
+            return (StatisticMeasure)i1;
     }
-    return ret;
+
+    return dft;
 }
 
 StatsMergeAction queryMergeMode(StatisticMeasure measure)
@@ -939,10 +962,10 @@ const char * queryTreeTag(StatisticKind kind)
 
 //--------------------------------------------------------------------------------------------------------------------
 
-StatisticKind queryStatisticKind(const char * search)
+StatisticKind queryStatisticKind(const char * search, StatisticKind dft)
 {
     if (!search)
-        return StKindNone;
+        return dft;
     if (streq(search, "*"))
         return StKindAll;
 
@@ -957,7 +980,7 @@ StatisticKind queryStatisticKind(const char * search)
                 return kind;
         }
     }
-    return StKindNone;
+    return dft;
 }
 
 //--------------------------------------------------------------------------------------------------------------------
@@ -967,10 +990,10 @@ const char * queryCreatorTypeName(StatisticCreatorType sct)
     return creatorTypeNames[sct];
 }
 
-StatisticCreatorType queryCreatorType(const char * sct)
+StatisticCreatorType queryCreatorType(const char * sct, StatisticCreatorType dft)
 {
     //MORE: Use a hash table??
-    return (StatisticCreatorType)matchString(creatorTypeNames, sct);
+    return (StatisticCreatorType)matchString(creatorTypeNames, sct, dft);
 }
 
 //--------------------------------------------------------------------------------------------------------------------
@@ -980,10 +1003,10 @@ const char * queryScopeTypeName(StatisticScopeType sst)
     return scopeTypeNames[sst];
 }
 
-extern jlib_decl StatisticScopeType queryScopeType(const char * sst)
+extern jlib_decl StatisticScopeType queryScopeType(const char * sst, StatisticScopeType dft)
 {
     //MORE: Use a hash table??
-    return (StatisticScopeType)matchString(scopeTypeNames, sst);
+    return (StatisticScopeType)matchString(scopeTypeNames, sst, dft);
 }
 
 //--------------------------------------------------------------------------------------------------------------------
@@ -1342,8 +1365,9 @@ void StatsScopeId::setId(StatisticScopeType _scopeType, unsigned _id, unsigned _
     extra = _extra;
 }
 
-bool StatsScopeId::setScopeText(const char * text, char * * next)
+bool StatsScopeId::setScopeText(const char * text, const char * * _next)
 {
+    char * * next = (char * *)_next;
     switch (*text)
     {
     case ActivityScopePrefix[0]:
@@ -1416,25 +1440,26 @@ bool StatsScopeId::setScopeText(const char * text, char * * next)
     return false;
 }
 
-void StatsScopeId::extractScopeText(const char * text, const char * * next)
+bool StatsScopeId::extractScopeText(const char * text, const char * * next)
 {
-    if (!setScopeText(text, (char * *)next))
+    if (setScopeText(text, next))
+        return true;
+
+    scopeType = SSTunknown;
+    const char * end = strchr(text, ':');
+    if (end)
     {
-        scopeType = SSTunknown;
-        const char * end = strchr(text, ':');
-        if (end)
-        {
-            name.set(text, end-text);
-            if (next)
-                *next = end;
-        }
-        else
-        {
-            name.set(text);
-            if (next)
-                *next = text + strlen(text);
-        }
+        name.set(text, end-text);
+        if (next)
+            *next = end;
     }
+    else
+    {
+        name.set(text);
+        if (next)
+            *next = text + strlen(text);
+    }
+    return false;
 }
 
 
@@ -2720,6 +2745,9 @@ void ScopeFilter::addId(const char * id)
 
 void ScopeFilter::setDepth(unsigned low, unsigned high)
 {
+    if (low > high)
+        throw makeStringExceptionV(0, "Depth parameters in wrong order %u..%u", low, high);
+
     minDepth = low;
     maxDepth = high;
 }
@@ -3013,8 +3041,8 @@ bool StatisticsFilter::recurseChildScopes(StatisticScopeType curScopeType, const
 
 void StatisticsFilter::set(const char * creatorTypeText, const char * scopeTypeText, const char * kindText)
 {
-    StatisticCreatorType creatorType = queryCreatorType(creatorTypeText);
-    StatisticScopeType scopeType = queryScopeType(scopeTypeText);
+    StatisticCreatorType creatorType = queryCreatorType(creatorTypeText, SCTnone);
+    StatisticScopeType scopeType = queryScopeType(scopeTypeText, SSTnone);
 
     if (creatorType != SCTnone)
         setCreatorType(creatorType);
@@ -3025,7 +3053,7 @@ void StatisticsFilter::set(const char * creatorTypeText, const char * scopeTypeT
 
 void StatisticsFilter::set(const char * _creatorTypeText, const char * _creator, const char * _scopeTypeText, const char * _scope, const char * _measureText, const char * _kindText)
 {
-    StatisticMeasure newMeasure = queryMeasure(_measureText);
+    StatisticMeasure newMeasure = queryMeasure(_measureText, SMeasureNone);
     if (newMeasure != SMeasureNone)
         setMeasure(newMeasure);
     set(_creatorTypeText, _scopeTypeText, _kindText);
@@ -3063,7 +3091,7 @@ void StatisticsFilter::addFilter(const char * filter)
     if (hasPrefix(filter, "creator[", false))
         setCreator(value);
     else if (hasPrefix(filter, "creatortype[", false))
-        setCreatorType(queryCreatorType(value));
+        setCreatorType(queryCreatorType(value, SCTall));
     else if (hasPrefix(filter, "depth[", false))
     {
         const char * comma = strchr(value, ',');
@@ -3075,11 +3103,11 @@ void StatisticsFilter::addFilter(const char * filter)
     else if (hasPrefix(filter, "kind[", false))
         setKind(value);
     else if (hasPrefix(filter, "measure[", false))
-        setMeasure(queryMeasure(value));
+        setMeasure(queryMeasure(value, SMeasureAll));
     else if (hasPrefix(filter, "scope[", false))
         setScope(value);
     else if (hasPrefix(filter, "scopetype[", false))
-        setScopeType(queryScopeType(value));
+        setScopeType(queryScopeType(value, SSTall));
     else if (hasPrefix(filter, "value[", false))
     {
         //value[exact|low..high] where low and high are optional
@@ -3206,7 +3234,7 @@ void StatisticsFilter::setKind(const char * _kind)
     }
 
     //Other wildcards not currently supported
-    kind = queryStatisticKind(_kind);
+    kind = queryStatisticKind(_kind, StKindAll);
 }
 
 
@@ -3302,20 +3330,20 @@ void verifyStatisticFunctions()
     {
         const char * prefix __attribute__((unused)) = queryMeasurePrefix((StatisticMeasure)i1);
         const char * name = queryMeasureName((StatisticMeasure)i1);
-        assertex(queryMeasure(name) == i1);
+        assertex(queryMeasure(name, SMeasureMax) == i1);
     }
 
     for (StatisticScopeType sst = SSTnone; sst < SSTmax; sst = (StatisticScopeType)(sst+1))
     {
         const char * name = queryScopeTypeName(sst);
-        assertex(queryScopeType(name) == sst);
+        assertex(queryScopeType(name, SSTmax) == sst);
 
     }
 
     for (StatisticCreatorType sct = SCTnone; sct < SCTmax; sct = (StatisticCreatorType)(sct+1))
     {
         const char * name = queryCreatorTypeName(sct);
-        assertex(queryCreatorType(name) == sct);
+        assertex(queryCreatorType(name, SCTmax) == sct);
     }
 
     for (unsigned i2=StKindAll+1; i2 < StMax; i2++)

+ 9 - 6
system/jlib/jstats.h

@@ -63,8 +63,8 @@ public:
     void deserialize(MemoryBuffer & in, unsigned version);
     void serialize(MemoryBuffer & out) const;
 
-    void extractScopeText(const char * text, const char * * next);
-    bool setScopeText(const char * text, char * * next = nullptr);
+    bool extractScopeText(const char * text, const char * * next);
+    bool setScopeText(const char * text, const char * * next = nullptr);
     void setId(StatisticScopeType _scopeType, unsigned _id, unsigned _extra = 0);
     void setActivityId(unsigned _id);
     void setEdgeId(unsigned _id, unsigned _output);
@@ -685,10 +685,10 @@ extern jlib_decl const char * queryMeasureName(StatisticMeasure measure);
 extern jlib_decl StatsMergeAction queryMergeMode(StatisticMeasure measure);
 extern jlib_decl StatsMergeAction queryMergeMode(StatisticKind kind);
 
-extern jlib_decl StatisticMeasure queryMeasure(const char *  measure);
-extern jlib_decl StatisticKind queryStatisticKind(const char *  kind);
-extern jlib_decl StatisticCreatorType queryCreatorType(const char * sct);
-extern jlib_decl StatisticScopeType queryScopeType(const char * sst);
+extern jlib_decl StatisticMeasure queryMeasure(const char *  measure, StatisticMeasure dft);
+extern jlib_decl StatisticKind queryStatisticKind(const char *  kind, StatisticKind dft);
+extern jlib_decl StatisticCreatorType queryCreatorType(const char * sct, StatisticCreatorType dft);
+extern jlib_decl StatisticScopeType queryScopeType(const char * sst, StatisticScopeType dft);
 
 extern jlib_decl IStatisticGatherer * createStatisticsGatherer(StatisticCreatorType creatorType, const char * creator, const StatsScopeId & rootScope);
 extern jlib_decl void serializeStatisticCollection(MemoryBuffer & out, IStatisticCollection * collection);
@@ -708,6 +708,9 @@ extern jlib_decl void verifyStatisticFunctions();
 extern jlib_decl void formatTimeCollatable(StringBuffer & out, unsigned __int64 value, bool nano);
 extern jlib_decl unsigned __int64 extractTimeCollatable(const char *s, bool nano);
 
+extern jlib_decl void validateScopeId(const char * idText);
+extern jlib_decl void validateScope(const char * scopeText);
+
 //Scopes need to be processed in a consistent order so they can be merged.
 //activities are in numeric order
 //edges must come before activities.

+ 1 - 0
system/jlib/jutil.cpp

@@ -29,6 +29,7 @@
 #include "jprop.hpp"
 #include "jerror.hpp"
 #include "jencrypt.hpp"
+#include "jerror.hpp"
 #ifdef _WIN32
 #include <mmsystem.h> // for timeGetTime 
 #include <float.h> //for _isnan and _fpclass