Merge pull request #13816 from richardkchapman/hpcc24161

HPCC-24161 COMPRESS(ROW) on variable size indexes fails

Reviewed-by: Gavin Halliday <ghalliday@hpccsystems.com>

system/jhtree/ctfile.cpp

@@ -529,7 +529,7 @@ void *CJHTreeNode::allocMem(size32_t len)
     return ret;
 }
 
-char *CJHTreeNode::expandKeys(void *src,unsigned keylength,size32_t &retsize)
+char *CJHTreeNode::expandKeys(void *src,size32_t &retsize)
 {
     Owned<IExpander> exp = createLZWExpander(true);
     int len=exp->init(src);
@@ -589,10 +589,10 @@ void CJHTreeNode::unpack(const void *node, bool needCopy)
         {
             MTIME_SECTION(queryActiveTimer(), "Compressed node expand");
             expandedSize = keyHdr->getNodeSize();
-            bool quick = !isBlob() && (keyType&HTREE_QUICK_COMPRESSED_KEY)==HTREE_QUICK_COMPRESSED_KEY;
+            bool quick = !isBlob() && (keyType&(HTREE_QUICK_COMPRESSED_KEY|HTREE_VARSIZE))==HTREE_QUICK_COMPRESSED_KEY;
             keyBuf = NULL;
             if (!quick)
-                keyBuf = expandKeys(keys,keyLen,expandedSize);
+                keyBuf = expandKeys(keys,expandedSize);
         }
     }
     else
@@ -755,28 +755,6 @@ bool CJHTreeNode::getValueAt(unsigned int index, char *dst) const
     return true;
 }
 
-const char * CJHTreeNode::queryValueAt(unsigned int index, char *scratchBuffer) const
-{
-    if (index >= hdr.numKeys)
-        return nullptr;
-    else if (keyHdr->hasSpecialFileposition())
-    {
-        getValueAt(index, scratchBuffer);
-        return scratchBuffer;
-    }
-    else
-        return keyBuf + index*keyRecLen;
-}
-
-const char * CJHTreeNode::queryKeyAt(unsigned int index, char *scratchBuffer) const
-{
-    if (index >= hdr.numKeys)
-        return nullptr;
-    else
-        return keyBuf + index*keyRecLen + (keyHdr->hasSpecialFileposition() ? sizeof(offset_t) : 0);
-}
-
-
 size32_t CJHTreeNode::getSizeAt(unsigned int index) const
 {
     if (keyHdr->hasSpecialFileposition())
@@ -934,26 +912,6 @@ bool CJHVarTreeNode::getValueAt(unsigned int num, char *dst) const
     return true;
 }
 
-const char *CJHVarTreeNode::queryValueAt(unsigned int index, char *scratchBuffer) const
-{
-    if (index >= hdr.numKeys)
-        return nullptr;
-    else if (keyHdr->hasSpecialFileposition())
-    {
-        getValueAt(index, scratchBuffer);
-        return scratchBuffer;
-    }
-    else
-        return recArray[index];
-}
-
-const char *CJHVarTreeNode::queryKeyAt(unsigned int index, char *scratchBuffer) const
-{
-    if (index >= hdr.numKeys)
-        return nullptr;
-    return recArray[index] + (keyHdr->hasSpecialFileposition() ? sizeof(offset_t) : 0);
-}
-
 size32_t CJHVarTreeNode::getSizeAt(unsigned int num) const
 {
     const char * p = recArray[num];
@@ -1009,29 +967,6 @@ bool CJHRowCompressedNode::getValueAt(unsigned int num, char *dst) const
     return true;
 }
 
-const char *CJHRowCompressedNode::queryValueAt(unsigned int num, char *scratchBuffer) const
-{
-    if (num >= hdr.numKeys)
-        return nullptr;
-    else
-    {
-        getValueAt(num, scratchBuffer);
-        return scratchBuffer;
-    }
-}
-
-const char *CJHRowCompressedNode::queryKeyAt(unsigned int num, char *scratchBuffer) const
-{
-    if (num >= hdr.numKeys)
-        return nullptr;
-    unsigned keyedSize = keyHdr->getNodeKeyLength();
-    if (keyHdr->hasSpecialFileposition())
-        rowexp->expandRow(scratchBuffer,num,sizeof(offset_t),keyedSize);
-    else
-        rowexp->expandRow(scratchBuffer,num,0,keyedSize);
-    return scratchBuffer;
-}
-
 offset_t CJHRowCompressedNode::getFPosAt(unsigned int num) const
 {
     if (num >= hdr.numKeys)

system/jhtree/ctfile.hpp

@@ -157,7 +157,7 @@ public:
     inline static size32_t getSize() { return sizeof(KeyHdr); }
     inline unsigned getNodeSize() { return hdr.nodeSize; }
     inline bool hasSpecialFileposition() const { return true; }
-    inline bool isRowCompressed() const { return (hdr.ktype & HTREE_QUICK_COMPRESSED_KEY) == HTREE_QUICK_COMPRESSED_KEY; }
+    inline bool isRowCompressed() const { return (hdr.ktype & (HTREE_QUICK_COMPRESSED_KEY|HTREE_VARSIZE)) == HTREE_QUICK_COMPRESSED_KEY; }
     __uint64 getPartitionFieldMask()
     {
         if (hdr.partitionFieldMask == (__uint64) -1)
@@ -217,7 +217,7 @@ protected:
     unsigned __int64 firstSequence;
     size32_t expandedSize;
 
-    static char *expandKeys(void *src,unsigned keylength,size32_t &retsize);
+    static char *expandKeys(void *src,size32_t &retsize);
     static void releaseMem(void *togo, size32_t size);
     static void *allocMem(size32_t size);
 
@@ -231,8 +231,6 @@ public:
     offset_t prevNodeFpos() const;
     offset_t nextNodeFpos() const ;
     virtual bool getValueAt(unsigned int num, char *key) const;
-    virtual const char *queryValueAt(unsigned int index, char *scratchBuffer) const;
-    virtual const char *queryKeyAt(unsigned int index, char *scratchBuffer) const;
     virtual size32_t getSizeAt(unsigned int num) const;
     virtual offset_t getFPosAt(unsigned int num) const;
     virtual int compareValueAt(const char *src, unsigned int index) const;
@@ -251,8 +249,6 @@ public:
     ~CJHVarTreeNode();
     virtual void load(CKeyHdr *keyHdr, const void *rawData, offset_t pos, bool needCopy);
     virtual bool getValueAt(unsigned int num, char *key) const;
-    virtual const char *queryValueAt(unsigned int index, char *scratchBuffer) const;
-    virtual const char *queryKeyAt(unsigned int index, char *scratchBuffer) const;
     virtual size32_t getSizeAt(unsigned int num) const;
     virtual offset_t getFPosAt(unsigned int num) const;
     virtual int compareValueAt(const char *src, unsigned int index) const;
@@ -265,8 +261,6 @@ class CJHRowCompressedNode : public CJHTreeNode
 public:
     virtual void load(CKeyHdr *keyHdr, const void *rawData, offset_t pos, bool needCopy);
     virtual bool getValueAt(unsigned int num, char *key) const;
-    virtual const char *queryValueAt(unsigned int index, char *scratchBuffer) const;
-    virtual const char *queryKeyAt(unsigned int index, char *scratchBuffer) const;
     virtual offset_t getFPosAt(unsigned int num) const;
     virtual int compareValueAt(const char *src, unsigned int index) const;
 };
@@ -277,8 +271,6 @@ public:
     CJHTreeBlobNode ();
     ~CJHTreeBlobNode ();
     virtual bool getValueAt(unsigned int num, char *key) const {throwUnexpected();}
-    virtual const char *queryValueAt(unsigned int index, char *scratchBuffer) const {throwUnexpected();}
-    virtual const char *queryKeyAt(unsigned int index, char *scratchBuffer) const {throwUnexpected();}
     virtual offset_t getFPosAt(unsigned int num) const {throwUnexpected();}
     virtual size32_t getSizeAt(unsigned int num) const {throwUnexpected();}
     virtual int compareValueAt(const char *src, unsigned int index) const {throwUnexpected();}
@@ -292,8 +284,6 @@ class CJHTreeMetadataNode : public CJHTreeNode
 {
 public:
     virtual bool getValueAt(unsigned int num, char *key) const {throwUnexpected();}
-    virtual const char *queryValueAt(unsigned int index, char *scratchBuffer) const {throwUnexpected();}
-    virtual const char *queryKeyAt(unsigned int index, char *scratchBuffer) const {throwUnexpected();}
     virtual offset_t getFPosAt(unsigned int num) const {throwUnexpected();}
     virtual size32_t getSizeAt(unsigned int num) const {throwUnexpected();}
     virtual int compareValueAt(const char *src, unsigned int index) const {throwUnexpected();}
@@ -305,8 +295,6 @@ class CJHTreeBloomTableNode : public CJHTreeNode
 {
 public:
     virtual bool getValueAt(unsigned int num, char *key) const {throwUnexpected();}
-    virtual const char *queryValueAt(unsigned int index, char *scratchBuffer) const {throwUnexpected();}
-    virtual const char *queryKeyAt(unsigned int index, char *scratchBuffer) const {throwUnexpected();}
     virtual offset_t getFPosAt(unsigned int num) const {throwUnexpected();}
     virtual size32_t getSizeAt(unsigned int num) const {throwUnexpected();}
     virtual int compareValueAt(const char *src, unsigned int index) const {throwUnexpected();}

system/jhtree/jhtree.cpp

@@ -3043,7 +3043,7 @@ class IKeyManagerTest : public CppUnit::TestFixture
 
     void testStepping()
     {
-        buildTestKeys(false, true, false);
+        buildTestKeys(false, true, false, false);
         {
             // We are going to treat as a 7-byte field then a 3-byte field, and request the datasorted by the 3-byte...
             Owned <IKeyIndex> index1 = createKeyIndex("keyfile1.$$$", 0, false, false);
@@ -3151,13 +3151,13 @@ class IKeyManagerTest : public CppUnit::TestFixture
         removeTestKeys();
     }
 
-    void buildTestKeys(bool variable, bool useTrailingHeader, bool noSeek)
+    void buildTestKeys(bool variable, bool useTrailingHeader, bool noSeek, bool quickCompressed)
     {
-        buildTestKey("keyfile1.$$$", false, variable, useTrailingHeader, noSeek);
-        buildTestKey("keyfile2.$$$", true, variable, useTrailingHeader, noSeek);
+        buildTestKey("keyfile1.$$$", false, variable, useTrailingHeader, noSeek, quickCompressed);
+        buildTestKey("keyfile2.$$$", true, variable, useTrailingHeader, noSeek, quickCompressed);
     }
 
-    void buildTestKey(const char *filename, bool skip, bool variable, bool useTrailingHeader, bool noSeek)
+    void buildTestKey(const char *filename, bool skip, bool variable, bool useTrailingHeader, bool noSeek, bool quickCompressed)
     {
         OwnedIFile file = createIFile(filename);
         OwnedIFileIO io = file->openShared(IFOcreate, IFSHfull);
@@ -3167,6 +3167,7 @@ class IKeyManagerTest : public CppUnit::TestFixture
         unsigned maxRecSize = variable ? 18 : 10;
         unsigned keyedSize = 10;
         Owned<IKeyBuilder> builder = createKeyBuilder(out, COL_PREFIX | HTREE_FULLSORT_KEY | HTREE_COMPRESSED_KEY |
+                (quickCompressed ? HTREE_QUICK_COMPRESSED_KEY : 0) |
                 (variable ? HTREE_VARSIZE : 0) |
                 (useTrailingHeader ? USE_TRAILING_HEADER : 0) |
                 (noSeek ? TRAILING_HEADER_ONLY : 0),
@@ -3238,7 +3239,7 @@ class IKeyManagerTest : public CppUnit::TestFixture
         key->releaseBlobs();
     }
 protected:
-    void testKeys(bool variable, bool useTrailingHeader, bool noSeek)
+    void testKeys(bool variable, bool useTrailingHeader, bool noSeek, bool quickCompressed)
     {
         const char *json = variable ?
                 "{ \"ty1\": { \"fieldType\": 4, \"length\": 10 }, "
@@ -3258,7 +3259,7 @@ protected:
                 "}";
         Owned<IOutputMetaData> meta = createTypeInfoOutputMetaData(json, false);
         const RtlRecord &recInfo = meta->queryRecordAccessor(true);
-        buildTestKeys(variable, useTrailingHeader, noSeek);
+        buildTestKeys(variable, useTrailingHeader, noSeek, quickCompressed);
         {
             Owned <IKeyIndex> index1 = createKeyIndex("keyfile1.$$$", 0, false, false);
             Owned <IKeyManager> tlk1 = createLocalKeyManager(recInfo, index1, NULL, false, false);
@@ -3424,10 +3425,11 @@ protected:
     void testKeys()
     {
         ASSERT(sizeof(CKeyIdAndPos) == sizeof(unsigned __int64) + sizeof(offset_t));
-        for (bool var : { false, true })
+        for (bool var : { true, false })
             for (bool trail : { false, true })
                 for (bool noseek : { false, true })
-                    testKeys(var, trail, noseek);
+                    for (bool quick : { true, false })
+                        testKeys(var, trail, noseek, quick);
     }
 };
 

system/jhtree/keybuild.cpp

@@ -123,6 +123,8 @@ public:
         assertex(nodeSize <= 0xffff); // stored in a short in the header - we should fix that if/when we restructure header
         if (flags & TRAILING_HEADER_ONLY)
             flags |= USE_TRAILING_HEADER;
+        if ((flags & (HTREE_QUICK_COMPRESSED_KEY|HTREE_VARSIZE)) == (HTREE_QUICK_COMPRESSED_KEY|HTREE_VARSIZE))
+            flags &= ~HTREE_QUICK_COMPRESSED;  // Quick does not support variable-size rows
         KeyHdr *hdr = keyHdr->getHdrStruct();
         hdr->nodeSize = nodeSize;
         hdr->extsiz = 4096;