HPCC-8335 Fix more issues with DEFAULT on field

Includes:
- Fix problems with checking the range of default values
- Use createExprAttr(defaultAtom) to avoid inconsistencies
- Preserve default when creating a simplified record
- key file for the default example

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

common/deftype/defvalue.cpp

@@ -100,23 +100,28 @@ int rangeCompare(double value, ITypeInfo * targetType)
     }
     else
     {
+        if (value < 0)
+            return -1;
         switch (targetType->getTypeCode())
         {
         case type_decimal:
-            if (value < 0)
-                return -1;
             if (value >= powerOfTen[targetType->getDigits()-targetType->getPrecision()])
                 return +1;
             break;
         case type_int:
         case type_swapint:
-            if (value < 0)
-                return -1;
             if (value > (double)maxUIntValue[targetType->getSize()])
                 return +1;
             break;
         case type_packedint:
             return rangeCompare(value, targetType->queryPromotedType());
+        case type_bitfield:
+            {
+                unsigned __int64 maxValue = (1 << targetType->getBitSize()) - 1;
+                if (value > maxValue)
+                    return +1;
+                break;
+            }
         }
     }
     return 0;

ecl/hql/hqlexpr.cpp

@@ -2749,7 +2749,6 @@ IHqlExpression * ensureExprType(IHqlExpression * expr, ITypeInfo * type, node_op
         return LINK(expr);
 
     ITypeInfo * exprType = queryUnqualifiedType(qualifiedType);
-//    type = queryUnqualifiedType(type);
     if (exprType == queryUnqualifiedType(type))
         return LINK(expr);
 
@@ -14170,7 +14169,7 @@ static void simplifyRecordTypes(HqlExprArray & fields, IHqlExpression * cur, boo
 
             ITypeInfo * type = cur->queryType();
             Owned<ITypeInfo> targetType;
-            OwnedHqlExpr attrs;
+            HqlExprArray attrs;
             switch (type->getTypeCode())
             {
             case type_groupedtable:
@@ -14194,9 +14193,10 @@ static void simplifyRecordTypes(HqlExprArray & fields, IHqlExpression * cur, boo
                     if (countAttr || cur->hasProperty(sizeofAtom))
                         forceSimplify = true;
 
-                    attrs.set(maxCountAttr);
+                    if (maxCountAttr)
+                        attrs.append(*LINK(maxCountAttr));
                     if (countAttr && !maxCountAttr && countAttr->queryChild(0)->queryValue())
-                        attrs.setown(createAttribute(maxCountAtom, LINK(countAttr->queryChild(0))));
+                        attrs.append(*createAttribute(maxCountAtom, LINK(countAttr->queryChild(0))));
                     break;
                 }
             case type_set:
@@ -14223,15 +14223,15 @@ static void simplifyRecordTypes(HqlExprArray & fields, IHqlExpression * cur, boo
                 break;
             }
 
-            IHqlExpression * xmlAttr = cur->queryProperty(xpathAtom);
-            if (xmlAttr)
-                attrs.setown(createComma(attrs.getClear(), LINK(xmlAttr)));
-
             LinkedHqlExpr newField = cur;
             if (forceSimplify || type != targetType)
             {
                 needsTransform = true;
-                newField.setown(createField(cur->queryName(), targetType.getLink(), NULL, attrs.getClear()));
+                //MORE xmldefault, default
+                inheritAttribute(attrs, cur, xpathAtom);
+                inheritAttribute(attrs, cur, xmlDefaultAtom);
+                inheritAttribute(attrs, cur, defaultAtom);
+                newField.setown(createField(cur->queryName(), targetType.getLink(), attrs));
             }
             fields.append(*LINK(newField));
             break;

ecl/hql/hqlgram.y

@@ -4406,7 +4406,7 @@ fieldAttr
                         }
     | DEFAULT '(' constExpression ')'
                         {
-                            $$.setExpr(createAttribute(defaultAtom, $3.getExpr()), $1);
+                            $$.setExpr(createExprAttribute(defaultAtom, $3.getExpr()), $1);
                         }
     | STRING_CONST
                         {

ecl/hql/hqlgram2.cpp

@@ -2270,31 +2270,31 @@ void HqlGram::addField(const attribute &errpos, _ATOM name, ITypeInfo *_type, IH
             value = newValue;
         }
     }
-    if (attrs)
+
+    IHqlExpression * defaultAttr = queryPropertyInList(defaultAtom, attrs);
+    if (defaultAttr)
     {
-        HqlExprArray allAttrs;
-        attrs->unwindList(allAttrs, no_comma);
-        IHqlExpression * defaultAttr = queryProperty(defaultAtom, allAttrs);
-        if (defaultAttr)
+        IHqlExpression * defaultValue = defaultAttr->queryChild(0);
+        if (defaultValue)
         {
-            IHqlExpression * defaultValue = defaultAttr->queryChild(0);
-            if (defaultValue)
+            ITypeInfo * defvalueType = defaultValue->queryType();
+            if (defvalueType != expectedType)
             {
-                ITypeInfo * defvalueType = defaultValue->queryType();
-                if (defvalueType != expectedType)
+                if (!expectedType->assignableFrom(defvalueType->queryPromotedType()))
+                    canNotAssignTypeError(fieldType,defvalueType,errpos);
+                IValue * constValue = defaultValue->queryValue();
+                if (constValue && (constValue->rangeCompare(expectedType) > 0))
+                    reportWarning(ERR_TYPE_INCOMPATIBLE, errpos.pos, "%s", "Default value too large");
+                if (expectedType->getTypeCode() != type_row)
                 {
-                    if (!expectedType->assignableFrom(defvalueType->queryPromotedType()))
-                        canNotAssignTypeError(fieldType,defvalueType,errpos);
-                    if (castLosesInformation(expectedType, defvalueType))
-                        reportWarning(ERR_TYPE_INCOMPATIBLE, errpos.pos, "%s", "Default value too large");
-                    if (expectedType->getTypeCode() != type_row)
-                    {
-                        IHqlExpression * newValue = ensureExprType(defaultValue, expectedType);
-                        allAttrs.zap(*defaultAttr);
-                        allAttrs.append(*createAttribute(defaultAtom, newValue));
-                        attrs->Release();
-                        attrs = createComma(allAttrs);
-                    }
+                    HqlExprArray allAttrs;
+                    attrs->unwindList(allAttrs, no_comma);
+
+                    IHqlExpression * newValue = ensureExprType(defaultValue, expectedType);
+                    allAttrs.zap(*defaultAttr);
+                    allAttrs.append(*createExprAttribute(defaultAtom, newValue));
+                    attrs->Release();
+                    attrs = createComma(allAttrs);
                 }
             }
         }
@@ -2364,6 +2364,7 @@ void HqlGram::addDatasetField(const attribute &errpos, _ATOM name, IHqlExpressio
         attrs = extractAttrsFromExpr(value);
 
     ITypeInfo * type = makeTableType(makeRowType(createRecordType(record)), NULL, NULL, NULL);
+
     IHqlExpression *newField = createField(name, type, value, attrs);
     addToActiveRecord(newField);
     record->Release();
@@ -6193,6 +6194,7 @@ IHqlExpression * HqlGram::checkParameter(const attribute * errpos, IHqlExpressio
             }
             return NULL;
         }
+        break;
     }
 
 //  if (formal->hasProperty(constAtom) && funcdef && !isExternalFunction(funcdef))
@@ -7875,7 +7877,8 @@ void HqlGram::modifyIndexPayloadRecord(SharedHqlExpr & record, SharedHqlExpr & p
                 break;
         }
 
-        fields.append(*createField(implicitFieldName, makeIntType(8, false), createConstant(I64C(0)), createAttribute(_implicitFpos_Atom)));
+        ITypeInfo * fileposType = makeIntType(8, false);
+        fields.append(*createField(implicitFieldName, fileposType, createConstant(I64C(0)), createAttribute(_implicitFpos_Atom)));
         payloadCount++;
     }
 

ecl/hql/hqlutil.cpp

@@ -4196,6 +4196,13 @@ IHqlExpression * appendOwnedOperandsF(IHqlExpression * expr, ...)
 }
 
 
+extern HQL_API void inheritAttribute(HqlExprArray & attrs, IHqlExpression * donor, _ATOM name)
+{
+    IHqlExpression * match = donor->queryProperty(name);
+    if (match)
+        attrs.append(*LINK(match));
+}
+
 IHqlExpression * inheritAttribute(IHqlExpression * expr, IHqlExpression * donor, _ATOM name)
 {
     return appendOwnedOperand(expr, LINK(donor->queryProperty(name)));

ecl/hql/hqlutil.hpp

@@ -158,6 +158,7 @@ extern HQL_API IHqlExpression * appendOwnedOperand(IHqlExpression * expr, IHqlEx
 extern HQL_API IHqlExpression * replaceOwnedProperty(IHqlExpression * expr, IHqlExpression * ownedProeprty);
 extern HQL_API IHqlExpression * appendOwnedOperandsF(IHqlExpression * expr, ...);
 extern HQL_API IHqlExpression * inheritAttribute(IHqlExpression * expr, IHqlExpression * donor, _ATOM name);
+extern HQL_API void inheritAttribute(HqlExprArray & attrs, IHqlExpression * donor, _ATOM name);
 extern HQL_API bool hasOperand(IHqlExpression * expr, IHqlExpression * child);
 
 extern HQL_API unsigned numRealChildren(IHqlExpression * expr);

ecl/hqlcpp/hqliproj.cpp

@@ -320,7 +320,7 @@ IHqlExpression * UsedFieldSet::createFilteredAssign(IHqlExpression * field, IHql
     {
         if (exceptions && exceptions->contains(*field))
         {
-            newValue.setown(createNullExpr(field));
+            newValue.setown(createNullExpr(newField ? newField.get() : field));
         }
     }
 
@@ -428,6 +428,8 @@ void UsedFieldSet::calcFinalRecord(bool canPack, bool ignoreIfEmpty)
             {
                 HqlExprArray args;
                 unwindChildren(args, &cur);
+                //MORE: Any default will now have the wrong type => remove it for the moment (ideally it would be projected)
+                removeProperty(args, defaultAtom);
                 OwnedHqlExpr newField = createField(cur.queryName(), makeRowType(newRecord->getType()), args);
                 recordFields.append(*newField.getClear());
             }

ecl/hqlcpp/hqlsource.cpp

@@ -3750,6 +3750,8 @@ static void createExpanded(HqlExprArray & fields, IHqlExpression * expr)
                 {
                     HqlExprArray attrs;
                     unwindChildren(attrs, expr);
+                    //MORE: Any default will now have the wrong type => remove it for the moment (ideally it would be projected)
+                    removeProperty(attrs, defaultAtom);
                     fields.append(*createField(expr->queryName(), LINK(expandedType), attrs));
                 }
             }

testing/ecl/key/default.xml

@@ -0,0 +1,30 @@
+<Dataset name='Result 1'>
+ <Row><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 2'>
+ <Row><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 3'>
+ <Row><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 4'>
+ <Row><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 5'>
+ <Row><v>0</v><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 6'>
+ <Row><v>0</v><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 7'>
+ <Row><v>0</v><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 8'>
+ <Row><v>0</v><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 9'>
+ <Row><f>B</f></Row>
+</Dataset>
+<Dataset name='Result 10'>
+ <Row><Result_10>B</Result_10></Row>
+</Dataset>