From 6c2bec25069e4c3dcf6426baa6b0cea960d26f52 Mon Sep 17 00:00:00 2001
From: Jake Smith <jake.smith@lexisnexisrisk.com>
Date: Wed, 18 Sep 2024 09:37:37 +0100
Subject: [PATCH] HPCC-32469 Add support for non-keyed index filtering

Rowservice index reads with filters on non-keyed fields failed.

Signed-off-by: Jake Smith <jake.smith@lexisnexisrisk.com>
---
 fs/dafsserver/dafsserver.cpp | 12 +++++++-----
 rtl/eclrtl/rtlnewkey.cpp     | 28 ++++++++++++++++++++++++++++
 rtl/eclrtl/rtlnewkey.hpp     |  2 ++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/fs/dafsserver/dafsserver.cpp b/fs/dafsserver/dafsserver.cpp
index c9d5385af52..5e94f8b8c8f 100644
--- a/fs/dafsserver/dafsserver.cpp
+++ b/fs/dafsserver/dafsserver.cpp
@@ -1176,7 +1176,7 @@ class CRemoteDiskBaseActivity : public CSimpleInterfaceOf<IRemoteReadActivity>,
     bool opened = false;
     bool eofSeen = false;
     const RtlRecord *record = nullptr;
-    RowFilter filters;
+    RowFilter filter;
     RtlDynRow *filterRow = nullptr;
     // virtual field values
     StringAttr logicalFilename;
@@ -1186,8 +1186,8 @@ class CRemoteDiskBaseActivity : public CSimpleInterfaceOf<IRemoteReadActivity>,
     {
         if (filterRow)
         {
-            filterRow->setRow(buffer, filters.getNumFieldsRequired());
-            return filters.matches(*filterRow);
+            filterRow->setRow(buffer, filter.getNumFieldsRequired());
+            return filter.matches(*filterRow);
         }
         else
             return true;
@@ -1217,7 +1217,7 @@ class CRemoteDiskBaseActivity : public CSimpleInterfaceOf<IRemoteReadActivity>,
             filterRow = new RtlDynRow(*record);
             Owned<IPropertyTreeIterator> filterIter = config.getElements("keyFilter");
             ForEach(*filterIter)
-                filters.addFilter(*record, filterIter->query().queryProp(nullptr));
+                filter.addFilter(*record, filterIter->query().queryProp(nullptr));
         }
     }
 // IRemoteReadActivity impl.
@@ -2228,6 +2228,7 @@ class CRemoteIndexBaseActivity : public CRemoteDiskBaseActivity
     unsigned fileCrc = 0;
     Owned<IKeyIndex> keyIndex;
     Owned<IKeyManager> keyManager;
+    RowFilter keyFilter;
 
     void checkOpen()
     {
@@ -2243,7 +2244,7 @@ class CRemoteIndexBaseActivity : public CRemoteDiskBaseActivity
 
         keyIndex.setown(createKeyIndex(fileName, crc, isTlk, 0));
         keyManager.setown(createLocalKeyManager(*record, keyIndex, nullptr, true, false));
-        filters.createSegmentMonitors(keyManager);
+        keyFilter.createSegmentMonitors(keyManager);
         keyManager->finishSegmentMonitors();
         keyManager->reset();
 
@@ -2260,6 +2261,7 @@ class CRemoteIndexBaseActivity : public CRemoteDiskBaseActivity
     CRemoteIndexBaseActivity(IPropertyTree &config, IFileDescriptor *fileDesc) : PARENT(config, fileDesc)
     {
         setupInputMeta(config, getTypeInfoOutputMetaData(config, "input", false));
+        filter.splitIntoKeyFilter(*record, keyFilter);
 
         isTlk = config.getPropBool("isTlk");
         fileCrc = config.getPropInt("crc");
diff --git a/rtl/eclrtl/rtlnewkey.cpp b/rtl/eclrtl/rtlnewkey.cpp
index 48af5a62e52..138edccc6b9 100644
--- a/rtl/eclrtl/rtlnewkey.cpp
+++ b/rtl/eclrtl/rtlnewkey.cpp
@@ -2245,6 +2245,27 @@ void RowFilter::extractMemKeyFilter(const RtlRecord & record, const UnsignedArra
     }
 }
 
+void RowFilter::splitIntoKeyFilter(const RtlRecord & record, RowFilter &keyFilter)
+{
+    if (0 == filters.ordinality())
+        return;
+
+    unsigned numKeyedFields = record.getNumKeyedFields();
+    ForEachItemInRev(i, filters)
+    {
+        const IFieldFilter & cur = filters.item(i);
+        if (cur.queryFieldIndex() < numKeyedFields)
+        {
+            keyFilter.addFilter(OLINK(cur));
+            filters.remove(i);
+        }
+    }
+    keyFilter.sortByFieldOrder(); // NB: need to be ordered ahead of keyFilter.createSegmentMonitors being used
+    //There is either a payload filter, in which case numFieldsRequired will not change, or now no filter
+    if (filters.ordinality() == 0)
+        numFieldsRequired = 0;
+}
+
 const IFieldFilter *RowFilter::findFilter(unsigned fieldNum) const
 {
     ForEachItemIn(i, filters)
@@ -2298,6 +2319,13 @@ void RowFilter::remapField(unsigned filterIdx, unsigned newFieldNum)
     filters.replace(*filters.item(filterIdx).remap(newFieldNum), filterIdx);
 }
 
+void RowFilter::sortByFieldOrder()
+{
+    if (0 == filters.ordinality())
+        return;
+    filters.sort(compareFieldFilters);
+}
+
 //---------------------------------------------------------------------------------------------------------------------
 
 bool RowCursor::setRowForward(const byte * row)
diff --git a/rtl/eclrtl/rtlnewkey.hpp b/rtl/eclrtl/rtlnewkey.hpp
index a6aadc9b787..aeccb9fd5a2 100644
--- a/rtl/eclrtl/rtlnewkey.hpp
+++ b/rtl/eclrtl/rtlnewkey.hpp
@@ -65,6 +65,7 @@ class ECLRTL_API RowFilter
     void createSegmentMonitors(IIndexReadContext *irc);
     void extractKeyFilter(const RtlRecord & record, IConstArrayOf<IFieldFilter> & keyFilters) const;
     void extractMemKeyFilter(const RtlRecord & record, const UnsignedArray &sortOrder, IConstArrayOf<IFieldFilter> & keyFilters) const;
+    void splitIntoKeyFilter(const RtlRecord & record, RowFilter &keyFilter);
     unsigned numFilterFields() const { return filters.ordinality(); }
     const IFieldFilter & queryFilter(unsigned i) const { return filters.item(i); }
     const IFieldFilter *findFilter(unsigned fieldIdx) const;
@@ -75,6 +76,7 @@ class ECLRTL_API RowFilter
     void remove(unsigned idx);
     RowFilter & clear();
     void appendFilters(const IConstArrayOf<IFieldFilter> &_filters);
+    void sortByFieldOrder();
 protected:
     IConstArrayOf<IFieldFilter> filters;
     unsigned numFieldsRequired = 0;