HPCC-34657 Hybrid index improve how fpos are stored

- Moved file positions to end of record
- Optional removed file positions if they will always be zero

Signed-off-by: James McMullan [email protected]

Author: jpmcmu

system/jhtree/hlzw.cpp

@@ -99,7 +99,7 @@ void KeyCompressor::openBlob(void *blk,int blksize)
     method = comp->getCompressionMethod();
 }
 
-int KeyCompressor::writekey(offset_t fPtr, const char *key, unsigned datalength)
+int KeyCompressor::writekey(offset_t fPtr, const char *key, unsigned datalength, unsigned options)
 {
     assert(!isBlob);
     assertex(__BYTE_ORDER == __LITTLE_ENDIAN); // otherwise the following code is wrong.
@@ -111,8 +111,14 @@ int KeyCompressor::writekey(offset_t fPtr, const char *key, unsigned datalength)
         KEYRECSIZE_T rs = datalength;
         tempKeyBuffer.appendSwap(sizeof(rs), &rs);
     }
-    tempKeyBuffer.appendSwap(sizeof(offset_t), &fPtr);
+
+    bool hasTrailingFilePos = (options & TrailingFilePosition) != 0 && (options & NoFilePosition) == 0;
+    bool hasLeadingFilePos = (options & NoFilePosition) == 0 && !hasTrailingFilePos;
+    if (hasLeadingFilePos)
+        tempKeyBuffer.appendSwap(sizeof(offset_t), &fPtr);
     tempKeyBuffer.append(datalength, key);
+    if (hasTrailingFilePos)
+        tempKeyBuffer.appendSwap(sizeof(offset_t), &fPtr);
 
     size32_t toWrite = tempKeyBuffer.length();
     if (comp->write(tempKeyBuffer.bufferBase(),toWrite)!=toWrite)

system/jhtree/hlzw.h

@@ -27,13 +27,20 @@ typedef unsigned short KEYRECSIZE_T;
 class KeyCompressor final
 {
 public:
+    enum KeyWriteOptions 
+    {
+        DefaultWriteOptions= 0x00,
+        TrailingFilePosition = 0x01,
+        NoFilePosition = 0x02
+    };
+
     KeyCompressor() {}
     ~KeyCompressor();
     void open(void *blk,int blksize, bool isVariable, bool rowcompression, size32_t fixedRowSize);
     void open(void *blk,int blksize, ICompressHandler * compressionHandler, const char * options, bool _isVariable, size32_t fixedRowSize);
     void open(void *blk,int blksize, ICompressor * compressor, bool _isVariable, size32_t _fixedRowSize);
 
-    int writekey(offset_t fPtr, const char *key, unsigned datalength);
+    int writekey(offset_t fPtr, const char *key, unsigned datalength, unsigned writeOptions = DefaultWriteOptions);
     bool write(const void * data, size32_t datalength);
 
     bool compressBlock(size32_t destSize, void * dest, size32_t srcSize, const void * src, ICompressHandler * compressionHandler, const char * options, bool isVariable, size32_t fixedSize);

system/jhtree/jhblockcompressed.cpp

@@ -110,8 +110,7 @@ void CJHBlockCompressedSearchNode::load(CKeyHdr *_keyHdr, const void *rawData, o
 
     keyLen = keyHdr->getMaxKeyLength();
     keyCompareLen = _keyHdr->getNodeKeyLength();
-    keyRecLen = keyLen + sizeof(offset_t);
-
+    
     const char *keys = ((const char *) rawData) + sizeof(hdr);
 
     firstSequence = *(unsigned __int64 *) keys;
@@ -120,6 +119,11 @@ void CJHBlockCompressedSearchNode::load(CKeyHdr *_keyHdr, const void *rawData, o
 
     CompressionMethod compressionMethod = *(CompressionMethod*) keys;
     keys += sizeof(CompressionMethod);
+    
+    hasFilePosition = *(bool*) keys;
+    keys += sizeof(bool);
+    
+    keyRecLen = hasFilePosition ? (keyLen + sizeof(offset_t)) : keyLen;
 
     CCycleTimer expansionTimer(true);
     keyBuf = expandBlock(keys, inMemorySize, compressionMethod);
@@ -128,7 +132,7 @@ void CJHBlockCompressedSearchNode::load(CKeyHdr *_keyHdr, const void *rawData, o
 
 int CJHBlockCompressedSearchNode::compareValueAt(const char *src, unsigned int index) const
 {
-    return memcmp(src, keyBuf + index*keyRecLen + (keyHdr->hasSpecialFileposition() ? sizeof(offset_t) : 0), keyCompareLen);
+    return memcmp(src, keyBuf + index*keyRecLen, keyCompareLen);
 }
 
 bool CJHBlockCompressedSearchNode::fetchPayload(unsigned int index, char *dst, PayloadReference & activePayload) const
@@ -139,11 +143,14 @@ bool CJHBlockCompressedSearchNode::fetchPayload(unsigned int index, char *dst, P
     const char * p = keyBuf + index*keyRecLen;
     if (keyHdr->hasSpecialFileposition())
     {
-        //It would make sense to have the fileposition at the start of the row from the perspective of the
-        //internal representation, but that would complicate everything else which assumes the keyed
-        //fields start at the beginning of the row.
-        memcpy(dst+keyCompareLen, p+keyCompareLen+sizeof(offset_t), keyLen-keyCompareLen);
-        memcpy(dst+keyLen, p, sizeof(offset_t));
+        memcpy(dst+keyCompareLen, p+keyCompareLen, keyLen-keyCompareLen);
+        if (hasFilePosition)
+            memcpy(dst+keyLen, p+keyLen, sizeof(offset_t));
+        else
+        {
+            offset_t zeroPos = 0;
+            memcpy(dst+keyLen, &zeroPos, sizeof(offset_t));
+        }
     }
     else
     {
@@ -158,8 +165,6 @@ bool CJHBlockCompressedSearchNode::getKeyAt(unsigned int index, char *dst) const
     if (dst)
     {
         const char * p = keyBuf + index*keyRecLen;
-        if (keyHdr->hasSpecialFileposition())
-            p += sizeof(offset_t);
         memcpy(dst, p, keyCompareLen);
     }
     return true;
@@ -168,17 +173,23 @@ bool CJHBlockCompressedSearchNode::getKeyAt(unsigned int index, char *dst) const
 size32_t CJHBlockCompressedSearchNode::getSizeAt(unsigned int index) const
 {
     if (keyHdr->hasSpecialFileposition())
-        return keyLen + sizeof(offset_t);
+    {
+        if (hasFilePosition)
+            return keyLen + sizeof(offset_t);
+        else
+            return keyLen;
+    }
     else
         return keyLen;
 }
 
 offset_t CJHBlockCompressedSearchNode::getFPosAt(unsigned int index) const
 {
     if (index >= hdr.numKeys) return 0;
+    if (!hasFilePosition) return 0;
 
     offset_t pos;
-    const char * p = keyBuf + index*keyRecLen;
+    const char * p = keyBuf + index*keyRecLen + keyLen;
     memcpy( &pos, p, sizeof(__int64));
     _WINREV(pos);
     return pos;
@@ -252,16 +263,23 @@ bool CBlockCompressedWriteNode::add(offset_t pos, const void *indata, size32_t i
         memcpy(keyPtr, &context.compressionMethod, sizeof(context.compressionMethod));
         keyPtr += sizeof(context.compressionMethod);
         hdr.keyBytes += sizeof(context.compressionMethod);
+        
+        bool hasFilepos = !context.zeroFilePos;
+        memcpy(keyPtr, &hasFilepos, sizeof(bool));
+        keyPtr += sizeof(bool);
+        hdr.keyBytes += sizeof(bool);
 
-        //Adjust the fixed key size to include the fileposition field which is written by writekey.
+        //Adjust the fixed key size to include the fileposition field which is written by writekey
         bool isVariable = keyHdr->isVariable();
-        size32_t fixedKeySize = isVariable ? 0 : keyLen + sizeof(offset_t);
+        size32_t fixedKeySize = isVariable ? 0 : (hasFilepos ? keyLen + sizeof(offset_t) : keyLen);
 
         ICompressHandler * handler = queryCompressHandler(context.compressionMethod);
         compressor.open(keyPtr, maxBytes-hdr.keyBytes, handler, context.compressionOptions, isVariable, fixedKeySize);
     }
 
-    if (0xffff == hdr.numKeys || 0 == compressor.writekey(pos, (const char *)indata, insize))
+    unsigned writeOptions = KeyCompressor::TrailingFilePosition | (context.zeroFilePos ? KeyCompressor::NoFilePosition : 0);
+    int written = compressor.writekey(pos, (const char *)indata, insize, writeOptions);
+    if (0xffff == hdr.numKeys || written == 0)
         return false;
 
     if (insize>keyLen)
@@ -278,7 +296,7 @@ void CBlockCompressedWriteNode::finalize()
 {
     compressor.close();
     if (hdr.numKeys)
-        hdr.keyBytes = compressor.buflen() + sizeof(unsigned __int64) + sizeof(CompressionMethod); // rsequence
+        hdr.keyBytes = compressor.buflen() + sizeof(unsigned __int64) + sizeof(CompressionMethod) + sizeof(bool); // rsequence + compressionMethod + hasFilePosition
 }
 
 BlockCompressedIndexCompressor::BlockCompressedIndexCompressor(unsigned keyedSize, IHThorIndexWriteArg *helper, const char* options)
@@ -310,6 +328,9 @@ BlockCompressedIndexCompressor::BlockCompressedIndexCompressor(unsigned keyedSiz
     context.compressionHandler = queryCompressHandler(compressionMethod);
     if (!context.compressionHandler)
         throw MakeStringException(0, "Unknown compression method %d", (int)compressionMethod);
+    
+    if (helper && (helper->getFlags() & TIWzerofilepos))
+        context.zeroFilePos = true;
 }
 
 CJHBlockCompressedVarNode::CJHBlockCompressedVarNode() {}
@@ -329,13 +350,15 @@ void CJHBlockCompressedVarNode::load(CKeyHdr *_keyHdr, const void *rawData, offs
         recArray[i] = finger + sizeof(KEYRECSIZE_T);
         KEYRECSIZE_T recsize = *(KEYRECSIZE_T *)finger;
         _WINREV(recsize);
-        finger += recsize + sizeof(KEYRECSIZE_T) + sizeof(offset_t);
+        finger += recsize + sizeof(KEYRECSIZE_T);
+        if (hasFilePosition)
+            finger += sizeof(offset_t);
     }
 }
 
 int CJHBlockCompressedVarNode::compareValueAt(const char *src, unsigned int index) const
 {
-    return memcmp(src, recArray[index] + (keyHdr->hasSpecialFileposition() ? sizeof(offset_t) : 0), keyCompareLen);
+    return memcmp(src, recArray[index], keyCompareLen);
 }
 
 bool CJHBlockCompressedVarNode::fetchPayload(unsigned int num, char *dst, PayloadReference & activePayload) const
@@ -349,8 +372,14 @@ bool CJHBlockCompressedVarNode::fetchPayload(unsigned int num, char *dst, Payloa
         _WINREV(reclen);
         if (keyHdr->hasSpecialFileposition())
         {
-            memcpy(dst+keyCompareLen, p+keyCompareLen+sizeof(offset_t), reclen-keyCompareLen);
-            memcpy(dst+reclen, p, sizeof(offset_t));
+            memcpy(dst+keyCompareLen, p+keyCompareLen, reclen-keyCompareLen);
+            if (hasFilePosition)
+                memcpy(dst+reclen, p+reclen, sizeof(offset_t));
+            else
+            {
+                offset_t zeroPos = 0;
+                memcpy(dst+reclen, &zeroPos, sizeof(offset_t));
+            }
         }
         else
             memcpy(dst+keyCompareLen, p+keyCompareLen, reclen-keyCompareLen);
@@ -368,10 +397,7 @@ bool CJHBlockCompressedVarNode::getKeyAt(unsigned int num, char *dst) const
         KEYRECSIZE_T reclen = ((KEYRECSIZE_T *) p)[-1];
         _WINREV(reclen);
         assertex(reclen >= keyCompareLen);
-        if (keyHdr->hasSpecialFileposition())
-            memcpy(dst, p + sizeof(offset_t), keyCompareLen);
-        else
-            memcpy(dst, p, keyCompareLen);
+        memcpy(dst, p, keyCompareLen);
     }
     return true;
 }
@@ -390,10 +416,13 @@ size32_t CJHBlockCompressedVarNode::getSizeAt(unsigned int num) const
 offset_t CJHBlockCompressedVarNode::getFPosAt(unsigned int num) const
 {
     if (num >= hdr.numKeys) return 0;
+    if (!hasFilePosition) return 0;
 
     const char * p = recArray[num];
+    KEYRECSIZE_T reclen = ((KEYRECSIZE_T *) p)[-1];
+    _WINREV(reclen);
     offset_t pos;
-    memcpy( &pos, p, sizeof(__int64) );
+    memcpy( &pos, p + reclen, sizeof(__int64) );
     _WINREV(pos);
     return pos;
 }

system/jhtree/jhblockcompressed.hpp

@@ -33,6 +33,7 @@ class CJHBlockCompressedSearchNode : public CJHSearchNode
     size32_t keyLen = 0;
     size32_t keyCompareLen = 0;
     size32_t keyRecLen = 0;
+    bool hasFilePosition = true;
 
     unsigned __int64 firstSequence = 0;
 
@@ -76,6 +77,7 @@ struct CBlockCompressedBuildContext
     ICompressHandler* compressionHandler = nullptr;
     StringBuffer compressionOptions;
     CompressionMethod compressionMethod = COMPRESS_METHOD_ZSTDS;
+    bool zeroFilePos = false;
 };
 
 class jhtree_decl CBlockCompressedWriteNode : public CWriteNode