Explorar el Código

Fix internal error on complex nested field selection

In some circumstances it was possible to get the following error:
Attempting to lookup field <name> in a dataset which has no active element.
This is preventing another option being enabled that would solve some
problems with conditional evaluation of expressions.

The root cause is the handling of no_select.  A no_select should have a
newAtom attribute associated with it if it is selecting a field from a
new row.  So a[1].b.c is represented as
  select(select(a[1],b,new),c)

Some places were incorrectly checking if the current select had an
associated new attribute instead of checking if the root select had
one.

no_select(no_selectnth) now always has a newAttribute associated with it.
This simplifies and optimizes the processing of unnormalized trees.

This also contains a fix for hqliproj projecting a datarow, and better
expansion of (a.b.c.d within transforms)

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday hace 13 años
padre
commit
2fad5fc2b7

+ 73 - 44
ecl/hql/hqlexpr.cpp

@@ -68,7 +68,7 @@
 //#define DEBUG_SCOPE
 //#define CHECK_RECORD_CONSITENCY
 //#define PARANOID
-//#define SEARCH_NAME1   "vQ8"
+//#define SEARCH_NAME1   "vL6R"
 //#define SEARCH_NAME2   "v19"
 //#define SEARCH_IEXPR 0x03289048
 //#define CHECK_SELSEQ_CONSISTENCY
@@ -270,6 +270,8 @@ extern HQL_API void clearCacheCounts()
 #endif
 }
 
+static IHqlExpression * doCreateSelectExpr(HqlExprArray & args);
+
 //==============================================================================================================
 
 //createSourcePath actually uses a kept hash table at the moment, because the source filename is stored in
@@ -3278,29 +3280,17 @@ void CHqlExpression::updateFlagsAfterOperands()
         {
             IHqlExpression * left = queryChild(0);
             node_operator lOp = left->getOperator();
-#if 0
-            //I think the following are correct, but cause problems.., probably because newAtom isn't handled correctly in all places
-            if ((lOp == no_select) && left->isDatarow())
+            infoFlags = (infoFlags & HEFretainedByActiveSelect);
+            infoFlags |= HEFcontainsActiveDataset;
+            switch (lOp)
             {
-                IHqlExpression * right = queryChild(1);
-                //inherit from parent... whether or not this is a new selector or not.
-                right = right;
-            }
-            else
-#endif
-            {
-                infoFlags = (infoFlags & HEFretainedByActiveSelect);
-                infoFlags |= HEFcontainsActiveDataset;
-                switch (lOp)
-                {
-                case no_self:
-                case no_left:
-                case no_right:
-                    break;
-                default:
-                    infoFlags |= HEFcontainsActiveNonSelector;
-                    break;
-                }
+            case no_self:
+            case no_left:
+            case no_right:
+                break;
+            default:
+                infoFlags |= HEFcontainsActiveNonSelector;
+                break;
             }
         }
         else
@@ -3558,6 +3548,22 @@ switch (op)
         }
     }
 
+#ifdef _DEBUG
+    if (op == no_select)
+    {
+        IHqlExpression * ds = queryChild(0);
+        if ((ds->getOperator() == no_select) && !ds->isDataset())
+        {
+            if (hasProperty(newAtom) && isNewSelector(ds))
+            {
+                IHqlExpression * root = queryDatasetCursor(ds);
+                if (root->isDataset())
+                    throwUnexpected();
+            }
+        }
+    }
+#endif
+
 #if 0
     //Useful code for detecting when types get messed up for comparisons
     if (op == no_eq || op == no_ne)
@@ -4744,7 +4750,7 @@ void CHqlExpression::cacheTablesUsed()
             case no_select:
                 {
                     IHqlExpression * ds = queryChild(0);
-                    if (hasProperty(newAtom) || ((ds->getOperator() == no_select) && ds->isDatarow()) || (ds->getOperator() == no_selectnth))
+                    if (hasProperty(newAtom) || ((ds->getOperator() == no_select) && ds->isDatarow()))
                         ds->gatherTablesUsed(&newScopeTables, &inScopeTables);
                     else
                         addActiveTable(inScopeTables, ds);
@@ -4758,11 +4764,9 @@ void CHqlExpression::cacheTablesUsed()
                 }
             case no_rows:
             case no_rowset:
+                //MORE: This is a bit strange!
                 addActiveTable(inScopeTables, queryChild(0));
                 break;
-                //first child is no_rows, use the grand child
-                addActiveTable(inScopeTables, queryChild(0)->queryChild(0));
-                break;
             case no_left:
             case no_right:
                 addActiveTable(inScopeTables, this);
@@ -4928,7 +4932,7 @@ IHqlExpression * CHqlExpression::calcNormalizedSelector() const
         appendArray(args, operands);
         args.replace(*LINK(normalizedLeft), 0);
         removeProperty(args, newAtom);
-        return createSelectExpr(args);
+        return doCreateSelectExpr(args);
     }
     return NULL;
 }
@@ -4941,8 +4945,6 @@ CHqlSelectExpression::CHqlSelectExpression(IHqlExpression * left, IHqlExpression
 #ifdef _DEBUG
     assertex(!isDataset());
     assertex(left->getOperator() != no_activerow);
-//  if ((left->getOperator() == no_select) && left->isDatarow())
-//      assertex(left->hasProperty(newAtom) == hasProperty(newAtom));
 #endif
     normalized.setown(calcNormalizedSelector());
 }
@@ -4954,8 +4956,6 @@ CHqlSelectExpression::CHqlSelectExpression(HqlExprArray & _ownedOperands)
 #ifdef _DEBUG
     assertex(!isDataset());
     assertex(left->getOperator() != no_activerow);
-//  if ((left->getOperator() == no_select) && left->isDatarow())
-//      assertex(left->hasProperty(newAtom) == hasProperty(newAtom));
 #endif
     normalized.setown(calcNormalizedSelector());
 }
@@ -11629,6 +11629,14 @@ inline IHqlExpression * normalizeSelectLhs(IHqlExpression * lhs, bool & isNew)
             if (isNew && isAlwaysActiveRow(lhs))
                 isNew = false;
             return lhs;
+        case no_selectnth:
+        case no_createrow:
+            assertex(isAlwaysNewRow(lhs));
+            //Ensure selects of these expressions always have new set - it saves problems when an
+            //unnormalized tree is walked or transformed.
+            //IF/PROJECTROW are not includes since they could be folded to an active selector
+            isNew = true;
+            return lhs;
         default:
             return lhs;
         }
@@ -11662,6 +11670,21 @@ extern IHqlExpression * createSelectExpr(IHqlExpression * _lhs, IHqlExpression *
     return ret->closeExpr();
 }
 
+static IHqlExpression * doCreateSelectExpr(HqlExprArray & args)
+{
+    IHqlExpression * rhs = &args.item(1);
+    checkRhsSelect(rhs);
+
+    type_t t = rhs->queryType()->getTypeCode();
+    if (t == type_table || t == type_groupedtable)
+        return createDataset(no_select, args);
+    if (t == type_row)
+        return createRow(no_select, args);
+
+    IHqlExpression * ret = new CHqlSelectExpression(args);
+    return ret->closeExpr();
+}
+
 extern IHqlExpression * createSelectExpr(HqlExprArray & args)
 {
     IHqlExpression * lhs = &args.item(0);
@@ -11682,17 +11705,7 @@ extern IHqlExpression * createSelectExpr(HqlExprArray & args)
     if (isNew)
         args.append(*LINK(newSelectAttrExpr));
 
-    IHqlExpression * rhs = &args.item(1);
-    checkRhsSelect(rhs);
-
-    type_t t = rhs->queryType()->getTypeCode();
-    if (t == type_table || t == type_groupedtable)
-        return createDataset(no_select, args);
-    if (t == type_row)
-        return createRow(no_select, args);
-
-    IHqlExpression * ret = new CHqlSelectExpression(args);
-    return ret->closeExpr();
+    return doCreateSelectExpr(args);
 }
 
 IHqlExpression * ensureDataset(IHqlExpression * expr)
@@ -11708,6 +11721,17 @@ IHqlExpression * ensureDataset(IHqlExpression * expr)
 }
 
 
+extern bool isAlwaysNewRow(IHqlExpression * expr)
+{
+    switch (expr->getOperator())
+    {
+    case no_createrow:
+    case no_selectnth:
+        return true;
+    }
+    return false;
+}
+
 extern bool isAlwaysActiveRow(IHqlExpression * expr)
 {
     switch (expr->getOperator())
@@ -12714,7 +12738,7 @@ static bool exprContainsCounter(RecursionChecker & checker, IHqlExpression * exp
     case no_select:
         {
             IHqlExpression * ds = expr->queryChild(0);
-            if (expr->hasProperty(newAtom) || ((ds->getOperator() == no_select) && ds->isDatarow()) || (ds->getOperator() == no_selectnth))
+            if (expr->hasProperty(newAtom) || ((ds->getOperator() == no_select) && ds->isDatarow()))
                 return exprContainsCounter(checker, ds, counter);
             return false;
         }
@@ -14481,6 +14505,11 @@ IHqlExpression * createDummySelectorSequence()
     return LINK(defaultSelectorSequenceExpr);
 }
 
+IHqlExpression * queryNewSelectAttrExpr()
+{
+    return newSelectAttrExpr;
+}
+
 //Follow inheritance structure when getting property value.
 IHqlExpression * queryRecordProperty(IHqlExpression * record, _ATOM name)
 {

+ 2 - 0
ecl/hql/hqlexpr.hpp

@@ -1491,6 +1491,7 @@ extern HQL_API IHqlExpression * createUniqueSelectorSequence();
 extern HQL_API IHqlExpression * createDummySelectorSequence();
 extern HQL_API IHqlExpression * expandBetween(IHqlExpression * expr);
 extern HQL_API bool isAlwaysActiveRow(IHqlExpression * expr);
+extern HQL_API bool isAlwaysNewRow(IHqlExpression * expr);
 extern HQL_API IHqlExpression * ensureActiveRow(IHqlExpression * expr);
 extern HQL_API bool isIndependentOfScope(IHqlExpression * expr);
 extern HQL_API bool isActivityIndependentOfScope(IHqlExpression * expr);
@@ -1624,6 +1625,7 @@ inline IHqlExpression * querySelSeq(IHqlExpression * expr)
 extern HQL_API IHqlExpression * createGroupedAttribute(ITypeInfo * type);
 extern HQL_API bool isSameUnqualifiedType(ITypeInfo * l, ITypeInfo * r);
 extern HQL_API bool isSameFullyUnqualifiedType(ITypeInfo * l, ITypeInfo * r);
+extern HQL_API IHqlExpression * queryNewSelectAttrExpr();
 
 //The following are wrappers for the code generator specific getInfoFlags()
 //inline bool isTableInvariant(IHqlExpression * expr)       { return (expr->getInfoFlags() & HEFtableInvariant) != 0; }

+ 2 - 2
ecl/hql/hqlfold.cpp

@@ -5243,7 +5243,7 @@ static void gatherConstantFilterMappings(HqlConstantPercolator & mappings, IHqlE
         {
             IHqlExpression * lhs = expr->queryChild(0);
             //MORE: Should also handle subselects now that the constant percolator does
-            if ((lhs->getOperator() != no_select) || lhs->hasProperty(newAtom) || lhs->queryChild(0) != selector)
+            if ((lhs->getOperator() != no_select) || isNewSelector(lhs) || lhs->queryChild(0) != selector)
                 break;
 
             IHqlExpression * rhs = expr->queryChild(1);
@@ -5782,7 +5782,7 @@ IHqlExpression * foldHqlExpression(IHqlExpression * expr, ITemplateContext *temp
     case no_attr:
         return LINK(expr);
     case no_select:
-        if (!expr->hasProperty(newAtom))
+        if (!isNewSelector(expr))
             return LINK(expr);
         break;
     }

+ 1 - 1
ecl/hql/hqlgram.hpp

@@ -500,7 +500,7 @@ public:
     ITypeInfo *promoteSetToSameType(HqlExprArray & exprs, attribute &errpos);
     ITypeInfo * queryElementType(const attribute & errpos, IHqlExpression * list);
     IHqlExpression *createINExpression(node_operator op, IHqlExpression *expr, IHqlExpression *set, attribute &errpos);
-    IHqlExpression * createLoopCondition(IHqlExpression * left, IHqlExpression * arg1, IHqlExpression * arg2, IHqlExpression * seq);
+    IHqlExpression * createLoopCondition(IHqlExpression * left, IHqlExpression * arg1, IHqlExpression * arg2, IHqlExpression * seq, IHqlExpression * rowsid);
     void setTemplateAttribute();
     void warnIfFoldsToConstant(IHqlExpression * expr, const attribute & errpos);
     void warnIfRecordPacked(IHqlExpression * expr, const attribute & errpos);

+ 2 - 2
ecl/hql/hqlgram.y

@@ -7405,7 +7405,7 @@ simpleDataSet
                             IHqlExpression * counter = $9.getExpr();
                             if (counter)
                                 body = createComma(body, createAttribute(_countProject_Atom, counter));
-                            IHqlExpression * loopCondition = parser->createLoopCondition(left, $6.getExpr(), NULL, $14.queryExpr());
+                            IHqlExpression * loopCondition = parser->createLoopCondition(left, $6.getExpr(), NULL, $14.queryExpr(), $12.queryExpr());
                             IHqlExpression * loopExpr = createDataset(no_loop, left, createComma(loopCondition, body, $10.getExpr(), createComma($12.getExpr(), $14.getExpr())));
                             parser->checkLoopFlags($1, loopExpr);
                             $$.setExpr(loopExpr);
@@ -7422,7 +7422,7 @@ simpleDataSet
                             IHqlExpression * counter = $11.getExpr();
                             if (counter)
                                 body = createComma(body, createAttribute(_countProject_Atom, counter));
-                            IHqlExpression * loopCondition = parser->createLoopCondition(left, $6.getExpr(), $8.getExpr(), $16.queryExpr());
+                            IHqlExpression * loopCondition = parser->createLoopCondition(left, $6.getExpr(), $8.getExpr(), $16.queryExpr(), $14.queryExpr());
                             IHqlExpression * loopExpr = createDataset(no_loop, left, createComma(loopCondition, body, $12.getExpr(), createComma($14.getExpr(), $16.getExpr())));
                             parser->checkLoopFlags($1, loopExpr);
                             $$.setExpr(loopExpr);

+ 5 - 8
ecl/hql/hqlgram2.cpp

@@ -5536,7 +5536,7 @@ IHqlExpression * HqlGram::createDistributeCond(IHqlExpression * leftDs, IHqlExpr
     return cond;
 }
 
-IHqlExpression * HqlGram::createLoopCondition(IHqlExpression * leftDs, IHqlExpression * arg1, IHqlExpression * arg2, IHqlExpression * seq)
+IHqlExpression * HqlGram::createLoopCondition(IHqlExpression * leftDs, IHqlExpression * arg1, IHqlExpression * arg2, IHqlExpression * seq, IHqlExpression * rowsid)
 {
     IHqlExpression * count = NULL;
     IHqlExpression * filter = NULL;
@@ -5553,14 +5553,11 @@ IHqlExpression * HqlGram::createLoopCondition(IHqlExpression * leftDs, IHqlExpre
         {
             //if refers to fields in LEFT then must be a row filter
             OwnedHqlExpr left = createSelector(no_left, leftDs, seq);
-            if (containsSelector(arg1, left))
-            {
-                filter = arg1;
-            }
-            else
-            {
+            OwnedHqlExpr rows = createDataset(no_rows, LINK(left), LINK(rowsid));
+            if (containsExpression(arg1, rows) || !containsSelector(arg1, left))
                 loopCond = arg1;
-            }
+            else
+                filter = arg1;
         }
     }
     else

+ 10 - 9
ecl/hql/hqlopt.cpp

@@ -147,13 +147,10 @@ static bool isComplexExpansion(IHqlExpression * expr)
     {
     case no_select:
         {
-            while (expr->getOperator() == no_select)
-            {
-                if (!expr->hasProperty(newAtom))
-                    return false;
-                expr = expr->queryChild(0);
-            }
-            return true;
+            bool isNew;
+            IHqlExpression * ds = querySelectorDataset(expr, isNew);
+            //A select from a create row is likely to be optimized
+            return isNew && (ds->getOperator() != no_createrow);
         }
     case NO_AGGREGATE:
     case no_call:
@@ -410,9 +407,12 @@ IHqlExpression * CTreeOptimizer::swapIntoAddFiles(IHqlExpression * expr, bool fo
 IHqlExpression * CTreeOptimizer::moveFilterOverSelect(IHqlExpression * expr)
 {
     IHqlExpression * select = expr->queryChild(0);
-    if (!select->hasProperty(newAtom))
+    if (!isNewSelector(select))
         return NULL;
     IHqlExpression * ds = select->queryChild(0);
+    //MORE: If ds is a row then the filter needs to be moved to the root dataset
+    if (!ds->isDataset())
+        return NULL;
     IHqlExpression * newScope = select->queryNormalizedSelector();
     HqlExprArray args, hoisted, notHoisted;
     HqlExprCopyArray inScope;
@@ -1776,7 +1776,7 @@ bool CTreeOptimizer::childrenAreShared(IHqlExpression * expr)
     switch (expr->getOperator())
     {
     case no_select:
-        if (!expr->hasProperty(newAtom))
+        if (!isNewSelector(expr))
             return false;
         return isShared(expr->queryChild(0));
     case NO_AGGREGATE:
@@ -2388,6 +2388,7 @@ IHqlExpression * CTreeOptimizer::doCreateTransformed(IHqlExpression * transforme
                                 cur = cur->queryChild(0);
                             switch (cur->getOperator())
                             {
+                            case no_createrow:
                             case no_constant:
                             case no_select:
                             case no_null:

+ 17 - 76
ecl/hql/hqltrans.cpp

@@ -1208,7 +1208,7 @@ void NewHqlTransformer::analyseExpr(IHqlExpression * expr)
 #ifdef _DEBUG
             IHqlExpression * field = expr->queryChild(1);
 #endif
-            if (expr->hasProperty(newAtom))
+            if (isNewSelector(expr))
                 analyseExpr(ds);
             else
                 analyseSelector(ds);
@@ -1266,19 +1266,8 @@ void NewHqlTransformer::analyseAssign(IHqlExpression * expr)
 
 void NewHqlTransformer::analyseSelector(IHqlExpression * expr)
 {
-    node_operator op = expr->getOperator();
-    switch (op)
-    {
-    case no_selectnth:
-        analyseExpr(expr);
-        break;
-    case no_select:
-        if (expr->hasProperty(newAtom))
-            analyseExpr(expr);
-        else
-            analyseSelector(expr->queryChild(0));
-        break;
-    }
+    if ((expr->getOperator() == no_select) && !expr->hasProperty(newAtom))
+        analyseSelector(expr->queryChild(0));
 }
 
 
@@ -1395,15 +1384,24 @@ IHqlExpression * NewHqlTransformer::createTransformed(IHqlExpression * expr)
         }
         break;
     case no_select:
-        if (expr->hasProperty(newAtom))
+        if (isNewSelector(expr))
         {
             same = transformChildren(expr, children);
             IHqlExpression & ds = children.item(0);
-            if (ds.getOperator() == no_activetable)
+            node_operator dsOp = ds.getOperator();
+            if (dsOp == no_activetable)
             {
                 children.replace(*LINK(ds.queryChild(0)), 0);
                 removeProperty(children, newAtom);
             }
+            else if (!expr->hasProperty(newAtom))
+            {
+                //unusual situation x.a<new>.b; x.a<new> is converted to d, but d.b is not right, it should now be d.b<new>
+                assertex(ds.isDatarow());
+                if ((dsOp != no_select) || !isNewSelector(&ds))
+                    children.append(*LINK(queryNewSelectAttrExpr()));
+            }
+
             if ((children.ordinality() > 2) && isAlwaysActiveRow(&ds))
                 removeProperty(children, newAtom);
         }
@@ -1521,14 +1519,7 @@ IHqlExpression * NewHqlTransformer::createTransformedSelector(IHqlExpression * e
     node_operator op = expr->getOperator();
     switch (op)
     {
-//  case no_left:
-//  case no_right:
-//  case no_self:
-    case no_selectnth:
-        return transform(expr);
     case no_select:
-        if (expr->hasProperty(newAtom))
-            return transform(expr);
         return createTransformedActiveSelect(expr);
     default:
         //NB: no_if etc. should have gone down the transform branch, and then met a no_activerow if in scope.
@@ -1552,15 +1543,7 @@ IHqlExpression * NewHqlTransformer::createTransformedActiveSelect(IHqlExpression
     if (newLeft->getOperator() == no_newrow)
         return createNewSelectExpr(LINK(newLeft->queryChild(0)), LINK(newRight));
 
-    if ((left->getOperator() == no_select) && left->isDatarow() && newLeft->isDatarow())
-    {
-        //weird situation x.a<new>.b   x.a<new> is converted to d, but d.b is not right, it should now be d.b<new>
-        if ((newLeft->getOperator() != no_select) || (isNewSelector(left) && !isNewSelector(newLeft)))
-            return createNewSelectExpr(LINK(newLeft), LINK(newRight));
-    }
-
     //NOTE: In very obscure situations: ds[1].x.ds<new>.y -> z.ds<new>.y (newLeft != normalizeSelector)
-
     //NB: newLeft.get() == newLeft->queryNormalizedSelector() - asserted above
     return createSelectExpr(LINK(newLeft->queryNormalizedSelector()), LINK(newRight));
 }
@@ -2777,7 +2760,7 @@ IHqlExpression * MergingHqlTransformer::createTransformed(IHqlExpression * expr)
     case no_activerow:
         return NewHqlTransformer::createTransformed(expr);
     case no_select:
-        if (!expr->hasProperty(newAtom))
+        if (!isNewSelector(expr))
             return createTransformedActiveSelect(expr);
         return NewHqlTransformer::createTransformed(expr);
     }
@@ -4451,56 +4434,14 @@ IHqlExpression * expandCreateRowSelectors(IHqlExpression * expr)
     return expander.createTransformed(expr);
 }
 
-//---------------------------------------------------------------------------
-
-static HqlTransformerInfo hqlSelectorLocatorInfo("HqlSelectorLocator");
-HqlSelectorLocator::HqlSelectorLocator(IHqlExpression * _selector) : NewHqlTransformer(hqlSelectorLocatorInfo)
-{ 
-    selector.set(_selector); 
-    foundSelector = false; 
-}
-
-
-void HqlSelectorLocator::analyseExpr(IHqlExpression * expr)
-{
-    if (foundSelector || alreadyVisited(expr))
-        return;
-    unsigned numNonHidden = activityHidesSelectorGetNumNonHidden(expr, selector);
-    if (numNonHidden != 0)
-    {
-        for (unsigned i=0; i < numNonHidden; i++)
-            analyseExpr(expr->queryChild(i));
-    }
-    else
-        NewHqlTransformer::analyseExpr(expr);
-}
-
-void HqlSelectorLocator::analyseSelector(IHqlExpression * expr)
-{
-    if (expr == selector)
-    {
-        foundSelector = true;
-        return;
-    }
-    NewHqlTransformer::analyseSelector(expr);
-}
-
-bool HqlSelectorLocator::containsSelector(IHqlExpression * expr)
-{
-    foundSelector = false;
-    analyse(expr, 0);
-    return foundSelector;
-}
-
 HQL_API bool containsSelector(IHqlExpression * expr, IHqlExpression * selector)
 {
-    HqlSelectorLocator locator(selector);
-    return locator.containsSelector(expr);
+    return exprReferencesDataset(expr, selector);
 }
 
 //---------------------------------------------------------------------------
 
-static HqlTransformerInfo hqlSelectorAnywhereLocatorInfo("HqlSelectorLocator");
+static HqlTransformerInfo hqlSelectorAnywhereLocatorInfo("HqlSelectorAnywhereLocator");
 HqlSelectorAnywhereLocator::HqlSelectorAnywhereLocator(IHqlExpression * _selector) : NewHqlTransformer(hqlSelectorAnywhereLocatorInfo)
 { 
     selector.set(_selector); 

+ 0 - 15
ecl/hql/hqltrans.ipp

@@ -1159,21 +1159,6 @@ inline bool activityHidesSelector(IHqlExpression * expr, IHqlExpression * select
 }
 
 
-class HQL_API HqlSelectorLocator : public NewHqlTransformer
-{
-public:
-    HqlSelectorLocator(IHqlExpression * _selector);
-
-    virtual void analyseExpr(IHqlExpression * expr);
-    virtual void analyseSelector(IHqlExpression * expr);
-
-    bool containsSelector(IHqlExpression * expr);
-
-protected:
-    bool foundSelector;
-    OwnedHqlExpr selector;
-};
-
 class HQL_API HqlSelectorAnywhereLocator : public NewHqlTransformer
 {
 public:

+ 5 - 6
ecl/hql/hqlutil.cpp

@@ -2295,7 +2295,7 @@ void DependencyGatherer::doGatherDependencies(IHqlExpression * expr)
     case no_attr_link:
         return;
     case no_select:
-        if (!expr->hasProperty(newAtom))
+        if (!isNewSelector(expr))
             return;
         max = 1;        // by now there should be no default values as children of fields.
         break;
@@ -2726,8 +2726,11 @@ IHqlExpression * queryNextMultiLevelDataset(IHqlExpression * expr, bool followAc
     }
 
     IHqlExpression * root = queryRoot(expr);
-    if (!root || (root->getOperator() != no_select) || (!followActiveSelectors && !root->hasProperty(newAtom)))
+    if (!root || (root->getOperator() != no_select))
         return NULL;
+    if (!followActiveSelectors && !isNewSelector(root))
+        return NULL;
+
     IHqlExpression * ds = root->queryChild(0);
     loop
     {
@@ -4269,10 +4272,6 @@ IHqlExpression * SplitDatasetAttributeTransformer::createTransformed(IHqlExpress
     case no_globalscope:
     case no_nothor:
         return LINK(expr);
-//  case NO_AGGREGATE:
-//  case no_createset:
-        //See comment in analyse above
-//      return LINK(expr);
     }
 
     return NewHqlTransformer::createTransformed(expr);

+ 10 - 6
ecl/hqlcpp/hqlcpp.cpp

@@ -7819,11 +7819,7 @@ IHqlExpression * HqlCppTranslator::querySimpleOrderSelector(IHqlExpression * exp
     if (expr->getOperator() != no_select)
         return NULL;
 
-    bool isNew;
-    IHqlExpression * ds = querySelectorDataset(expr, isNew);
-    if (isNew)
-        return NULL;
-    return ds;
+    return queryDatasetCursor(expr);
 }
 
 static unsigned getMemcmpSize(IHqlExpression * left, IHqlExpression * right, bool isEqualityCompare)
@@ -11671,7 +11667,15 @@ bool HqlCppTranslator::requiresTemp(BuildCtx & ctx, IHqlExpression * expr, bool
             return true;
         }
     case no_select:
-        return expr->hasProperty(newAtom);
+        if (expr->hasProperty(newAtom))
+        {
+            IHqlExpression * ds= expr->queryChild(0);
+            if (!ds->isPure() || !ds->isDatarow())
+                return true;
+            if (!ctx.queryAssociation(ds, AssocRow, NULL))
+                return true;
+        }
+        return false;
     case no_field:
         throwUnexpected();
         return false;       // more, depends on whether conditional etc.

+ 2 - 2
ecl/hqlcpp/hqlcppds.cpp

@@ -479,7 +479,7 @@ IReferenceSelector * HqlCppTranslator::buildNewRow(BuildCtx & ctx, IHqlExpressio
             IHqlExpression * field = expr->queryChild(1);
 #endif
             Owned<IReferenceSelector> selector;
-            if (expr->hasProperty(newAtom))
+            if (isNewSelector(expr))
                 selector.setown(buildNewRow(ctx, expr->queryChild(0)));
             else
                 selector.setown(buildActiveRow(ctx, expr->queryChild(0)));
@@ -570,7 +570,7 @@ IReferenceSelector * HqlCppTranslator::buildActiveRow(BuildCtx & ctx, IHqlExpres
 #ifdef _DEBUG
             IHqlExpression * field = expr->queryChild(1);
 #endif
-            Owned<IReferenceSelector> selector = buildNewOrActiveRow(ctx, expr->queryChild(0), expr->hasProperty(newAtom));
+            Owned<IReferenceSelector> selector = buildNewOrActiveRow(ctx, expr->queryChild(0), isNewSelector(expr));
             return selector->select(ctx, expr);
         }
     case no_id2blob:

+ 9 - 5
ecl/hqlcpp/hqlcse.cpp

@@ -504,7 +504,8 @@ bool CseSpotter::checkPotentialCSE(IHqlExpression * expr, CseSpotterInfo * extra
     case no_field:
         throwUnexpected();
     case no_select:
-        return false; //expr->hasProperty(newAtom);
+        //MORE: ds[n].x would probably be worth cseing.
+        return false;
     case no_list:
     case no_datasetlist:
     case no_getresult:      // these are commoned up in the code generator, so don't do it twice.
@@ -1105,7 +1106,7 @@ IHqlExpression * spotScalarCSE(IHqlExpression * expr, IHqlExpression * limit)
     switch (expr->getOperator())
     {
     case no_select:
-        if (!expr->hasProperty(newAtom))
+        if (!isNewSelector(expr))
             return LINK(expr);
         break;
     }
@@ -1267,9 +1268,12 @@ bool TableInvariantTransformer::isInvariant(IHqlExpression * expr)
         break;
     case no_select:
         {
-            IHqlExpression * ds = expr->queryChild(0);
-            if ((expr->hasProperty(newAtom) || ds->isDatarow()) && !expr->isDataset())
-                invariant = isInvariant(ds);
+            if (!expr->isDataset())
+            {
+                IHqlExpression * ds = expr->queryChild(0);
+                if (expr->hasProperty(newAtom) || ds->isDatarow())
+                    invariant = isInvariant(ds);
+            }
             break;
         }
     case no_newaggregate:

+ 2 - 3
ecl/hqlcpp/hqlgraph.cpp

@@ -312,7 +312,7 @@ void LogicalGraphCreator::createGraphActivity(IHqlExpression * expr)
         dependencyKind = NULL;
         break;
     case no_select:
-        if (!expr->hasProperty(newAtom))
+        if (!isNewSelector(expr))
         {
             last = first;
         }
@@ -490,8 +490,7 @@ bool LogicalGraphCreator::gatherGraphActivities(IHqlExpression * expr, HqlExprAr
     {
     case no_select:
         {
-            IHqlExpression * attr = expr->queryProperty(newAtom);
-            if (attr)
+            if (isNewSelector(expr))
             {
                 if (exprIsGlobal(expr))
                     createRootGraphActivity(expr);

+ 10 - 7
ecl/hqlcpp/hqlhtcpp.cpp

@@ -5763,7 +5763,7 @@ IReferenceSelector * HqlCppTranslator::buildReference(BuildCtx & ctx, IHqlExpres
             IHqlExpression * ds = expr->queryChild(0);
             IHqlExpression * field = expr->queryChild(1);
             Owned<IReferenceSelector> selector;
-            if (expr->hasProperty(newAtom))
+            if (isNewSelector(expr))
             {
                 //could optimize selection from a project of an in-scope dataset.
                 if (((ds->getOperator() == no_newusertable) || (ds->getOperator() == no_hqlproject)) &&
@@ -13839,22 +13839,25 @@ ABoundActivity * HqlCppTranslator::doBuildActivitySelectNew(BuildCtx & ctx, IHql
     if (options.useLinkedNormalize)
         return doBuildActivityNormalizeLinkedChild(ctx, expr);
 
-    IHqlExpression * ds = expr->queryChild(0);
+    bool isNew;
+    IHqlExpression * ds = querySelectorDataset(expr, isNew);
 //  if (!isContextDependent(ds) && !containsActiveDataset(ds))
 //      return doBuildActivityChildDataset(ctx, expr);
     if (canEvaluateInline(&ctx, ds))
         return doBuildActivityChildDataset(ctx, expr);
 
     //Convert the ds.x to normalize(ds, left.x, transform(right));
-    IHqlExpression * field = expr->queryChild(1); 
-    OwnedHqlExpr selSeq = createDummySelectorSequence();
+    OwnedHqlExpr selSeq = createSelectorSequence();
     OwnedHqlExpr left = createSelector(no_left, ds, selSeq);
-    OwnedHqlExpr selector = createSelectExpr(LINK(left), LINK(field));
+    OwnedHqlExpr selector = replaceExpression(expr, ds, left);//createSelectExpr(LINK(left), LINK(field));
     OwnedHqlExpr transformedExpr;
-    if (field->isDatarow())
+    if (expr->isDatarow())
     {
         OwnedHqlExpr transform = createTransformFromRow(selector);
-        transformedExpr.setown(createDatasetF(no_hqlproject, LINK(ds), LINK(transform), LINK(selSeq), NULL));       // MORE: UID
+        if (ds->isDatarow())
+            transformedExpr.setown(createRowF(no_projectrow, LINK(ds), LINK(transform), LINK(selSeq), NULL));
+        else
+            transformedExpr.setown(createDatasetF(no_hqlproject, LINK(ds), LINK(transform), LINK(selSeq), NULL));
     }
     else
     {

+ 1 - 1
ecl/hqlcpp/hqlinline.cpp

@@ -112,7 +112,7 @@ static unsigned calcInlineFlags(BuildCtx * ctx, IHqlExpression * expr)
     {
     case no_select:
         //MORE: What about child datasets in nested records?
-        if (expr->hasProperty(newAtom))
+        if (isNewSelector(expr))
         {
             unsigned childFlags = getInlineFlags(ctx, expr->queryChild(0));
             if ((childFlags & HEFevaluateinline) && isMultiLevelDatasetSelector(expr, false))

+ 3 - 1
ecl/hqlcpp/hqliproj.cpp

@@ -486,7 +486,9 @@ IHqlExpression * ComplexImplicitProjectInfo::createOutputProject(IHqlExpression
         assigns.append(*createAssign(createSelectExpr(LINK(self), LINK(cur)), createSelectExpr(LINK(left), LINK(cur))));
     }
     IHqlExpression * transform = createValue(no_transform, makeTransformType(newOutputRecord->getType()), assigns);
-    return createDataset(no_hqlproject, LINK(ds), createComma(transform, LINK(seq)));
+    if (ds->isDataset())
+        return createDataset(no_hqlproject, LINK(ds), createComma(transform, LINK(seq)));
+    return createRow(no_projectrow, LINK(ds), createComma(transform, LINK(seq)));
 }
 
 

+ 19 - 4
ecl/hqlcpp/hqlresource.cpp

@@ -1561,7 +1561,7 @@ bool ResourcerInfo::expandRatherThanSpill(bool noteOtherSpills)
                 if (options->targetClusterType == RoxieCluster)
                     return false;
 
-                if (!expr->hasProperty(newAtom))
+                if (!isNewSelector(expr))
                     return true;
                 expr = expr->queryChild(0);
                 break;
@@ -1648,7 +1648,7 @@ bool ResourcerInfo::expandRatherThanSplit()
         case no_select:
             if (options->targetClusterType == RoxieCluster)
                 return false;
-            if (!expr->hasProperty(newAtom))
+            if (!isNewSelector(expr))
             {
                 if (!options->useLinkedRawIterator || !hasLinkCountedModifier(expr))
                     return false;
@@ -2137,6 +2137,7 @@ protected:
         switch (op)
         {
         case no_select:
+            //Only interested in the leftmost no_select
             if (expr->hasProperty(newAtom))
             {
                 IHqlExpression * ds = expr->queryChild(0);
@@ -2547,7 +2548,7 @@ bool EclResourcer::findSplitPoints(IHqlExpression * expr)
         {
         case no_select:
             //either a select from a setresult or use of a child-dataset
-            if (expr->hasProperty(newAtom))
+            if (isNewSelector(expr))
             {
                 info->containsActivity = findSplitPoints(expr->queryChild(0));
                 assertex(queryResourceInfo(expr->queryChild(0))->isActivity);
@@ -3373,7 +3374,7 @@ bool EclResourcer::addExprDependency(IHqlExpression * expr, ResourceGraphInfo *
             return !filename->isConstant();
         }
     case no_select:
-        return expr->hasProperty(newAtom);
+        return isNewSelector(expr);
     case no_workunit_dataset:
         {
             IHqlExpression * sequence = queryPropertyChild(expr, sequenceAtom, 0);
@@ -4177,6 +4178,20 @@ IHqlExpression * EclResourcer::doCreateResourced(IHqlExpression * expr, Resource
             same = false;
             break;
         }
+    case no_select:
+        {
+            IHqlExpression * ds = expr->queryChild(0);
+            OwnedHqlExpr newDs = createResourced(ds, ownerGraph, expandInParent, false);
+            if (ds != newDs)
+            {
+                args.append(*LINK(newDs));
+                unwindChildren(args, expr, 1);
+                if (!expr->hasProperty(newAtom) && isNewSelector(expr) && (newDs->getOperator() != no_select))
+                    args.append(*LINK(queryNewSelectAttrExpr()));
+                same = false;
+            }
+            break;
+        }
     case no_join:
     case no_denormalize:
     case no_denormalizegroup:

+ 10 - 7
ecl/hqlcpp/hqlsource.cpp

@@ -926,16 +926,19 @@ void SourceBuilder::analyse(IHqlExpression * expr)
             break;
         }
     case no_select:
-        if (expr->hasProperty(newAtom) && isMultiLevelDatasetSelector(expr, false))
         {
-            IHqlExpression * ds = expr->queryChild(0);
-            if (!translator.resolveSelectorDataset(instance->startctx, ds))
+            bool isNew;
+            IHqlExpression * ds = querySelectorDataset(expr, isNew);
+            if (isNew && isMultiLevelDatasetSelector(expr, false))
             {
-                analyse(ds);
-                isNormalize = true;
+                if (!translator.resolveSelectorDataset(instance->startctx, ds))
+                {
+                    analyse(ds);
+                    isNormalize = true;
+                }
             }
+            break;
         }
-        break;
     case no_stepped:
         if ((translator.getTargetClusterType() == ThorCluster) && translator.queryOptions().checkThorRestrictions)
             throwError(HQLERR_ThorNotSupportStepping);
@@ -1573,7 +1576,7 @@ void SourceBuilder::buildTransformElements(BuildCtx & ctx, IHqlExpression * expr
 void SourceBuilder::doBuildAggregateSelectIterator(BuildCtx & ctx, IHqlExpression * expr)
 {
     IHqlExpression * ds = expr->queryChild(0);
-    if (expr->hasProperty(newAtom))
+    if (isNewSelector(expr))
         buildTransformElements(ctx, ds, false);
     Owned<IHqlCppDatasetCursor> cursor = translator.createDatasetSelector(ctx, expr->queryNormalizedSelector());
     cursor->buildIterateLoop(ctx, false);

+ 1 - 4
ecl/hqlcpp/hqlttcpp.cpp

@@ -2812,7 +2812,7 @@ IHqlExpression * ThorHqlTransformer::normalizeSelect(IHqlExpression * expr)
     a.b.xyz would need to be converted to in.parent.xyz.  It will generate very inefficient code, so not going
     to go this way at the moment.
     */
-    if (!expr->hasProperty(newAtom) || !expr->isDataset())
+    if (!isNewSelector(expr) || !expr->isDataset())
         return NULL;
 
     IHqlExpression * ds = expr->queryChild(0);
@@ -8097,7 +8097,6 @@ static IHqlExpression * splitSelector(IHqlExpression * expr, SharedHqlExpr & old
     IHqlExpression * ds = expr->queryChild(0);
     if (expr->hasProperty(newAtom))
     {
-
         oldDataset.set(ds);
         OwnedHqlExpr left = createSelector(no_left, ds, querySelSeq(expr));
         return createSelectExpr(left.getClear(), LINK(expr->queryChild(1)));
@@ -8574,7 +8573,6 @@ no_select
     attached to select node.  That may contain the following attributes:
         global  - is an outer level activity
         noTable - no other tables are active at this point      e.g., global[1].field;
-        relatedAccess - it contains access to other active tables.
         relatedTable - I think this could be deprecated in favour of the flag above.
 
 Datasets
@@ -8602,7 +8600,6 @@ void HqlScopeTagger::beginTableScope()
 void HqlScopeTagger::endTableScope(HqlExprArray & attrs, IHqlExpression * ds, IHqlExpression * newExpr)
 {
 #if 1
-    //This is still needed because the relatedAccess test doesn't seem to work correctly for HOLe datasets
     //A quick test which is often succeeds.
     if (isDatasetRelatedToScope(ds))
         attrs.append(*createAttribute(relatedTableAtom));