Merge remote-tracking branch 'origin/closedown-4.2.x'

Conflicts:
	esp/scm/ws_workunits.ecm
	esp/services/ws_workunits/ws_workunitsQuerySets.cpp

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

common/workunit/pkgimpl.hpp

@@ -384,7 +384,7 @@ public:
         Owned<IPropertyTree> query = resolveQueryAlias(querySet, queryname, true);
         if (!query)
             throw MakeStringException(PACKAGE_QUERY_NOT_FOUND, "Query %s not found", queryname);
-        Owned<IReferencedFileList> filelist = createReferencedFileList(NULL, NULL);
+        Owned<IReferencedFileList> filelist = createReferencedFileList(NULL, NULL, true);
         Owned<IWorkUnitFactory> wufactory = getWorkUnitFactory(NULL, NULL);
         Owned<IConstWorkUnit> cw = wufactory->openWorkUnit(query->queryProp("@wuid"), false);
 
@@ -478,7 +478,7 @@ public:
             const char *queryid = queries->query().queryProp("@id");
             if (queryid && *queryid)
             {
-                Owned<IReferencedFileList> filelist = createReferencedFileList(NULL, NULL);
+                Owned<IReferencedFileList> filelist = createReferencedFileList(NULL, NULL, true);
                 Owned<IConstWorkUnit> cw = wufactory->openWorkUnit(queries->query().queryProp("@wuid"), false);
 
                 StringArray libnames, unresolvedLibs;

common/workunit/referencedfilelist.cpp

@@ -36,6 +36,21 @@ bool getIsOpt(const IPropertyTree &graphNode)
         return graphNode.getPropBool("att[@name='_isIndexOpt']/@value", false);
 }
 
+bool checkForeign(const char *lfn)
+{
+    if (*lfn=='~')
+        lfn++;
+    static size_t l = strlen("foreign");
+    if (strncasecmp("foreign", lfn, l)==0)
+    {
+        lfn+=l;
+        while (isspace(*lfn))
+            lfn++;
+        if (lfn[0]==':' && lfn[1]==':')
+            return true;
+    }
+    return false;
+}
 const char *skipForeign(const char *name, StringBuffer *ip)
 {
     if (*name=='~')
@@ -164,7 +179,7 @@ class ReferencedFileList : public CInterface, implements IReferencedFileList
 {
 public:
     IMPLEMENT_IINTERFACE;
-    ReferencedFileList(const char *username, const char *pw)
+    ReferencedFileList(const char *username, const char *pw, bool allowForeignFiles) : allowForeign(allowForeignFiles)
     {
         if (username && pw)
         {
@@ -204,6 +219,7 @@ public:
     StringAttr process;
     StringAttr srcCluster;
     StringAttr remotePrefix;
+    bool allowForeign;
 };
 
 void ReferencedFile::processLocalFileInfo(IDistributedFile *df, const char *dstCluster, const char *srcCluster, StringArray *subfiles)
@@ -461,6 +477,9 @@ public:
 
 void ReferencedFileList::ensureFile(const char *ln, unsigned flags, const char *pkgid, const char *daliip, const char *srcCluster, const char *prefix)
 {
+    if (!allowForeign && checkForeign(ln))
+        throw MakeStringException(-1, "Foreign file not allowed%s: %s", (flags & RefFileInPackage) ? " (declared in package)" : "", ln);
+
     Owned<ReferencedFile> file = new ReferencedFile(ln, daliip, srcCluster, prefix, false, flags, pkgid);
     if (!file->logicalName.length())
         return;
@@ -597,7 +616,11 @@ void ReferencedFileList::resolveSubFiles(StringArray &subfiles, bool checkLocalF
     StringArray childSubFiles;
     ForEachItemIn(i, subfiles)
     {
-        Owned<ReferencedFile> file = new ReferencedFile(subfiles.item(i), NULL, NULL, NULL, true, 0, NULL);
+        const char *lfn = subfiles.item(i);
+        if (!allowForeign && checkForeign(lfn))
+            throw MakeStringException(-1, "Foreign sub file not allowed: %s", lfn);
+
+        Owned<ReferencedFile> file = new ReferencedFile(lfn, NULL, NULL, NULL, true, 0, NULL);
         if (file->logicalName.length() && !map.getValue(file->getLogicalName()))
         {
             file->resolve(process.get(), srcCluster, user, remote, remotePrefix, checkLocalFirst, &childSubFiles, resolveForeign);
@@ -676,7 +699,7 @@ IReferencedFileIterator *ReferencedFileList::getFiles()
     return new ReferencedFileIterator(this);
 }
 
-IReferencedFileList *createReferencedFileList(const char *user, const char *pw)
+IReferencedFileList *createReferencedFileList(const char *user, const char *pw, bool allowForeignFiles)
 {
-    return new ReferencedFileList(user, pw);
+    return new ReferencedFileList(user, pw, allowForeignFiles);
 }

common/workunit/referencedfilelist.hpp

@@ -66,7 +66,7 @@ interface IReferencedFileList : extends IInterface
 
 extern WORKUNIT_API const char *skipForeign(const char *name, StringBuffer *ip=NULL);
 
-extern WORKUNIT_API IReferencedFileList *createReferencedFileList(const char *user, const char *pw);
+extern WORKUNIT_API IReferencedFileList *createReferencedFileList(const char *user, const char *pw, bool allowForeignFiles);
 
 extern WORKUNIT_API void splitDfsLocation(const char *address, StringBuffer &cluster, StringBuffer &ip, StringBuffer &prefix, const char *defaultCluster);
 extern WORKUNIT_API void splitDerivedDfsLocation(const char *address, StringBuffer &cluster, StringBuffer &ip, StringBuffer &prefix, const char *defaultCluster, const char *baseCluster, const char *baseIP, const char *basePrefix);

dali/base/dafdesc.cpp

@@ -385,7 +385,7 @@ struct CClusterInfo: public CInterface, implements IClusterInfo
                     }
                     if (mspec.defaultCopies>1 && mspec.defaultReplicateDir.isEmpty())
                     {
-                        mspec.setDefaultReplicateDir(queryBaseDirectory(groupType, 1));
+                        mspec.setDefaultReplicateDir(queryBaseDirectory(groupType, 1));  // MORE - not sure this is strictly correct
                     }
                     return; // ok
                 }
@@ -430,10 +430,14 @@ public:
             StringBuffer defaultDir;
             GroupType groupType;
             group.setown(resolver->lookup(name.get(), defaultDir, groupType));
+            // MORE - common some of this with checkClusterName?
             if (mspec.defaultBaseDir.isEmpty())
             {
                 mspec.setDefaultBaseDir(defaultDir);   // MORE - should possibly set up the rest of the mspec info from the group info here
-                // MORE - work out why this code pulled out of checkClusterName
+            }
+            if (mspec.defaultCopies>1 && mspec.defaultReplicateDir.isEmpty())
+            {
+                mspec.setDefaultReplicateDir(queryBaseDirectory(groupType, 1));  // MORE - not sure this is strictly correct
             }
         }
         else

ecl/ecl-package/ecl-package.cpp

@@ -498,7 +498,7 @@ private:
 class EclCmdPackageAdd : public EclCmdCommon
 {
 public:
-    EclCmdPackageAdd() : optActivate(false), optOverWrite(false), optGlobalScope(false)
+    EclCmdPackageAdd() : optActivate(false), optOverWrite(false), optGlobalScope(false), optAllowForeign(false)
     {
     }
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
@@ -537,6 +537,8 @@ public:
                 continue;
             if (iter.matchFlag(optGlobalScope, ECLOPT_GLOBAL_SCOPE))
                 continue;
+            if (iter.matchFlag(optAllowForeign, ECLOPT_ALLOW_FOREIGN))
+                continue;
             if (EclCmdCommon::matchCommandLineOption(iter, true)!=EclCmdOptionMatch)
                 return false;
         }
@@ -592,6 +594,7 @@ public:
         request->setOverWrite(optOverWrite);
         request->setGlobalScope(optGlobalScope);
         request->setSourceProcess(optSourceProcess);
+        request->setAllowForeignFiles(optAllowForeign);
 
         Owned<IClientAddPackageResponse> resp = packageProcessClient->AddPackage(request);
         if (resp->getExceptions().ordinality())
@@ -623,6 +626,7 @@ public:
                     "   --pmid                      Identifier of package map - defaults to filename if not specified\n"
                     "   --global-scope              The specified packagemap can be shared across multiple targets\n"
                     "   --source-process            Process cluster to copy files from\n"
+                    "   --allow-foreign             Do not fail if foreign files are used in packagemap\n"
                     "   <target>                    Name of target to use when adding package map information\n"
                     "   <filename>                  Name of file containing package map information\n",
                     stdout);
@@ -640,6 +644,7 @@ private:
     bool optActivate;
     bool optOverWrite;
     bool optGlobalScope;
+    bool optAllowForeign;
 };
 
 class EclCmdPackageValidate : public EclCmdCommon

ecl/eclcmd/eclcmd_common.hpp

@@ -65,6 +65,7 @@ typedef IEclCommand *(*EclCommandFactory)(const char *cmdname);
 #define ECLOPT_OVERWRITE_ENV NULL
 
 #define ECLOPT_DONT_COPY_FILES "--no-files"
+#define ECLOPT_ALLOW_FOREIGN "--allow-foreign"
 
 #define ECLOPT_ACTIVE "--active"
 #define ECLOPT_ALL "--all"

ecl/eclcmd/eclcmd_core.cpp

@@ -195,7 +195,7 @@ class EclCmdPublish : public EclCmdWithEclTarget
 {
 public:
     EclCmdPublish() : optNoActivate(false), optSuspendPrevious(false), optDeletePrevious(false),
-        activateSet(false), optNoReload(false), optDontCopyFiles(false), optMsToWait(10000)
+        activateSet(false), optNoReload(false), optDontCopyFiles(false), optMsToWait(10000), optAllowForeign(false)
     {
         optObj.accept = eclObjWuid | eclObjArchive | eclObjSharedObject;
         optTimeLimit = (unsigned) -1;
@@ -233,6 +233,8 @@ public:
                 continue;
             if (iter.matchFlag(optDontCopyFiles, ECLOPT_DONT_COPY_FILES))
                 continue;
+            if (iter.matchFlag(optAllowForeign, ECLOPT_ALLOW_FOREIGN))
+                continue;
             if (iter.matchFlag(optNoActivate, ECLOPT_NO_ACTIVATE))
             {
                 activateSet=true;
@@ -325,6 +327,7 @@ public:
         req->setWait(optMsToWait);
         req->setNoReload(optNoReload);
         req->setDontCopyFiles(optDontCopyFiles);
+        req->setAllowForeignFiles(optAllowForeign);
 
         if (optTimeLimit != (unsigned) -1)
             req->setTimeLimit(optTimeLimit);
@@ -381,6 +384,7 @@ public:
             "   -A-, --no-activate     Do not activate query when published\n"
             "   --no-reload            Do not request a reload of the (roxie) cluster\n"
             "   --no-files             Do not copy files referenced by query\n"
+            "   --allow-foreign        Do not fail if foreign files are used in query (roxie)\n"
             "   --daliip=<IP>          The IP of the DALI to be used to locate remote files\n"
             "   --source-process       Process cluster to copy files from\n"
             "   --timeLimit=<ms>       Value to set for query timeLimit configuration\n"
@@ -410,6 +414,7 @@ private:
     bool optDontCopyFiles;
     bool optSuspendPrevious;
     bool optDeletePrevious;
+    bool optAllowForeign;
 };
 
 class EclCmdRun : public EclCmdWithEclTarget

ecl/eclcmd/queries/ecl-queries.cpp

@@ -292,7 +292,7 @@ private:
 class EclCmdQueriesCopy : public EclCmdCommon
 {
 public:
-    EclCmdQueriesCopy() : optActivate(false), optNoReload(false), optMsToWait(10000), optDontCopyFiles(false), optOverwrite(false)
+    EclCmdQueriesCopy() : optActivate(false), optNoReload(false), optMsToWait(10000), optDontCopyFiles(false), optOverwrite(false), optAllowForeign(false)
     {
         optTimeLimit = (unsigned) -1;
         optWarnTimeLimit = (unsigned) -1;
@@ -335,6 +335,8 @@ public:
                 continue;
             if (iter.matchFlag(optDontCopyFiles, ECLOPT_DONT_COPY_FILES))
                 continue;
+            if (iter.matchFlag(optAllowForeign, ECLOPT_ALLOW_FOREIGN))
+                continue;
             if (iter.matchOption(optMsToWait, ECLOPT_WAIT))
                 continue;
             if (iter.matchOption(optTimeLimit, ECLOPT_TIME_LIMIT))
@@ -393,6 +395,7 @@ public:
         req->setDontCopyFiles(optDontCopyFiles);
         req->setWait(optMsToWait);
         req->setNoReload(optNoReload);
+        req->setAllowForeignFiles(optAllowForeign);
 
         if (optTimeLimit != (unsigned) -1)
             req->setTimeLimit(optTimeLimit);
@@ -440,6 +443,7 @@ public:
             "   -A, --activate         Activate the new query\n"
             "   --no-reload            Do not request a reload of the (roxie) cluster\n"
             "   -O, --overwrite        Overwrite existing files\n"
+            "   --allow-foreign        Do not fail if foreign files are used in query (roxie)\n"
             "   --wait=<ms>            Max time to wait in milliseconds\n"
             "   --timeLimit=<sec>      Value to set for query timeLimit configuration\n"
             "   --warnTimeLimit=<sec>  Value to set for query warnTimeLimit configuration\n"
@@ -470,6 +474,7 @@ private:
     bool optNoReload;
     bool optOverwrite;
     bool optDontCopyFiles;
+    bool optAllowForeign;
 };
 
 class EclCmdQueriesConfig : public EclCmdCommon

esp/scm/ws_packageprocess.ecm

@@ -36,6 +36,7 @@ ESPrequest AddPackageRequest
     string DaliIp;
     bool GlobalScope(0);
     string SourceProcess;
+    bool AllowForeignFiles(true);
 };
 
 

esp/scm/ws_workunits.ecm

@@ -1109,6 +1109,7 @@ ESPrequest [nil_remove] WUPublishWorkunitRequest
     string Comment;
     bool DontCopyFiles(false);
     string SourceProcess;
+    bool AllowForeignFiles(true);
 };
 
 ESPresponse [exceptions_inline] WUPublishWorkunitResponse
@@ -1410,6 +1411,7 @@ ESPrequest [nil_remove] WUQuerySetCopyQueryRequest
     string Comment;
     string SourceProcess;
     string DestName;
+    bool AllowForeignFiles(true);
 };
 
 ESPresponse [exceptions_inline] WUQuerySetCopyQueryResponse

esp/services/ws_packageprocess/ws_packageprocessService.cpp

@@ -123,7 +123,7 @@ bool isFileKnownOnCluster(const char *logicalname, const char *target, IUserDesc
     return isFileKnownOnCluster(logicalname, clusterInfo, userdesc);
 }
 
-void cloneFileInfoToDali(StringArray &notFound, IPropertyTree *packageMap, const char *lookupDaliIp, IConstWUClusterInfo *dstInfo, const char *srcCluster, const char *remotePrefix, bool overWrite, IUserDescriptor* userdesc)
+void cloneFileInfoToDali(StringArray &notFound, IPropertyTree *packageMap, const char *lookupDaliIp, IConstWUClusterInfo *dstInfo, const char *srcCluster, const char *remotePrefix, bool overWrite, IUserDescriptor* userdesc, bool allowForeignFiles)
 {
     StringBuffer user;
     StringBuffer password;
@@ -134,7 +134,7 @@ void cloneFileInfoToDali(StringArray &notFound, IPropertyTree *packageMap, const
         userdesc->getPassword(password);
     }
 
-    Owned<IReferencedFileList> wufiles = createReferencedFileList(user, password);
+    Owned<IReferencedFileList> wufiles = createReferencedFileList(user, password, allowForeignFiles);
     wufiles->addFilesFromPackageMap(packageMap);
     SCMStringBuffer processName;
     dstInfo->getRoxieProcess(processName);
@@ -151,13 +151,13 @@ void cloneFileInfoToDali(StringArray &notFound, IPropertyTree *packageMap, const
     }
 }
 
-void cloneFileInfoToDali(StringArray &notFound, IPropertyTree *packageMap, const char *lookupDaliIp, const char *dstCluster, const char *srcCluster, const char *prefix, bool overWrite, IUserDescriptor* userdesc)
+void cloneFileInfoToDali(StringArray &notFound, IPropertyTree *packageMap, const char *lookupDaliIp, const char *dstCluster, const char *srcCluster, const char *prefix, bool overWrite, IUserDescriptor* userdesc, bool allowForeignFiles)
 {
     Owned<IConstWUClusterInfo> clusterInfo = getTargetClusterInfo(dstCluster);
     if (!clusterInfo)
         throw MakeStringException(PKG_TARGET_NOT_DEFINED, "Could not find information about target cluster %s ", dstCluster);
 
-    cloneFileInfoToDali(notFound, packageMap, lookupDaliIp, clusterInfo, srcCluster, prefix, overWrite, userdesc);
+    cloneFileInfoToDali(notFound, packageMap, lookupDaliIp, clusterInfo, srcCluster, prefix, overWrite, userdesc, allowForeignFiles);
 }
 
 
@@ -175,7 +175,7 @@ void makePackageActive(IPropertyTree *pkgSetRegistry, IPropertyTree *pkgSetTree,
 
 //////////////////////////////////////////////////////////
 
-void addPackageMapInfo(StringArray &filesNotFound, IPropertyTree *pkgSetRegistry, const char *target, const char *pmid, const char *packageSetName, const char *lookupDaliIp, const char *srcCluster, const char *prefix, IPropertyTree *packageInfo, bool activate, bool overWrite, IUserDescriptor* userdesc)
+void addPackageMapInfo(StringArray &filesNotFound, IPropertyTree *pkgSetRegistry, const char *target, const char *pmid, const char *packageSetName, const char *lookupDaliIp, const char *srcCluster, const char *prefix, IPropertyTree *packageInfo, bool activate, bool overWrite, IUserDescriptor* userdesc, bool allowForeignFiles)
 {
     if (srcCluster && *srcCluster)
     {
@@ -251,7 +251,7 @@ void addPackageMapInfo(StringArray &filesNotFound, IPropertyTree *pkgSetRegistry
     }
 
     mergePTree(mapTree, baseInfo);
-    cloneFileInfoToDali(filesNotFound, mapTree, lookupDaliIp, clusterInfo, srcCluster, prefix, overWrite, userdesc);
+    cloneFileInfoToDali(filesNotFound, mapTree, lookupDaliIp, clusterInfo, srcCluster, prefix, overWrite, userdesc, allowForeignFiles);
 
     globalLock->commit();
 
@@ -547,7 +547,7 @@ bool CWsPackageProcessEx::onAddPackage(IEspContext &context, IEspAddPackageReque
     StringArray filesNotFound;
     StringBuffer pkgSetId;
     buildPkgSetId(pkgSetId, processName.get());
-    addPackageMapInfo(filesNotFound, pkgSetRegistry, target.get(), pmid.str(), pkgSetId.str(), daliip.str(), srcCluster.str(), prefix.str(), LINK(packageTree), activate, overWrite, userdesc);
+    addPackageMapInfo(filesNotFound, pkgSetRegistry, target.get(), pmid.str(), pkgSetId.str(), daliip.str(), srcCluster.str(), prefix.str(), LINK(packageTree), activate, overWrite, userdesc, req.getAllowForeignFiles());
     resp.setFilesNotFound(filesNotFound);
 
     StringBuffer msg;
@@ -784,7 +784,7 @@ bool CWsPackageProcessEx::onValidatePackage(IEspContext &context, IEspValidatePa
 
     if (req.getCheckDFS())
     {
-        Owned<IReferencedFileList> pmfiles = createReferencedFileList(context.queryUserId(), context.queryPassword());
+        Owned<IReferencedFileList> pmfiles = createReferencedFileList(context.queryUserId(), context.queryPassword(), true);
         pmfiles->addFilesFromPackageMap(mapTree);
         pmfiles->resolveFiles(process.str(), NULL, NULL, NULL, true, false);
         Owned<IReferencedFileIterator> files = pmfiles->getFiles();

esp/services/ws_workunits/ws_workunitsQuerySets.cpp

@@ -434,7 +434,7 @@ static inline void updateQueryPriority(IPropertyTree *queryTree, const char *val
     }
 }
 
-void copyQueryFilesToCluster(IEspContext &context, IConstWorkUnit *cw, const char *remoteIP, const char *remotePrefix, const char *target, const char *srcCluster, const char *queryname, bool overwrite)
+void copyQueryFilesToCluster(IEspContext &context, IConstWorkUnit *cw, const char *remoteIP, const char *remotePrefix, const char *target, const char *srcCluster, const char *queryname, bool overwrite, bool allowForeignFiles)
 {
     if (!target || !*target)
         return;
@@ -446,7 +446,7 @@ void copyQueryFilesToCluster(IEspContext &context, IConstWorkUnit *cw, const cha
         clusterInfo->getRoxieProcess(process);
         if (!process.length())
             return;
-        Owned<IReferencedFileList> wufiles = createReferencedFileList(context.queryUserId(), context.queryPassword());
+        Owned<IReferencedFileList> wufiles = createReferencedFileList(context.queryUserId(), context.queryPassword(), allowForeignFiles);
         Owned<IHpccPackageSet> ps = createPackageSet(process.str());
         StringBuffer queryid;
         if (queryname && *queryname)
@@ -540,7 +540,7 @@ bool CWsWorkunitsEx::onWUPublishWorkunit(IEspContext &context, IEspWUPublishWork
     }
 
     if (!req.getDontCopyFiles())
-        copyQueryFilesToCluster(context, cw, daliIP, srcPrefix, target.str(), srcCluster, queryName.str(), false);
+        copyQueryFilesToCluster(context, cw, daliIP, srcPrefix, target.str(), srcCluster, queryName.str(), false, req.getAllowForeignFiles());
 
     WorkunitUpdate wu(&cw->lock());
     if (req.getUpdateWorkUnitName() && notEmpty(req.getJobName()))
@@ -1596,7 +1596,7 @@ bool CWsWorkunitsEx::onWUQuerysetCopyQuery(IEspContext &context, IEspWUQuerySetC
         StringBuffer srcCluster;
         StringBuffer srcPrefix;
         splitDerivedDfsLocation(req.getDaliServer(), srcCluster, daliIP, srcPrefix, req.getSourceProcess(), req.getSourceProcess(), remoteIP.str(), NULL);
-        copyQueryFilesToCluster(context, cw, daliIP.str(), srcPrefix, target, srcCluster, targetQueryName.get(), req.getOverwrite());
+        copyQueryFilesToCluster(context, cw, daliIP.str(), srcPrefix, target, srcCluster, targetQueryName.get(), req.getOverwrite(), req.getAllowForeignFiles());
     }
 
     WorkunitUpdate wu(&cw->lock());

initfiles/etc/bash_completion/ecl

@@ -89,7 +89,7 @@ _ecl_opts_queries()
             echo "--help list copy config"
             ;;
         copy)
-            echo -n "--no-reload --wait= --timeLimit= --warnTimeLimit= --memoryLimit= --priority= --daliip= "
+            echo -n "--no-reload --allow-foreign --wait= --timeLimit= --warnTimeLimit= --memoryLimit= --priority= --daliip= "
             _ecl_opts_common
             ;;
         config)
@@ -136,7 +136,7 @@ _ecl_opts_packagemap()
             then
                 _ecl_opts_file
             else
-                echo -n "-O --overwrite "
+                echo -n "-O --overwrite --allow-foreign "
                 _ecl_opts_common
             fi
             ;;
@@ -166,7 +166,7 @@ _ecl_opts_core_file()
                 _ecl_opts_common
                 ;;
             publish)
-                echo -n "-A --activate --no-reload --timeLimit= --warnTimeLimit= --memoryLimit= --priority= --daliip= "
+                echo -n "-A --activate --no-reload --allow-foreign --timeLimit= --warnTimeLimit= --memoryLimit= --priority= --daliip= "
                 _ecl_opts_deploy
                 _ecl_opts_common
                 ;;

rtl/eclrtl/eclrtl.cpp

@@ -787,12 +787,19 @@ double rtlUnicodeToReal(size32_t l, UChar const * t)
 
 //---------------------------------------------------------------------------
 
-void rtlRealToStr(size32_t l, char * t, double val)
+static void truncFixedReal(size32_t l, char * t, StringBuffer & temp)
 {
-    StringBuffer temp;
-    temp.append(val);
+    const char * str = temp.str();
     unsigned len = temp.length();
     if (len > l)
+    {
+        //If we don't lose significant digits left of the decimal point then truncate the string.
+        const char * dot = strchr(str, '.');
+        if (dot && ((size_t)(dot - str) <= l))
+            len = l;
+    }
+
+    if (len > l)
         memset(t,'*',l);
     else
     {
@@ -801,12 +808,53 @@ void rtlRealToStr(size32_t l, char * t, double val)
     }
 }
 
-void rtlRealToStr(size32_t l, char * t, float val)
+static void roundFixedReal(size32_t l, char * t, StringBuffer & temp)
 {
-    StringBuffer temp;
-    temp.append(val);
+    const char * str = temp.str();
     unsigned len = temp.length();
     if (len > l)
+    {
+        //If we don't lose significant digits left of the decimal point then truncate the string.
+        const char * dot = strchr(str, '.');
+        if (dot && ((size_t)(dot - str) <= l))
+        {
+            len = l;
+            //Unfortunately we now need to potentially round the number which could even lead to
+            //an extra digit, and failure to fit.  Is there a simpler way of handling this?
+            bool decimalIsNext = ((dot - str) == l);
+            char next = decimalIsNext ? dot[1] : str[len];
+            bool rounding = (next >= '5');
+            unsigned cur = len;
+            while ((cur > 0) && rounding)
+            {
+                next = str[cur-1];
+                if (next == '-')
+                    break;
+                if (next != '.')
+                {
+                    if (next != '9')
+                    {
+                        temp.setCharAt(cur-1, next+1);
+                        rounding = false;
+                        break;
+                    }
+                    else
+                        temp.setCharAt(cur-1, '0');
+                }
+                cur--;
+            }
+            if (rounding)
+            {
+                //Ugly, but it is an exceptional case.
+                if (!decimalIsNext)
+                    temp.insert(cur, '1');
+                else
+                    len++; // overflow
+            }
+        }
+    }
+
+    if (len > l)
         memset(t,'*',l);
     else
     {
@@ -815,6 +863,26 @@ void rtlRealToStr(size32_t l, char * t, float val)
     }
 }
 
+void rtlRealToStr(size32_t l, char * t, double val)
+{
+    StringBuffer temp;
+    temp.append(val);
+
+    //This could either truncate or round when converting a real to a string
+    //Rounding is more user friendly, but then (string3)(string)1.99 != (string3)1.99 which is
+    //rather count intuitive.  (That is still true if the value is out of range.)
+    truncFixedReal(l, t, temp);
+}
+
+void rtlRealToStr(size32_t l, char * t, float val)
+{
+    StringBuffer temp;
+    temp.append(val);
+
+    //See comment above
+    truncFixedReal(l, t, temp);
+}
+
 void rtlRealToStrX(size32_t & l, char * & t, double val)
 {
     StringBuffer temp;

testing/regress/ecl/issue10882.ecl

@@ -0,0 +1,61 @@
+/*##############################################################################
+
+    HPCC SYSTEMS software Copyright (C) 2014 HPCC Systems.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+############################################################################## */
+
+
+value := 12345.678901234;
+
+output(value);
+output((string)value);
+output((string4)value);
+output((string4)nofold(value));
+output((string5)value);
+output((string5)nofold(value));
+output((string6)value);
+output((string6)nofold(value));
+output((string7)value);
+output((string7)nofold(value));
+output((string8)value);
+output((string8)nofold(value));
+output((string20)value);
+output((string20)nofold(value));
+
+value2 := 0.01456;
+
+output((string1)value2);
+output((string2)value2);
+output((string3)value2);
+output((string4)value2);
+output((string5)value2);
+output((string6)value2);
+output((string7)value2);
+
+output((string5)1.9994);
+output((string5)1.9995);
+output((string5)9.9995);
+output((string5)9.99951);
+
+output((string5)-1.994);
+output((string5)-1.995);
+output((string5)-9.995);
+output((string5)-9.995001);
+
+output((string5)-199.4);
+output((string5)-199.6);
+output((string5)-1999.4);
+output((string5)-1999.6);
+output((string5)-9999.4);
+output((string5)-9999.6);

testing/regress/ecl/key/issue10882.xml

@@ -0,0 +1,105 @@
+<Dataset name='Result 1'>
+ <Row><Result_1>12345.678901234</Result_1></Row>
+</Dataset>
+<Dataset name='Result 2'>
+ <Row><Result_2>12345.678901234</Result_2></Row>
+</Dataset>
+<Dataset name='Result 3'>
+ <Row><Result_3>****</Result_3></Row>
+</Dataset>
+<Dataset name='Result 4'>
+ <Row><Result_4>****</Result_4></Row>
+</Dataset>
+<Dataset name='Result 5'>
+ <Row><Result_5>12345</Result_5></Row>
+</Dataset>
+<Dataset name='Result 6'>
+ <Row><Result_6>12345</Result_6></Row>
+</Dataset>
+<Dataset name='Result 7'>
+ <Row><Result_7>12345.</Result_7></Row>
+</Dataset>
+<Dataset name='Result 8'>
+ <Row><Result_8>12345.</Result_8></Row>
+</Dataset>
+<Dataset name='Result 9'>
+ <Row><Result_9>12345.6</Result_9></Row>
+</Dataset>
+<Dataset name='Result 10'>
+ <Row><Result_10>12345.6</Result_10></Row>
+</Dataset>
+<Dataset name='Result 11'>
+ <Row><Result_11>12345.67</Result_11></Row>
+</Dataset>
+<Dataset name='Result 12'>
+ <Row><Result_12>12345.67</Result_12></Row>
+</Dataset>
+<Dataset name='Result 13'>
+ <Row><Result_13>12345.678901234     </Result_13></Row>
+</Dataset>
+<Dataset name='Result 14'>
+ <Row><Result_14>12345.678901234     </Result_14></Row>
+</Dataset>
+<Dataset name='Result 15'>
+ <Row><Result_15>0</Result_15></Row>
+</Dataset>
+<Dataset name='Result 16'>
+ <Row><Result_16>0.</Result_16></Row>
+</Dataset>
+<Dataset name='Result 17'>
+ <Row><Result_17>0.0</Result_17></Row>
+</Dataset>
+<Dataset name='Result 18'>
+ <Row><Result_18>0.01</Result_18></Row>
+</Dataset>
+<Dataset name='Result 19'>
+ <Row><Result_19>0.014</Result_19></Row>
+</Dataset>
+<Dataset name='Result 20'>
+ <Row><Result_20>0.0145</Result_20></Row>
+</Dataset>
+<Dataset name='Result 21'>
+ <Row><Result_21>0.01456</Result_21></Row>
+</Dataset>
+<Dataset name='Result 22'>
+ <Row><Result_22>1.999</Result_22></Row>
+</Dataset>
+<Dataset name='Result 23'>
+ <Row><Result_23>1.999</Result_23></Row>
+</Dataset>
+<Dataset name='Result 24'>
+ <Row><Result_24>9.999</Result_24></Row>
+</Dataset>
+<Dataset name='Result 25'>
+ <Row><Result_25>9.999</Result_25></Row>
+</Dataset>
+<Dataset name='Result 26'>
+ <Row><Result_26>-1.99</Result_26></Row>
+</Dataset>
+<Dataset name='Result 27'>
+ <Row><Result_27>-1.99</Result_27></Row>
+</Dataset>
+<Dataset name='Result 28'>
+ <Row><Result_28>-9.99</Result_28></Row>
+</Dataset>
+<Dataset name='Result 29'>
+ <Row><Result_29>-9.99</Result_29></Row>
+</Dataset>
+<Dataset name='Result 30'>
+ <Row><Result_30>-199.</Result_30></Row>
+</Dataset>
+<Dataset name='Result 31'>
+ <Row><Result_31>-199.</Result_31></Row>
+</Dataset>
+<Dataset name='Result 32'>
+ <Row><Result_32>-1999</Result_32></Row>
+</Dataset>
+<Dataset name='Result 33'>
+ <Row><Result_33>-1999</Result_33></Row>
+</Dataset>
+<Dataset name='Result 34'>
+ <Row><Result_34>-9999</Result_34></Row>
+</Dataset>
+<Dataset name='Result 35'>
+ <Row><Result_35>-9999</Result_35></Row>
+</Dataset>

thorlcr/activities/lookupjoin/thlookupjoinslave.cpp

@@ -1942,6 +1942,14 @@ class CLookupHT : public CHTBase
         }
         return NULL;
     }
+    void releaseHTRows()
+    {
+        for (rowidx_t r=0; r<htSize; r++)
+        {
+            if (ht[r])
+                ReleaseThorRow(ht[r]);
+        }
+    }
 public:
     CLookupHT()
     {
@@ -1949,11 +1957,7 @@ public:
     }
     ~CLookupHT()
     {
-        for (rowidx_t r=0; r<htSize; r++)
-        {
-            if (ht[r])
-                ReleaseThorRow(ht[r]);
-        }
+        releaseHTRows();
     }
     void setup(CLookupJoinActivityBase<CLookupHT> *_activity, rowidx_t size, IHash *leftHash, IHash *rightHash, ICompare *compareLeftRight)
     {
@@ -1966,6 +1970,7 @@ public:
     }
     void reset()
     {
+        releaseHTRows();
         CHTBase::reset();
         ht = NULL;
     }

thorlcr/thorutil/thmem.cpp

@@ -259,17 +259,18 @@ class CSharedSpillableRowSet : public CSpillableStreamBase, implements IInterfac
             if (spillStream)
                 return spillStream->nextRow();
             CThorArrayLockBlock block(owner->rows);
-            if (pos == owner->rows.numCommitted())
-                return NULL;
-            else if (owner->spillFile) // i.e. has spilt
+            if (owner->spillFile) // i.e. has spilt
             {
                 assertex(((offset_t)-1) != outputOffset);
                 unsigned rwFlags = DEFAULT_RWFLAGS;
                 if (owner->preserveNulls)
                     rwFlags |= rw_grouped;
                 spillStream.setown(::createRowStreamEx(owner->spillFile, owner->rowIf, outputOffset, (offset_t)-1, (unsigned __int64)-1, rwFlags));
+                owner->rows.unregisterWriteCallback(*this); // no longer needed
                 return spillStream->nextRow();
             }
+            else if (pos == owner->rows.numCommitted())
+                return NULL;
             return owner->rows.get(pos++);
         }
         virtual void stop() { }
@@ -1170,6 +1171,16 @@ void CThorSpillableRowArray::sort(ICompare &compare, unsigned maxCores)
     }
 }
 
+static int callbackSortRev(IInterface **cb2, IInterface **cb1)
+{
+    rowidx_t i2 = ((IWritePosCallback *)(*cb2))->queryRecordNumber();
+    rowidx_t i1 = ((IWritePosCallback *)(*cb1))->queryRecordNumber();
+
+    if (i1==i2) return 0;
+    if (i1<i2) return -1;
+    return 1;
+}
+
 rowidx_t CThorSpillableRowArray::save(IFile &iFile, bool useCompression)
 {
     rowidx_t n = numCommitted();
@@ -1185,24 +1196,43 @@ rowidx_t CThorSpillableRowArray::save(IFile &iFile, bool useCompression)
         rwFlags |= rw_compress;
     if (allowNulls)
         rwFlags |= rw_grouped;
-    Owned<IExtRowWriter> writer = createRowWriter(&iFile, rowIf, rwFlags);
 
+    // NB: This is always called within a CThorArrayLockBlock, as such no writebacks are added or updating
+    rowidx_t nextCBI = RCIDXMAX; // indicates none
+    IWritePosCallback *nextCB = NULL;
+    ICopyArrayOf<IWritePosCallback> cbCopy;
+    if (writeCallbacks.ordinality())
+    {
+        ForEachItemIn(c, writeCallbacks)
+            cbCopy.append(writeCallbacks.item(c));
+        cbCopy.sort(callbackSortRev);
+        nextCB = &cbCopy.pop();
+        nextCBI = nextCB->queryRecordNumber();
+    }
+    Owned<IExtRowWriter> writer = createRowWriter(&iFile, rowIf, rwFlags);
     const void **rows = getBlock(n);
     for (rowidx_t i=0; i < n; i++)
     {
         const void *row = rows[i];
         assertex(row || allowNulls);
-        writer->putRow(row);
-        rows[i] = NULL;
-        ForEachItemIn(c, writeCallbacks)
+        if (i == nextCBI)
         {
-            IWritePosCallback &callback = writeCallbacks.item(c);
-            if (i == callback.queryRecordNumber())
+            writer->flush();
+            do
             {
-                writer->flush();
-                callback.filePosition(writer->getPosition());
+                nextCB->filePosition(writer->getPosition());
+                if (cbCopy.ordinality())
+                {
+                    nextCB = &cbCopy.pop();
+                    nextCBI = nextCB->queryRecordNumber();
+                }
+                else
+                    nextCBI = RCIDXMAX; // indicating no more
             }
+            while (i == nextCBI); // loop as may be >1 IWritePosCallback at same pos
         }
+        writer->putRow(row);
+        rows[i] = NULL;
     }
     writer->flush();
     offset_t bytesWritten = writer->getPosition();