Merge pull request #5635 from jakesmith/hpcc-11138

HPCC-11138 - Fix TopN binaryInsert regression

Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

thorlcr/activities/topn/thtopnslave.cpp

@@ -130,10 +130,7 @@ public:
             {
                 const void *lastRow = sortedRows.query(topNLimit-1);
                 if (compare->docompare(lastRow, row) > 0)
-                {
-                    sortedRows.binaryInsert(row.getClear(), *compare);
-                    OwnedConstThorRow rowToDelete = sortedRows.getClear(topNLimit); // Nth+1, fall out now free.
-                }
+                    sortedRows.binaryInsert(row.getClear(), *compare, true);
                 else // had enough and out of range
                     ;
             }

thorlcr/thorutil/thmem.cpp

@@ -735,7 +735,7 @@ bool CThorExpandingRowArray::appendRows(CThorSpillableRowArray &inRows, bool tak
     return true;
 }
 
-bool CThorExpandingRowArray::binaryInsert(const void *row, ICompare &compare)
+bool CThorExpandingRowArray::binaryInsert(const void *row, ICompare &compare, bool dropLast)
 {
     dbgassertex(NULL != row);
     if (numRows >= maxRows)
@@ -743,7 +743,15 @@ bool CThorExpandingRowArray::binaryInsert(const void *row, ICompare &compare)
         if (!ensure(numRows+1))
             return false;
     }
-    binary_vec_insert_stable(row, rows, numRows++, compare); // takes ownership of row
+    binary_vec_insert_stable(row, rows, numRows, compare); // takes ownership of row
+    if (dropLast)
+    {
+    	// last row falls out, i.e. release last row and don't increment numRows
+    	dbgassertex(numRows); // numRows must be >=1 for dropLast
+    	ReleaseThorRow(rows[numRows]);
+    }
+    else
+    	++numRows;
     return true;
 }
 

thorlcr/thorutil/thmem.hpp

@@ -317,7 +317,7 @@ public:
         rows[numRows++] = row;
         return true;
     }
-    bool binaryInsert(const void *row, ICompare &compare); // NB: takes ownership on success
+    bool binaryInsert(const void *row, ICompare &compare, bool dropLast=false); // NB: takes ownership on success
     inline const void *query(rowidx_t i) const
     {
         if (i>=numRows)