Merge pull request #15386 from jakesmith/HPCC-26563-subdirpart-fix

HPCC-26563 Fix sub dir per parts issues

Reviewed-By: Gavin Halliday <gavin.halliday@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

common/thorhelper/roxiehelper.cpp

@@ -2399,7 +2399,7 @@ void ClusterWriteHandler::getPhysicalName(StringBuffer & name, const char * clus
 {
     Owned<IStoragePlane> plane = getDataStoragePlane(cluster, false);
     const char * prefix = plane ? plane->queryPrefix() : nullptr;
-    makePhysicalPartName(logicalName.get(), 1, 1, name, 0, DFD_OSdefault, prefix);
+    makePhysicalPartName(logicalName.get(), 1, 1, name, 0, DFD_OSdefault, prefix, false);
 }
 
 void ClusterWriteHandler::addCluster(char const * cluster)

dali/base/dadfs.cpp

@@ -4255,16 +4255,19 @@ public:
             myBase = newbasedir;
         }
         else
+        {
             myBase = queryBaseDirectory(grp_unknown, 0, os);
+            diroverride = myBase;
+        }
 
         StringBuffer baseDir, newPath;
-        makePhysicalPartName(logicalName.get(), 0, 0, newPath, false, os, diroverride);
+        getLFNDirectoryUsingBaseDir(newPath, logicalName.get(), diroverride);
         if (!getBase(directory, newPath, baseDir))
             baseDir.append(myBase); // getBase returns false, if directory==newPath, so have common base
         getPartMask(newmask,newname,width);
         if (newmask.length()==0)
             return false;
-        makePhysicalPartName(newname, 0, 0, newPath.clear(), false, os, diroverride);
+        getLFNDirectoryUsingBaseDir(newPath.clear(), newname, diroverride);
         if (newPath.length()==0)
             return false;
         if (isPathSepChar(newPath.charAt(newPath.length()-1)))
@@ -4278,7 +4281,7 @@ public:
             newNames.append(*new CIStringArray);
             CDistributedFilePart &part = parts.item(i);
             for (unsigned copy=0; copy<part.numCopies(); copy++) {
-                makePhysicalPartName(newname, i+1, width, newPath.clear(), false, os, myBase);
+                makePhysicalPartName(newname, i+1, width, newPath.clear(), 0, os, myBase, hasDirPerPart());
                 newPath.remove(0, strlen(myBase));
 
                 StringBuffer copyDir(baseDir);

dali/base/dafdesc.cpp

@@ -2325,13 +2325,13 @@ IFileDescriptor *createFileDescriptor(const char *lname,IGroup *grp,IPropertyTre
     IFileDescriptor *res = createFileDescriptor(tree);
     res->setTraceName(lname);
     StringBuffer dir;
-    makePhysicalPartName(lname, 0, 0, dir,false,os);
+    getLFNDirectoryUsingDefaultBaseDir(dir, lname, os);
     res->setDefaultDir(dir.str());
     if (width==0)
         width = grp->ordinality();
     StringBuffer s;
     for (unsigned i=0;i<width;i++) {
-        makePhysicalPartName(lname, i+1, width, s.clear(),false,os);
+        makePhysicalPartName(lname, i+1, width, s.clear(), 0, os, nullptr, false);
         RemoteFilename rfn;
         rfn.setPath(grp->queryNode(i%grp->ordinality()).endpoint(),s.str());
         res->setPart(i,rfn,NULL);
@@ -2350,9 +2350,9 @@ IFileDescriptor *createFileDescriptor(const char *lname, const char *clusterType
 
     StringBuffer curDir, defaultDir;
     if (!getConfigurationDirectory(nullptr, "data", clusterType, groupName, defaultDir))
-        makePhysicalPartName(lname, 0, 0, curDir, false, DFD_OSdefault); // legacy
+        getLFNDirectoryUsingDefaultBaseDir(curDir, lname, DFD_OSdefault); // legacy
     else
-        makePhysicalPartName(lname, 0, 0, curDir, false, SepCharBaseOs(getPathSepChar(defaultDir)), defaultDir.str());
+        getLFNDirectoryUsingBaseDir(curDir, lname, defaultDir.str());
 
     Owned<IFileDescriptor> fileDesc = createFileDescriptor();
     fileDesc->setNumParts(parts);
@@ -2374,7 +2374,7 @@ IFileDescriptor *createFileDescriptor(const char *lname, const char *planeName,
 
     StringBuffer partMask, dir;
     getPartMask(partMask, lname, numParts);
-    makePhysicalPartName(lname, 0, 0, dir, false, DFD_OSdefault, plane->queryPrefix());
+    getLFNDirectoryUsingBaseDir(dir, lname, plane->queryPrefix());
 
     Owned<IFileDescriptor> fileDesc = createFileDescriptor();
     fileDesc->setNumParts(numParts);
@@ -2650,9 +2650,30 @@ inline const char *skipRoot(const char *lname)
     return lname;
 }
 
+// returns position in result buffer of tail lfn name
+static size32_t translateLFNToPath(StringBuffer &result, const char *lname, char pathSep)
+{
+    lname = skipRoot(lname);
+    char c;
+    size32_t l = result.length();
+    while ((c=*(lname++))!=0)
+    {
+        if ((c==':')&&(*lname==':'))
+        {
+            lname++;
+            result.clip().append(pathSep);
+            l = result.length();
+            lname = skipRoot(lname);
+        }
+        else if (validFNameChar(c))
+            result.append((char)tolower(c));
+        else
+            result.appendf("%%%.2X", (int) c);
+    }
+    return l;
+}
 
-
-StringBuffer &makePhysicalPartName(const char *lname, unsigned partno, unsigned partmax, StringBuffer &result, unsigned replicateLevel, DFD_OS os,const char *diroverride)
+StringBuffer &makePhysicalPartName(const char *lname, unsigned partno, unsigned partmax, StringBuffer &result, unsigned replicateLevel, DFD_OS os,const char *diroverride,bool dirPerPart)
 {
     assertex(lname);
     if (strstr(lname,"::>")) { // probably query
@@ -2695,24 +2716,13 @@ StringBuffer &makePhysicalPartName(const char *lname, unsigned partno, unsigned
         l++;
     }
 
-    lname = skipRoot(lname);
+    l = translateLFNToPath(result, lname, OsSepChar(os));
+
     char c;
-    while ((c=*(lname++))!=0) {
-        if ((c==':')&&(*lname==':')) {
-            lname++;
-            result.clip().append(OsSepChar(os));
-            l = result.length();
-            lname = skipRoot(lname);
-        }
-        else if (validFNameChar(c))
-            result.append((char)tolower(c));
-        else
-            result.appendf("%%%.2X", (int) c);
-    }
-    if (partno==0) { // just return directory (with trailing PATHSEP)
+    if (partno==0) // just return directory (with trailing PATHSEP)
         result.setLength(l);
-    }
-    else {
+    else
+    {
 #ifndef INCLUDE_1_OF_1
         if (partmax>1)  // avoid 1_of_1
 #endif
@@ -2720,6 +2730,8 @@ StringBuffer &makePhysicalPartName(const char *lname, unsigned partno, unsigned
             StringBuffer tail(result.str()+l);
             tail.trim();
             result.setLength(l);
+            if (dirPerPart && (partmax>1))
+                result.append(partno).append(OsSepChar(os));
             const char *m = queryPartMask();
             while ((c=*(m++))!=0) {
                 if (c=='$') {
@@ -2752,10 +2764,36 @@ StringBuffer &makeSinglePhysicalPartName(const char *lname, StringBuffer &result
 {
     wasdfs = !(allowospath&&(isAbsolutePath(lname)||(stdIoHandle(lname)>=0)));
     if (wasdfs)
-        return makePhysicalPartName(lname, 1, 1, result, false, DFD_OSdefault,diroverride);
+        return makePhysicalPartName(lname, 1, 1, result, 0, DFD_OSdefault, diroverride, false);
     return result.append(lname);
 }
 
+StringBuffer &getLFNDirectoryUsingBaseDir(StringBuffer &result, const char *lname, const char *baseDir)
+{
+    assertex(lname);
+    if (isEmptyString(baseDir))
+        baseDir = queryBaseDirectory(grp_unknown, 0, DFD_OSdefault);
+
+    result.append(baseDir);
+    char pathSep = getPathSepChar(baseDir);
+    size32_t l = result.length();
+    if ((l>3) && (pathSep != result.charAt(l-1)))
+    {
+        result.append(pathSep);
+        l++;
+    }
+
+    l = translateLFNToPath(result, lname, pathSep);
+    result.setLength(l);
+    return result.clip();
+}
+
+StringBuffer &getLFNDirectoryUsingDefaultBaseDir(StringBuffer &result, const char *lname, DFD_OS os)
+{
+    const char *baseDir = queryBaseDirectory(grp_unknown, 0, os);
+    return getLFNDirectoryUsingBaseDir(result, lname, baseDir);
+}
+
 bool setReplicateDir(const char *dir,StringBuffer &out,bool isrep,const char *baseDir,const char *repDir)
 {
     // assumes that dir contains a separator (like base)

dali/base/dafdesc.hpp

@@ -322,15 +322,19 @@ extern da_decl StringBuffer &makePhysicalPartName(
                                 unsigned partno,                    // part number (1..)
                                 unsigned partmax,                   // number of parts (1..)
                                 StringBuffer &result,               // result filename (or directory name if part 0)
-                                unsigned replicateLevel = 0,       // uses replication directory
-                                DFD_OS os=DFD_OSdefault,            // os must be specified if no dir specified
-                                const char *diroverride=NULL);      // override default directory
+                                unsigned replicateLevel,            // uses replication directory
+                                DFD_OS os,                          // os must be specified if no dir specified
+                                const char *diroverride,            // override default directory
+                                bool dirPerPart);                   // generate a subdirectory per part
 extern da_decl StringBuffer &makeSinglePhysicalPartName(const char *lname, // single part file
                                                         StringBuffer &result,
                                                         bool allowospath,   // allow an OS (absolute) file path
                                                         bool &wasdfs,       // not OS path
                                                         const char *diroverride=NULL
                                                         );
+// 
+extern da_decl StringBuffer &getLFNDirectoryUsingBaseDir(StringBuffer &result, const char *lname, const char *baseDir);
+extern da_decl StringBuffer &getLFNDirectoryUsingDefaultBaseDir(StringBuffer &result, const char *lname, DFD_OS os);
 
 // set/get defaults
 extern da_decl const char *queryBaseDirectory(GroupType groupType, unsigned replicateLevel=0, DFD_OS os=DFD_OSdefault);

dali/base/dautils.cpp

@@ -3262,7 +3262,7 @@ public:
 #endif
             // MORE - should we create the IDistributedFile here ready for publishing (and/or to make sure it's locked while we write)?
             StringBuffer physicalPath;
-            makePhysicalPartName(lfn.get(), 1, 1, physicalPath, false, DFD_OSdefault, dir); // more - may need to override path for roxie
+            makePhysicalPartName(lfn.get(), 1, 1, physicalPath, 0, DFD_OSdefault, dir, false); // more - may need to override path for roxie
             localpath.set(physicalPath);
             fileExists = (dfile != NULL);
             return write;

dali/datest/datest.cpp

@@ -74,7 +74,7 @@ static void addTestFile(const char *name,unsigned n)
     Owned<IPropertyTree> fileInfo = createPTree();
     Owned<IFileDescriptor> fileDesc = createFileDescriptor();
     StringBuffer dir;
-    makePhysicalPartName(name, 0, 0, dir, false, DFD_OSdefault);
+    getLFNDirectoryUsingDefaultBaseDir(dir, name, DFD_OSdefault);
     StringBuffer partmask;
     getPartMask(partmask,name,n);
     StringBuffer path;

dali/datest/dfuwutest.cpp

@@ -435,7 +435,7 @@ IFileDescriptor *createRoxieFileDescriptor(const char *cluster, const char *lfn,
             UERRLOG("dataDirectory not specified");
             return NULL;
         }
-        makePhysicalPartName(lfn,i+1,width,filename,false,DFD_OSdefault,dir);
+        makePhysicalPartName(lfn,i+1,width,filename,0,DFD_OSdefault,dir,false);
         RemoteFilename rfn;
         rfn.setPath(grp->queryNode(i).endpoint(),filename.str());
         ret->setPart(i,rfn,NULL);

dali/dfu/dfuutil.cpp

@@ -476,10 +476,9 @@ public:
         dstfdesc->setPartMask(dstpartmask.str());
         unsigned np = srcfdesc->numParts();
         dstfdesc->setNumParts(srcfdesc->numParts());
-        DFD_OS os = (getPathSepChar(srcfdesc->queryDefaultDir())=='\\')?DFD_OSwindows:DFD_OSunix;
         StringBuffer dir;
         StringBuffer dstdir;
-        makePhysicalPartName(dstlfn.get(),0,0,dstdir,false,os,spec.defaultBaseDir.get());
+        getLFNDirectoryUsingBaseDir(dstdir, dstlfn.get(), spec.defaultBaseDir.get());
         dstfdesc->setDefaultDir(dstdir.str());
         dstfdesc->addCluster(cluster1,grp1,spec);
         if (iskey&&!cluster2.isEmpty())

dali/dfu/dfuwu.cpp

@@ -875,7 +875,7 @@ public:
                 getClusterPartDefaultBaseDir(NULL,baseoverride);
                 bool iswin;
                 getWindowsOS(iswin); // sets os
-                makePhysicalPartName(lfn.get(),0,0,str,false,os,baseoverride.str());
+                getLFNDirectoryUsingBaseDir(str, lfn.get(), baseoverride.str());
             }
         }
         return str;

ecl/eclagent/eclagent.cpp

@@ -3184,7 +3184,7 @@ char *EclAgent::getFilePart(const char *lfn, bool create)
         expandLogicalName(full_lfn, lfn);
 
         StringBuffer physical;
-        makePhysicalPartName(full_lfn.str(), 1, 1, physical, false);//MORE: What do we do if local ?
+        makePhysicalPartName(full_lfn.str(), 1, 1, physical, 0, DFD_OSdefault, nullptr, false);//MORE: What do we do if local ?
 
         StringBuffer dir,base;
         splitFilename(physical.str(), &dir, &dir, &base, &base);

esp/services/ws_fs/ws_fsService.cpp

@@ -577,7 +577,7 @@ bool CFileSprayEx::ParseLogicalPath(const char * pLogicalPath, const char* group
 #endif
     }
 
-    makePhysicalPartName(pLogicalPath,0,0,folder,false,os,defaultFolder.str());
+    getLFNDirectoryUsingBaseDir(folder, pLogicalPath, defaultFolder.str());
 
     const char *n = pLogicalPath;
     const char* p;

roxie/ccd/ccd.hpp

@@ -359,6 +359,7 @@ extern StringBuffer roxieName;
 #ifdef _CONTAINERIZED
 extern StringBuffer defaultPlane;
 extern StringBuffer defaultPlaneDirPrefix;
+extern bool defaultPlaneDirPerPart;
 #endif
 extern bool trapTooManyActiveQueries;
 extern unsigned maxEmptyLoopIterations;

roxie/ccd/ccddali.cpp

@@ -387,8 +387,7 @@ private:
                     // Due to the really weird code in dadfs, this MUST be set to match the leading portion of cloneFromDir
                     // in order to properly handle remote systems with different default directory locations
                     StringBuffer tail;
-                    DFD_OS os = (getPathSepChar(srcfdesc->queryDefaultDir())=='\\')?DFD_OSwindows:DFD_OSunix;
-                    makePhysicalPartName(dstlfn.get(),0,0,tail,0,os,PATHSEPSTR);  // if lfn is a::b::c, tail will be /a/b/
+                    getLFNDirectoryUsingBaseDir(tail, dstlfn.get(), PATHSEPSTR); // if lfn is a::b::c, tail will be /a/b/
                     assertex(tail.length() > 1);
                     tail.setLength(tail.length()-1);   // strip off the trailing /
                     StringBuffer head(srcfdesc->queryProperties().queryProp("@cloneFromDir")); // Will end with /a/b

roxie/ccd/ccdfile.cpp

@@ -1710,10 +1710,12 @@ public:
                 dlfn.clearForeign();
 #ifdef _CONTAINERIZED
             const char *defaultDir = defaultPlaneDirPrefix;
+            bool defaultDirPerPart = defaultPlaneDirPerPart;
 #else
             const char *defaultDir = nullptr;
+            bool defaultDirPerPart = false;
 #endif
-            makePhysicalPartName(dlfn.get(), partNo, numParts, localLocation, replicationLevel, DFD_OSdefault, defaultDir);
+            makePhysicalPartName(dlfn.get(), partNo, numParts, localLocation, replicationLevel, DFD_OSdefault, defaultDir, defaultDirPerPart);
         }
         Owned<ILazyFileIO> ret;
         try

roxie/ccd/ccdmain.cpp

@@ -180,6 +180,7 @@ StringBuffer roxieName;
 #ifdef _CONTAINERIZED
 StringBuffer defaultPlane;
 StringBuffer defaultPlaneDirPrefix;
+bool defaultPlaneDirPerPart = false;
 #endif
 bool trapTooManyActiveQueries;
 unsigned maxEmptyLoopIterations;
@@ -724,6 +725,7 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
         {
             Owned<IStoragePlane> plane = getDataStoragePlane(defaultPlane, true);
             defaultPlaneDirPrefix.set(plane->queryPrefix());
+            defaultPlaneDirPerPart = plane->queryDirPerPart();
         }
 #endif
         installDefaultFileHooks(topology);

thorlcr/mfilemanager/thmfilemanager.cpp

@@ -477,6 +477,7 @@ public:
             desc->queryProperties().setProp("@job", jobStr.str());
             desc->queryProperties().setProp("@owner", userStr.str());
 
+#ifndef _CONTAINERIZED
             // if supporting different OS's in CLUSTER this should be checked where addCluster called
             DFD_OS os = DFD_OSdefault;
             EnvMachineOS thisOs = queryOS(groups.item(0).queryNode(0).endpoint());
@@ -492,7 +493,7 @@ public:
                 default:
                     break;
             };
-
+#endif
             unsigned offset = 0;
             unsigned total;
             if (restrictedWidth)
@@ -522,7 +523,7 @@ public:
                     }
 #else
                     if (!getConfigurationDirectory(globals->queryPropTree("Directories"), "data", "thor", groupNames.item(gn), thisPlaneDir))
-                        makePhysicalPartName(logicalName, 0, 0, thisPlaneDir, 0, os); // legacy
+                        getLFNDirectoryUsingDefaultBaseDir(thisPlaneDir, logicalName, os); // legacy
 #endif
                     if (!planeDir.length()) // 1st output plane
                     {
@@ -539,11 +540,11 @@ public:
                     }
                 }
                 // places logical filename directory in 'dir'
-                makePhysicalPartName(logicalName, 0, 0, dir, false, os, planeDir.str());
+                getLFNDirectoryUsingBaseDir(dir, logicalName, planeDir.str());
             }
             desc->setDefaultDir(dir.str());
 
-            if (job.getOptBool("subDirPerFilePart", dirPerPart))
+            if (job.getOptBool("subDirPerFilePart", dirPerPart) && total>1)
                 desc->queryProperties().setPropInt("@flags", static_cast<int>(FileDescriptorFlags::dirperpart));
 
             StringBuffer partmask;