Merge pull request #7565 from ghalliday/issue13895

HPCC-13895 Fix problems with embedded datasets in nested rows

Reviewed-By: Jamie Noss <james.noss@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/hql/hqlattr.cpp

@@ -1286,7 +1286,7 @@ ITypeInfo * getSerializedForm(ITypeInfo * type, IAtom * variation)
         {
             //MORE: If (variant == internalAtom) consider using a format that prefixes the dataset with a count instead of a size
             OwnedITypeInfo noOutOfLineType = removeModifier(type, typemod_outofline);
-            OwnedITypeInfo noLinkCountType = removeProperty(noOutOfLineType, _linkCounted_Atom);
+            OwnedITypeInfo noLinkCountType = removeAttribute(noOutOfLineType, _linkCounted_Atom);
             ITypeInfo * childType = noLinkCountType->queryChildType();
             OwnedITypeInfo newChild = getSerializedForm(childType, variation);
             return replaceChildType(noLinkCountType, newChild);
@@ -1294,7 +1294,7 @@ ITypeInfo * getSerializedForm(ITypeInfo * type, IAtom * variation)
     case type_dictionary:
         {
             OwnedITypeInfo noOutOfLineType = removeModifier(type, typemod_outofline);
-            OwnedITypeInfo noLinkCountType = removeProperty(noOutOfLineType, _linkCounted_Atom);
+            OwnedITypeInfo noLinkCountType = removeAttribute(noOutOfLineType, _linkCounted_Atom);
             ITypeInfo * childType = noLinkCountType->queryChildType();
             OwnedITypeInfo newChild = getSerializedForm(childType, variation);
             if (variation == internalAtom)
@@ -3131,7 +3131,7 @@ ITypeInfo * cloneEssentialFieldModifiers(ITypeInfo * donor, ITypeInfo * rawtype)
     return type.getClear();
 }
 
-ITypeInfo * removeProperty(ITypeInfo * t, IAtom * search)
+ITypeInfo * removeAttribute(ITypeInfo * t, IAtom * search)
 {
     typemod_t curModifier = t->queryModifier();
     if (curModifier == typemod_none)
@@ -3145,7 +3145,7 @@ ITypeInfo * removeProperty(ITypeInfo * t, IAtom * search)
             return LINK(base);
     }
 
-    OwnedITypeInfo newBase = removeProperty(base, search);
+    OwnedITypeInfo newBase = removeAttribute(base, search);
     if (newBase == base)
         return LINK(t);
     return makeModifier(newBase.getClear(), curModifier, LINK(t->queryModifierExtra()));
@@ -3701,7 +3701,7 @@ ITypeInfo * setLinkCountedAttr(ITypeInfo * _type, bool setValue)
     {
         if (setValue)
             return LINK(type);
-        return removeProperty(type, _linkCounted_Atom);
+        return removeAttribute(type, _linkCounted_Atom);
     }
     else
     {
@@ -3735,7 +3735,7 @@ ITypeInfo * setStreamedAttr(ITypeInfo * _type, bool setValue)
     {
         if (setValue)
             return LINK(type);
-        return removeProperty(type, streamedAtom);
+        return removeAttribute(type, streamedAtom);
     }
     else
     {

ecl/hql/hqlattr.hpp

@@ -26,7 +26,7 @@ extern HQL_API IHqlExpression * queryAttribute(ITypeInfo * type, IAtom * search)
 extern HQL_API IHqlExpression * queryAttributeChild(ITypeInfo * type, IAtom * search, unsigned idx);
 extern HQL_API void cloneFieldModifier(Owned<ITypeInfo> & type, ITypeInfo * donorType, IAtom * attr);
 extern HQL_API ITypeInfo * cloneEssentialFieldModifiers(ITypeInfo * donor, ITypeInfo * rawtype);
-extern HQL_API ITypeInfo * removeProperty(ITypeInfo * type, IAtom * search);
+extern HQL_API ITypeInfo * removeAttribute(ITypeInfo * type, IAtom * search);
 
 extern HQL_API size32_t getMinRecordSize(IHqlExpression * record);
 extern HQL_API size32_t getExpectedRecordSize(IHqlExpression * record);

ecl/hql/hqlexpr.cpp

@@ -3328,6 +3328,41 @@ void CHqlExpression::initFlagsBeforeOperands()
     }
 }
 
+//Can be called from updateFlagsAfterOperands to help track down inconsistencies
+static bool verifyTransform(IHqlExpression * record, IHqlExpression * expr)
+{
+    ForEachChild(i, expr)
+    {
+        IHqlExpression * cur = expr->queryChild(i);
+        switch (cur->getOperator())
+        {
+        case no_assignall:
+            if (!verifyTransform(record, cur))
+                return false;
+            break;
+        case no_assign:
+            {
+                IHqlExpression * lhs = cur->queryChild(0);
+                if (lhs->getOperator() == no_select)
+                {
+                    if ((lhs->queryChild(0)->getOperator() == no_select) &&
+                        (lhs->queryChild(0)->queryChild(0)->getOperator() == no_self))
+                        return false;
+
+                    IHqlExpression * field = lhs->queryChild(1);
+                    OwnedHqlExpr match = record->querySimpleScope()->lookupSymbol(field->queryId());
+                    if (match != field)
+                        return false;
+                }
+                break;
+            }
+            break;
+        }
+    }
+    return true;
+}
+
+
 void CHqlExpression::updateFlagsAfterOperands()
 {
 //  DBGLOG("%p: Create(%s) type = %lx", (unsigned)(IHqlExpression *)this, getOpString(op), (unsigned)type);

ecl/hql/hqlutil.cpp

@@ -8960,6 +8960,8 @@ IHqlExpression * queryTransformAssign(IHqlExpression * transform, IHqlExpression
                 IHqlExpression * lhs = cur->queryChild(0)->queryChild(1);
                 if (lhs == searchField)
                     return cur;
+                if (lhs->queryId() == searchField->queryId())
+                    return cur;
                 break;
             }
         }

ecl/hqlcpp/hqlcerrors.hpp

@@ -318,6 +318,7 @@
 #define HQLERR_UnknownCompoundAssign            4837
 #define HQLERR_ReadSpillBeforeWriteFix          4838
 #define HQLERR_AccessUnavailableGraph           4839
+#define HQLERR_NoMappingForField                4840
 //#define HQLERR_Max                            4999
 
 //---- Text for all errors (make it easy to internationalise) ---------------------------
@@ -603,6 +604,7 @@
 #define HQLERR_ReadSpillBeforeWriteFix_Text     "INTERNAL: Attempt to read spill file %s before it is written.  Try adding #option ('allowThroughSpill', false); to the query."
 #define HQLERR_CouldNotGenerateDefault_Text     "INTERNAL: Could not generate default value for field %s"
 #define HQLERR_AccessUnavailableGraph_Text      "INTERNAL: Attempt to access result from unavailable graph (%s)"
+#define HQLERR_NoMappingForField_Text           "INTERNAL: Mapping for field %s is missing from transform"
 
 #define WARNINGAT(cat, e, x)                 reportWarning(cat, SeverityUnknown, e, x, x##_Text)
 #define WARNINGAT1(cat, e, x, a)             reportWarning(cat, SeverityUnknown, e, x, x##_Text, a)

ecl/hqlcpp/hqliproj.cpp

@@ -211,6 +211,16 @@ bool UsedFieldSet::contains(IAtom * name) const
     return false;
 }
 
+IHqlExpression * UsedFieldSet::findByName(IAtom * name) const
+{
+    ForEachItemIn(i, fields)
+    {
+        if (fields.item(i).queryName() == name)
+            return &fields.item(i);
+    }
+    return NULL;
+}
+
 //Calculate left - right
 void UsedFieldSet::createDifference(const UsedFieldSet & left, const UsedFieldSet & right)
 {
@@ -272,7 +282,12 @@ NestedField * UsedFieldSet::findNestedByName(IHqlExpression * field) const
 IHqlExpression * UsedFieldSet::createFilteredAssign(IHqlExpression * field, IHqlExpression * value, IHqlExpression * newSelf, const UsedFieldSet * exceptions) const
 {
     if (!contains(*field))
+    {
+        IHqlExpression * match = findByName(field->queryName());
+        if (match)
+            return createFilteredAssign(match, value, newSelf, exceptions);
         return NULL;
+    }
 
     OwnedHqlExpr newValue = LINK(value);
     OwnedHqlExpr newField;
@@ -539,7 +554,8 @@ void UsedFieldSet::gatherTransformValuesUsed(HqlExprArray * selfSelects, HqlExpr
                 IHqlExpression * transformValue = queryTransformAssignValue(transform, &cur);
                 //If no transform value is found then we almost certainly have an invalid query (e.g, LEFT inside a
                 //global).  Don't add the value - you'll definitely get a later follow on error
-                assertex(transformValue);
+                if (!transformValue)
+                    throwError1(HQLERR_NoMappingForField, str(cur.queryId()));
                 values->append(*LINK(transformValue));
             }
         }

ecl/hqlcpp/hqliproj.ipp

@@ -158,6 +158,7 @@ public:
 protected:
     bool contains(IHqlExpression & field) const;
     bool contains(IAtom * name) const; // debugging only
+    IHqlExpression * findByName(IAtom * name) const;
     IHqlExpression * createFilteredAssign(IHqlExpression * field, IHqlExpression * value, IHqlExpression * newSelf, const UsedFieldSet * exceptions) const;
     void createFilteredAssigns(HqlExprArray & assigns, IHqlExpression * transform, IHqlExpression * newSelf, const UsedFieldSet * exceptions) const;
     IHqlExpression * createRowTransform(IHqlExpression * row, const UsedFieldSet * exceptions) const;

ecl/hqlcpp/hqlttcpp.cpp

@@ -9054,10 +9054,14 @@ IHqlExpression * HqlLinkedChildRowTransformer::createTransformedBody(IHqlExpress
                     if (expr->hasAttribute(embeddedAtom) || queryAttribute(type, embeddedAtom) || expr->hasAttribute(countAtom) || expr->hasAttribute(sizeofAtom))
                         throwError1(HQLERR_InconsistentEmbedded, str(expr->queryId()));
                 }
-                if (expr->hasAttribute(embeddedAtom))
+                if (expr->hasAttribute(embeddedAtom) || queryAttribute(type, embeddedAtom))
                 {
-                    OwnedHqlExpr transformed = QuickHqlTransformer::createTransformedBody(expr);
-                    return removeAttribute(transformed, embeddedAtom);
+                    HqlExprArray args;
+                    transformChildren(expr, args);
+                    removeAttribute(args, embeddedAtom);
+                    OwnedITypeInfo newType = transformType(type);
+                    OwnedITypeInfo cleanType = removeAttribute(newType, embeddedAtom);
+                    return createField(expr->queryId(), cleanType.getClear(), args);
                 }
                 if (implicitLinkedChildRows && !expr->hasAttribute(_linkCounted_Atom) && !queryAttribute(type, embeddedAtom))
                 {
@@ -9094,6 +9098,7 @@ IHqlExpression * HqlLinkedChildRowTransformer::createTransformedBody(IHqlExpress
             }
         }
         break;
+    case no_attr:
     case no_embedbody:
         //Don't change the type of an embed body - otherwise result it will become link counted when not expected.
         return LINK(expr);