WIP: Dictionary support

Various code cleanup in response to code review.
Added proper type checks for ROW IN dict.

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>

ecl/hql/hqlexpr.cpp

@@ -4128,23 +4128,7 @@ bool CHqlExpression::isDataset()
 
 bool CHqlExpression::isDictionary()
 {
-    ITypeInfo * cur = queryType();
-    loop
-    {
-        if (!cur)
-            return false;
-
-        switch(cur->getTypeCode())
-        {
-        case type_dictionary:
-            return true;
-        case type_function:
-            cur = cur->queryChildType();
-            break;
-        default:
-            return false;
-        }
-    }
+    return matchesTypeCode(queryType(), type_dictionary);
 }
 
 bool CHqlExpression::isDatarow() 
@@ -9937,7 +9921,7 @@ IHqlExpression *createDictionary(node_operator op, HqlExprArray & parms)
             type.setown(makeDictionaryType(rowType));
         }
         else
-            type.setown(getTypeFromMeta(record, metadata, 0));
+            UNIMPLEMENTED_XY("Type calculation for dictionary operator", getOpString(op));
 
         if (linkCounted)
             type.setown(setLinkCountedAttr(type, true));
@@ -15817,10 +15801,8 @@ IHqlExpression * createTypeTransfer(IHqlExpression * expr, ITypeInfo * _newType)
     case type_table:
     case type_groupedtable:
         return createDataset(no_typetransfer, LINK(queryOriginalRecord(newType)), expr);
-        break;
     case type_dictionary:
         return createDictionary(no_typetransfer, LINK(queryOriginalRecord(newType)), expr);
-        break;
     default:
         return createValue(no_typetransfer, newType.getClear(), expr);
     }

ecl/hql/hqlgram.y

@@ -3776,8 +3776,7 @@ recordDef
                         {
                             OwnedHqlExpr record = $4.getExpr();
                             parser->checkRecordIsValid($1, record);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
 
     | startrecord fieldDefs payloadPart endrecord
@@ -3785,16 +3784,14 @@ recordDef
                             OwnedHqlExpr record = $3.getExpr();
                             OwnedHqlExpr payload = $4.getExpr();
                             parser->mergeDictionaryPayload(record, payload, $1);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
 
     | startrecord recordOptions fieldDefs optSemiComma endrecord
                         {
                             OwnedHqlExpr record = $5.getExpr();
                             parser->checkRecordIsValid($1, record);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
 
     | startrecord recordOptions fieldDefs payloadPart endrecord
@@ -3802,16 +3799,14 @@ recordDef
                             OwnedHqlExpr record = $4.getExpr();
                             OwnedHqlExpr payload = $5.getExpr();
                             parser->mergeDictionaryPayload(record, payload, $1);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
 
     | startrecord recordBase optFieldDefs endrecord
                         {
                             OwnedHqlExpr record = $4.getExpr();
                             parser->checkRecordIsValid($1, record);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
 
     | startrecord recordBase optFieldDefs payloadPart endrecord
@@ -3819,16 +3814,14 @@ recordDef
                             OwnedHqlExpr record = $4.getExpr();
                             OwnedHqlExpr payload = $5.getExpr();
                             parser->mergeDictionaryPayload(record, payload, $1);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
 
     | startrecord recordBase recordOptions optFieldDefs endrecord
                         {
                             OwnedHqlExpr record = $5.getExpr();
                             parser->checkRecordIsValid($1, record);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
 
     | startrecord recordBase recordOptions optFieldDefs payloadPart endrecord
@@ -3836,8 +3829,7 @@ recordDef
                             OwnedHqlExpr record = $5.getExpr();
                             OwnedHqlExpr payload = $6.getExpr();
                             parser->mergeDictionaryPayload(record, payload, $1);
-                            $$.setExpr(record.getClear());
-                            $$.setPosition($1);
+                            $$.setExpr(record.getClear(), $1);
                         }
     | simpleRecord
     | recordDef AND recordDef
@@ -4254,15 +4246,15 @@ fieldDef
                         }
 
 
-    | DICTIONARY '(' recordDef childDictionaryOptions ')' knownOrUnknownId optFieldAttrs
+    | DICTIONARY '(' recordDef ')' knownOrUnknownId optFieldAttrs
                         {
                             $$.clear($1);
-                            parser->addDictionaryField($6, $6.getName(), $3.getExpr(), NULL, createComma($4.getExpr(), $7.getExpr()));
+                            parser->addDictionaryField($5, $5.getName(), $3.getExpr(), NULL, $6.getExpr());
                         }
-    | DICTIONARY '(' recordDef childDictionaryOptions ')' knownOrUnknownId optFieldAttrs ASSIGN dictionary
+    | DICTIONARY '(' recordDef ')' knownOrUnknownId optFieldAttrs ASSIGN dictionary
                         {
                             $$.clear($1);
-                            parser->addDictionaryField($6, $6.getName(), $3.getExpr(), $9.getExpr(), createComma($4.getExpr(), $7.getExpr()));
+                            parser->addDictionaryField($5, $5.getName(), $3.getExpr(), $8.getExpr(), $6.getExpr());
                         }
     | DICTIONARY knownOrUnknownId optFieldAttrs ASSIGN dictionary
                         {
@@ -4703,24 +4695,6 @@ userTypedefPattern
     ;
 
 
-childDictionaryOptions
-    :
-                        { $$.setNullExpr(); }
-    | childDictionaryOptions ',' childDictionaryOption
-                        {
-                            $$.setExpr(createComma($1.getExpr(), $3.getExpr()));
-                        }
-    ;
-
-childDictionaryOption
-    : COUNT '(' expression ')'
-                        {
-                            parser->normalizeExpression($3);
-                            $$.setExpr(createLinkAttribute(countAtom, $3.getExpr()));
-                            $$.setPosition($1);
-                        }
-    ;
-
 childDatasetOptions
     :
                         { $$.setNullExpr(); }
@@ -5084,8 +5058,7 @@ compareExpr
                             parser->normalizeExpression($4, type_dictionary, false);
                             IHqlExpression *dict = $4.getExpr();
                             IHqlExpression *row = $1.getExpr();
-                            OwnedHqlExpr indict = createBoolExpr(no_indict, row, dict); // more - some type checking might be nice
-//                          OwnedHqlExpr indict = createINDictExpr(parser->errorHandler, $4.pos, row, dict);
+                            OwnedHqlExpr indict = createINDictRow(parser->errorHandler, $4.pos, row, dict);
                             $$.setExpr(getInverse(indict));
                             $$.setPosition($3);
                         }
@@ -5106,8 +5079,7 @@ compareExpr
                             parser->normalizeExpression($3, type_dictionary, false);
                             IHqlExpression *dict = $3.getExpr();
                             IHqlExpression *row = $1.getExpr();
-                            $$.setExpr(createBoolExpr(no_indict, row, dict)); // more - some type checking might be nice
-//                            $$.setExpr(createINDictExpr(parser->errorHandler, $3.pos, row, dict));
+                            $$.setExpr(createINDictRow(parser->errorHandler, $3.pos, row, dict));
                             $$.setPosition($2);
                         }
     | dataSet EQ dataSet    

ecl/hql/hqlgram2.cpp

@@ -572,8 +572,7 @@ IHqlExpression* HqlGram::endRecordDef()
     IHqlExpression * record = popRecord();
     record->Link();     // logically link should be in startrecord, but can only link after finished updating
     popSelfScope();
-    OwnedHqlExpr newRecord = record->closeExpr();
-    return newRecord.getClear();
+    return record->closeExpr();
 }
 
 void HqlGram::beginFunctionCall(attribute & function)
@@ -8424,9 +8423,11 @@ void HqlGram::createAppendDictionaries(attribute & targetAttr, attribute & leftA
 {
     OwnedHqlExpr left = leftAttr.getExpr();
     OwnedHqlExpr right = rightAttr.getExpr();
-//    if (left->isDatarow())
-//        left.setown(createDatasetFromRow(LINK(left)));
+    assertex(!left->isDictionary());
+    if (!right->isDictionary())
+        reportError(WRN_UNSUPPORTED_FEATURE, rightAttr, "Only dictionary may be appended to dictionary");
     right.setown(checkEnsureRecordsMatch(left, right, rightAttr, right->isDatarow()));
+    // TODO: support for dict + row, dict + dataset
 //    if (right->isDatarow())
 //        right.setown(createDatasetFromRow(LINK(right)));
     IHqlExpression * attr = kind ? createAttribute(kind) : NULL;

ecl/hql/hqlutil.cpp

@@ -5327,15 +5327,14 @@ void TempTableTransformer::reportWarning(IHqlExpression * location, int code,con
 IHqlExpression *getDictionaryKeyRecord(IHqlExpression *record)
 {
     // MORE - should probably use an attr to cache this?
-    IHqlExpression * payload = record ? record->queryProperty(_payload_Atom) : NULL;
+    IHqlExpression * payload = record->queryProperty(_payload_Atom);
     unsigned payloadSize = payload ? getIntValue(payload->queryChild(0)) : 0;
     unsigned max = record->numChildren() - payloadSize;
     IHqlExpression *newrec = createRecord();
-    HqlExprArray fields;
     for (unsigned idx = 0; idx < max; idx++)
     {
         IHqlExpression *child = record->queryChild(idx);
-        if (!child->isAttribute())  // Strip off the payload attribute
+        if (!child->isAttribute() || child->queryName()!=_payload_Atom)  // Strip off the payload attribute
             newrec->addOperand(LINK(child));
     }
     return newrec->closeExpr();
@@ -5357,6 +5356,13 @@ IHqlExpression *createINDictExpr(IErrorReceiver * errors, ECLlocation & location
     return createBoolExpr(no_indict, createRow(no_createrow, newTransform.getClear()), dict);
 }
 
+IHqlExpression *createINDictRow(IErrorReceiver * errors, ECLlocation & location, IHqlExpression *row, IHqlExpression *dict)
+{
+    OwnedHqlExpr record = getDictionaryKeyRecord(dict->queryRecord());
+    if (!record->queryType()->assignableFrom(row->queryType()))
+        errors->reportError(ERR_TYPE_DIFFER, "Type mismatch", location.sourcePath->str(), location.lineno, location.column, location.position);
+    return createBoolExpr(no_indict, row, dict);
+}
 
 IHqlExpression * convertTempRowToCreateRow(IErrorReceiver * errors, ECLlocation & location, IHqlExpression * expr)
 {

ecl/hql/hqlutil.hpp

@@ -449,6 +449,7 @@ extern HQL_API bool isNullList(IHqlExpression * expr);
 
 extern HQL_API IHqlExpression * createSelectMapRow(IErrorReceiver * errors, ECLlocation & location, IHqlExpression * dict, IHqlExpression *values);
 extern HQL_API IHqlExpression * createINDictExpr(IErrorReceiver * errors, ECLlocation & location, IHqlExpression *expr, IHqlExpression *dict);
+extern HQL_API IHqlExpression *createINDictRow(IErrorReceiver * errors, ECLlocation & location, IHqlExpression *row, IHqlExpression *dict);
 extern HQL_API IHqlExpression * convertTempRowToCreateRow(IErrorReceiver * errors, ECLlocation & location, IHqlExpression * expr);
 extern HQL_API IHqlExpression * convertTempTableToInlineTable(IErrorReceiver * errors, ECLlocation & location, IHqlExpression * expr);
 extern HQL_API IHqlExpression * convertTempTableToInlineDictionary(IErrorReceiver * errors, ECLlocation & location, IHqlExpression * expr);

ecl/hqlcpp/hqlcppds.cpp

@@ -4255,18 +4255,8 @@ IReferenceSelector * HqlCppTranslator::buildDatasetSelectMap(BuildCtx & ctx, IHq
         return createReferenceSelector(static_cast<BoundRow *>(match));
 
     OwnedHqlExpr dataset = normalizeAnyDatasetAliases(expr->queryChild(0));
-    // Create a row to pass to the lookup function
-
-
     BoundRow * row = NULL;
-    if (!canProcessInline(&ctx, expr))
-    {
-        CHqlBoundExpr bound;
-        OwnedHqlExpr dsExpr = expr->isDatarow() ? createDatasetFromRow(LINK(expr)) : LINK(expr);
-        buildDataset(ctx, dsExpr, bound, FormatNatural);
-        convertBoundDatasetToFirstRow(expr, bound);
-        row = bindRow(ctx, expr, bound.expr);
-    }
+    assertex(canProcessInline(&ctx, expr));
 
     if (!row)
     {

ecl/hqlcpp/hqlhtcpp.cpp

@@ -5309,7 +5309,7 @@ void HqlCppTranslator::buildDictionaryHashClass(BuildCtx &ctx, IHqlExpression *r
         BuildCtx classctx(declarectx);
         beginNestedClass(classctx, lookupHelperName, "IHThorHashLookupInfo");
         HqlExprArray keyedFields;
-        IHqlExpression * payload = record ? record->queryProperty(_payload_Atom) : NULL;
+        IHqlExpression * payload = record->queryProperty(_payload_Atom);
         unsigned payloadSize = payload ? getIntValue(payload->queryChild(0)) : 0;
         unsigned max = record->numChildren() - payloadSize;
         for (unsigned idx = 0; idx < max; idx++)

ecl/hqlcpp/hqlwcpp.cpp

@@ -1204,12 +1204,6 @@ StringBuffer & HqlCppWriter::generateExprCpp(IHqlExpression * expr)
             generateChildExpr(expr, 1);
             out.append(']');
             break;
-        case no_selectmap:
-            generateChildExpr(expr, 0);
-            out.append("[<");
-            generateChildExpr(expr, 1);
-            out.append(">]");
-            break;
         case no_postinc:
         case no_postdec:
             generateChildExpr(expr, 0);

testing/ecl/dict2.ecl

@@ -24,13 +24,13 @@ count(d1n) = 1;
 
 d2[5,2].name = 'Richard';
 d2[5,1].name = '';
-ROW({5,2}, { integer i1, integer i2} ) in d2;
-ROW({5,1}, { integer i1, integer i2} ) not in d2;
+ROW({5,2}, { integer id1, integer id2} ) in d2;
+ROW({5,1}, { integer id1, integer id2} ) not in d2;
 count(d2) = 1;
 d2n[5,2].name = 'Richard';
 d2n[5,1].name = '';
-ROW({5,2}, { integer i1, integer i2} ) in d2n;
-ROW({5,1}, { integer i1, integer i2} ) not in d2n;
+ROW({5,2}, { integer id1, integer id2} ) in d2n;
+ROW({5,1}, { integer id1, integer id2} ) not in d2n;
 count(d2n) = 1;
 
 5 not in d3;