Merge pull request #11317 from ghalliday/issue19931

HPCC-19931 Refactoring in preparation for index field projection

Reviewed-By: Shamser Ahmed <shamser.ahmed@lexisnexis.co.uk>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/hqlcpp/hqlckey.cpp

@@ -1365,7 +1365,7 @@ ABoundActivity * HqlCppTranslator::doBuildActivityKeyedJoinOrDenormalize(BuildCt
         //Remove virtual attributes from the record, so the crc will be compatible with the disk read record
         //can occur with a (highly unusual) full keyed join to a persist file... (see indexread14.ecl)
         OwnedHqlExpr noVirtualRecord = removeVirtualAttributes(info.queryRawRhs()->queryRecord());
-        buildFormatCrcFunction(instance->classctx, "getDiskFormatCrc", false, noVirtualRecord, NULL, 0);
+        buildFormatCrcFunction(instance->classctx, "getDiskFormatCrc", noVirtualRecord);
         buildEncryptHelper(instance->startctx, info.queryFile()->queryAttribute(encryptAtom), "getFileEncryptKey");
     }
 

ecl/hqlcpp/hqlcpp.ipp

@@ -1831,6 +1831,8 @@ public:
 
     void buildEncryptHelper(BuildCtx & ctx, IHqlExpression * encryptAttr, const char * funcname = NULL);
     void buildFormatCrcFunction(BuildCtx & ctx, const char * name, bool removeFilepos, IHqlExpression * dataset, IHqlExpression * expr, unsigned payloadDelta);
+    void buildFormatCrcFunction(BuildCtx & ctx, const char * name, IHqlExpression * record, unsigned payloadSize);
+    void buildFormatCrcFunction(BuildCtx & ctx, const char * name, IHqlExpression * record);
     void buildLimitHelpers(BuildCtx & ctx, IHqlExpression * expr, IHqlExpression * filename, unique_id_t id);
     void buildLimitHelpers(BuildCtx & ctx, IHqlExpression * rowLimit, IHqlExpression * failAction, bool isSkip, IHqlExpression * filename, unique_id_t id);
 

ecl/hqlcpp/hqlhtcpp.cpp

@@ -10188,8 +10188,7 @@ void HqlCppTranslator::buildRecordEcl(BuildCtx & subctx, IHqlExpression * record
 void HqlCppTranslator::buildFormatCrcFunction(BuildCtx & ctx, const char * name, bool removeFilepos, IHqlExpression * dataset, IHqlExpression * expr, unsigned payloadDelta)
 {
     IHqlExpression * payload = expr ? expr->queryAttribute(_payload_Atom) : NULL;
-    // MORE - do we need to keep this consistent - if so will have to trim out the originals and the filepos
-    OwnedHqlExpr exprToCrc = getSerializedForm(dataset->queryRecord(), diskAtom);
+    OwnedHqlExpr exprToCrc = LINK(dataset->queryRecord());
 
     unsigned payloadSize = getBoolAttribute(expr, filepositionAtom, true) ? 1 : 0;
     if (payload)
@@ -10206,13 +10205,23 @@ void HqlCppTranslator::buildFormatCrcFunction(BuildCtx & ctx, const char * name,
         exprToCrc.setown(exprToCrc->clone(args));
     }
 
-    exprToCrc.setown(createComma(exprToCrc.getClear(), getSizetConstant(payloadSize)));
+    buildFormatCrcFunction(ctx, name, exprToCrc, payloadSize);
+}
+
+void HqlCppTranslator::buildFormatCrcFunction(BuildCtx & ctx, const char * name, IHqlExpression * record, unsigned payloadSize)
+{
+    OwnedHqlExpr exprToCrc = createComma(LINK(record), getSizetConstant(payloadSize));
 
     traceExpression("crc:", exprToCrc);
     OwnedHqlExpr crc = getSizetConstant(getExpressionCRC(exprToCrc));
     doBuildUnsignedFunction(ctx, name, crc);
 }
 
+void HqlCppTranslator::buildFormatCrcFunction(BuildCtx & ctx, const char * name, IHqlExpression * record)
+{
+    buildFormatCrcFunction(ctx, name, record, 1);
+}
+
 static void createOutputIndexRecord(HqlMapTransformer & mapper, HqlExprArray & fields, IHqlExpression * record, bool hasFileposition, bool allowTranslate)
 {
     unsigned numFields = record->numChildren();
@@ -10965,7 +10974,8 @@ ABoundActivity * HqlCppTranslator::doBuildActivityOutput(BuildCtx & ctx, IHqlExp
         if (!pipe)
         {
             OwnedHqlExpr noVirtualRecord = removeVirtualAttributes(dataset->queryRecord());
-            buildFormatCrcFunction(instance->classctx, "getFormatCrc", false, noVirtualRecord, NULL, 0);
+            OwnedHqlExpr serializedRecord = getSerializedForm(noVirtualRecord, diskAtom);
+            buildFormatCrcFunction(instance->classctx, "getFormatCrc", serializedRecord);
         }
 
         bool grouped = isGrouped(dataset);

ecl/hqlcpp/hqlsource.cpp

@@ -864,11 +864,9 @@ void SourceBuilder::analyse(IHqlExpression * expr)
     {
     case no_table:
     case no_newkeyindex:
-        if (fieldInfo.hasVirtuals() && !newDiskReadMapping)
+        if (!newDiskReadMapping)
         {
-            assertex(fieldInfo.simpleVirtualsAtEnd);
-            needToCallTransform = true;
-            needDefaultTransform = false;
+            assertex(!fieldInfo.hasVirtuals());
         }
         if (newDiskReadMapping)
         {
@@ -1165,17 +1163,7 @@ void SourceBuilder::buildTransformBody(BuildCtx & transformCtx, IHqlExpression *
         }
         else
         {
-            if (newDiskReadMapping)
-            {
-                translator.bindTableCursor(transformCtx, projectedSelector, "left");
-            }
-            else
-            {
-                //NOTE: The source is not link counted - it comes from a prefetched row, and does not include any virtual file position field.
-                OwnedHqlExpr boundSrc = createVariable("left", makeRowReferenceType(physicalRecord));
-                IHqlExpression * accessor = NULL;
-                transformCtx.associateOwn(*new BoundRow(tableExpr->queryNormalizedSelector(), boundSrc, accessor, translator.queryRecordOffsetMap(physicalRecord, (accessor != NULL)), no_none, NULL));
-            }
+            translator.bindTableCursor(transformCtx, projectedSelector, "left");
         }
         transformCtx.addGroup();
     }
@@ -1278,53 +1266,7 @@ void SourceBuilder::buildTransformElements(BuildCtx & ctx, IHqlExpression * expr
         }
         else
         {
-            if (fieldInfo.hasVirtuals())
-            {
-                IHqlExpression * record = expr->queryRecord();
-                assertex(fieldInfo.simpleVirtualsAtEnd);
-                assertex(!recordRequiresSerialization(record, diskAtom));
-                CHqlBoundExpr bound;
-                StringBuffer s;
-                translator.getRecordSize(ctx, expr, bound);
-
-                size32_t virtualSize = 0;
-                ForEachChild(idx, record)
-                {
-                    IHqlExpression * field = record->queryChild(idx);
-                    IHqlExpression * virtualAttr = field->queryAttribute(virtualAtom);
-                    if (virtualAttr)
-                    {
-                        size32_t fieldSize = field->queryType()->getSize();
-                        assertex(fieldSize != UNKNOWN_LENGTH);
-                        virtualSize += fieldSize;
-                    }
-                }
-
-                if (!isFixedSizeRecord(record))
-                {
-                    OwnedHqlExpr ensureSize = adjustValue(bound.expr, virtualSize);
-                    s.clear().append("crSelf.ensureCapacity(");
-                    translator.generateExprCpp(s, ensureSize).append(", NULL);");
-                    ctx.addQuoted(s);
-                }
-
-                s.clear().append("memcpy(crSelf.row(), left, ");
-                translator.generateExprCpp(s, bound.expr).append(");");
-                ctx.addQuoted(s);
-
-                ForEachChild(idx2, record)
-                {
-                    IHqlExpression * field = record->queryChild(idx2);
-                    IHqlExpression * virtualAttr = field->queryAttribute(virtualAtom);
-                    if (virtualAttr)
-                    {
-                        IHqlExpression * self = rootSelfRow->querySelector();
-                        OwnedHqlExpr target = createSelectExpr(LINK(self), LINK(field));
-                        OwnedHqlExpr value = getVirtualReplacement(field, virtualAttr->queryChild(0), expr);
-                        translator.buildAssign(ctx, target, value);
-                    }
-                }
-            }
+            assertex(!fieldInfo.hasVirtuals());
         }
         break;
     case no_null:
@@ -2453,14 +2395,10 @@ void SourceBuilder::buildGlobalGroupAggregateHelpers(IHqlExpression * expr)
     }
 
     //virtual void processRows(void * self, size32_t srcLen, const void * src) = 0;
+    //Only meaningful for a dataset, and even then I'm not sure it is ever used.
+    if (!isKey(tableExpr))
     {
         OwnedHqlExpr newTableExpr = LINK(tableExpr);
-        if (isKey(tableExpr))
-        {
-            IHqlExpression * record = tableExpr->queryRecord();
-            OwnedHqlExpr newRecord = removeChild(record, record->numChildren()-1);
-            newTableExpr.setown(replaceChild(tableExpr, 1, newRecord));
-        }
 
         MemberFunction func(translator, instance->startctx, "virtual void processRows(size32_t srcLen, const void * _left, IHThorGroupAggregateCallback * callback) override");
         func.ctx.addQuotedLiteral("unsigned char * left = (unsigned char *)_left;");
@@ -2892,12 +2830,12 @@ void DiskReadBuilderBase::buildMembers(IHqlExpression * expr)
     {
         //Spill files can still have virtual attributes in their physical records => remove them.
         OwnedHqlExpr noVirtualRecord = removeVirtualAttributes(expectedRecord);
-        translator.buildFormatCrcFunction(instance->classctx, "getDiskFormatCrc", false, noVirtualRecord, NULL, 0);
+        translator.buildFormatCrcFunction(instance->classctx, "getDiskFormatCrc", noVirtualRecord);
 
         if (newDiskReadMapping)
-            translator.buildFormatCrcFunction(instance->classctx, "getProjectedFormatCrc", false, projectedRecord, NULL, 0);
+            translator.buildFormatCrcFunction(instance->classctx, "getProjectedFormatCrc", projectedRecord);
         else
-            translator.buildFormatCrcFunction(instance->classctx, "getProjectedFormatCrc", false, noVirtualRecord, NULL, 0);
+            translator.buildFormatCrcFunction(instance->classctx, "getProjectedFormatCrc", noVirtualRecord);
     }
 
     translator.buildMetaMember(instance->classctx, expectedRecord, isGrouped(tableExpr), "queryDiskRecordSize");
@@ -3204,8 +3142,7 @@ ABoundActivity * HqlCppTranslator::doBuildActivityDiskRead(BuildCtx & ctx, IHqlE
     }
     else
     {
-        if (!info.fieldInfo.simpleVirtualsAtEnd || info.fieldInfo.requiresDeserialize ||
-            (info.recordHasVirtuals() && (modeOp == no_csv || !isSimpleSource(expr))))
+        if (info.recordHasVirtuals() || info.fieldInfo.requiresDeserialize)
         {
             OwnedHqlExpr transformed = buildTableWithoutVirtuals(info.fieldInfo, expr);
             //Need to wrap a possible no_usertable, otherwise the localisation can go wrong.
@@ -3881,6 +3818,7 @@ void IndexReadBuilderBase::buildMembers(IHqlExpression * expr)
 
     buildKeyedLimitHelper(expr);
 
+    translator.buildFormatCrcFunction(instance->classctx, "getDiskFormatCrc", true, tableExpr, tableExpr, 1);
     translator.buildFormatCrcFunction(instance->classctx, "getProjectedFormatCrc", true, tableExpr, tableExpr, 1);
     IHqlExpression * originalKey = queryAttributeChild(tableExpr, _original_Atom, 0);
     translator.buildSerializedLayoutMember(instance->classctx, originalKey->queryRecord(), "getIndexLayout", numKeyedFields(originalKey));
@@ -4191,24 +4129,8 @@ void IndexAggregateBuilder::buildMembers(IHqlExpression * expr)
         rowctx.addQuotedLiteral("doProcessRow(crSelf, (const byte *)src);");
     }
 
-    {
-        IHqlExpression * record = tableExpr->queryRecord();
-        OwnedHqlExpr newRecord = removeChild(record, record->numChildren()-1);
-        OwnedHqlExpr newTableExpr = replaceChild(tableExpr, 1, newRecord);
-
-        MemberFunction func(translator, instance->startctx, "virtual void processRows(ARowBuilder & crSelf, size32_t srcLen, const void * _left) override");
-        func.ctx.addQuotedLiteral("unsigned char * left = (unsigned char *)_left;");
-        OwnedHqlExpr ds = createVariable("left", makeReferenceModifier(newTableExpr->getType()));
-        OwnedHqlExpr len = createVariable("srcLen", LINK(sizetType));
-        OwnedHqlExpr fullDs = createTranslated(ds, len);
-
-        Owned<IHqlCppDatasetCursor> iter = translator.createDatasetSelector(func.ctx, fullDs);
-        BoundRow * curRow = iter->buildIterateLoop(func.ctx, false);
-        s.clear().append("doProcessRow(crSelf, ");
-        translator.generateExprCpp(s, curRow->queryBound());
-        s.append(");");
-        func.ctx.addQuoted(s);
-    }
+    //virtual void processRows(ARowBuilder & crSelf, size32_t srcLen, const void * _left)
+    //is meaningless for an index - uses the default error implementation in the base class
 }
 
 
@@ -4801,7 +4723,7 @@ void FetchBuilder::buildMembers(IHqlExpression * expr)
     case no_json:
         break;
     default:
-        translator.buildFormatCrcFunction(instance->classctx, "getDiskFormatCrc", false, physicalRecord, NULL, 0);
+        translator.buildFormatCrcFunction(instance->classctx, "getDiskFormatCrc", physicalRecord);
         break;
     }
 

rtl/eclrtl/eclhelper_base.cpp

@@ -637,6 +637,7 @@ unsigned CThorIndexAggregateArg::getFlags() { return 0; }
 bool CThorIndexAggregateArg::getIndexLayout(size32_t & _retLen, void * & _retData) { return false; }
 void CThorIndexAggregateArg::setCallback(IThorIndexCallback * _tc) { fpp = _tc; }
 size32_t CThorIndexAggregateArg::mergeAggregate(ARowBuilder & rowBuilder, const void * src) { rtlFailUnexpected(); return 0; }
+void CThorIndexAggregateArg::processRows(ARowBuilder & rowBuilder, size32_t srcLen, const void * src) { rtlFailUnexpected(); }
 
 //CThorIndexCountArg
 
@@ -663,6 +664,8 @@ unsigned CThorIndexGroupAggregateArg::getGroupingMaxField() { return 0; }
 size32_t CThorIndexGroupAggregateArg::initialiseCountGrouping(ARowBuilder & rowBuilder, const void * src) { rtlFailUnexpected(); return 0; }
 size32_t CThorIndexGroupAggregateArg::processCountGrouping(ARowBuilder & rowBuilder, unsigned __int64 count) { rtlFailUnexpected(); return 0; }
 size32_t CThorIndexGroupAggregateArg::mergeAggregate(ARowBuilder & rowBuilder, const void * src) { rtlFailUnexpected(); return 0; }
+void CThorIndexGroupAggregateArg::processRows(size32_t srcLen, const void * src, IHThorGroupAggregateCallback * callback) { rtlFailUnexpected(); }
+
 
 //CThorDiskReadArg
 

rtl/eclrtl/eclhelper_dyn.cpp

@@ -219,6 +219,10 @@ public:
     {
         return projected;
     }
+    virtual unsigned getDiskFormatCrc() override final
+    {
+        return 0;  // engines should treat 0 as 'ignore'
+    }
     virtual unsigned getProjectedFormatCrc() override final
     {
         return 0;  // engines should treat 0 as 'ignore'

rtl/include/eclhelper.hpp

@@ -2344,7 +2344,7 @@ struct IHThorIndexReadBaseArg : extends IHThorCompoundBaseArg
     virtual IOutputMetaData * queryProjectedDiskRecordSize() = 0;   // Projected layout
     virtual unsigned getFlags() = 0;
     virtual unsigned getProjectedFormatCrc() = 0;                   // Corresponding to projectedDiskRecordSize
-    virtual unsigned getDiskFormatCrc() { return getProjectedFormatCrc(); }  // Should correspond to queryDiskRecordSize() meta really - codegen needs to fix that
+    virtual unsigned getDiskFormatCrc() = 0;                        // Corresponding to diskRecordSize
     virtual void setCallback(IThorIndexCallback * callback) = 0;
     virtual bool getIndexLayout(size32_t & _retLen, void * & _retData) = 0;
 

rtl/include/eclhelper_base.hpp

@@ -853,6 +853,7 @@ class ECLRTL_API CThorIndexAggregateArg : public CThorArgOf<IHThorIndexAggregate
     virtual bool getIndexLayout(size32_t & _retLen, void * & _retData) override;
     virtual void setCallback(IThorIndexCallback * _tc) override;
     virtual size32_t mergeAggregate(ARowBuilder & rowBuilder, const void * src) override;
+    virtual void processRows(ARowBuilder & rowBuilder, size32_t srcLen, const void * src) override;
 
 public:
     IThorIndexCallback * fpp;
@@ -885,6 +886,7 @@ class ECLRTL_API CThorIndexGroupAggregateArg : public CThorArgOf<IHThorIndexGrou
     virtual size32_t initialiseCountGrouping(ARowBuilder & rowBuilder, const void * src) override;
     virtual size32_t processCountGrouping(ARowBuilder & rowBuilder, unsigned __int64 count) override;
     virtual size32_t mergeAggregate(ARowBuilder & rowBuilder, const void * src) override;
+    virtual void processRows(size32_t srcLen, const void * src, IHThorGroupAggregateCallback * callback) override;
 
 public:
     IThorIndexCallback * fpp;