no PerformQuery() with ScSortedRangeCache if not needed (tdf#144777)

It should not be necessary in that case, since the rows are restricted
only to those that match, selected by BinarySearch(). Still at least
assert that, just in case I'm missing something (or BinarySearch()
doesn't quite match what validQuery() does).

Change-Id: Ie91d3ba997692e5b3650b1549e8a38b6c5f44c01
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134130
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

2 files changed, 45 insertions, 13 deletions

diff --git a/sc/inc/queryiter.hxx b/sc/inc/queryiter.hxx
index 7c454f784a2e..fa8f08083981 100644
--- a/sc/inc/queryiter.hxx
+++ b/sc/inc/queryiter.hxx
@@ -94,12 +94,15 @@ class ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache
 {
 public:
     void SetSortedRangeCache( const ScSortedRangeCache& cache );
+    template<bool fast>
+    bool IncPosImpl();
 protected:
     ScQueryCellIteratorAccessSpecific( ScDocument& rDocument, ScInterpreterContext& rContext,
         const ScQueryParam& rParam );
     void InitPosStart();
     void InitPosFinish( SCROW beforeRow, SCROW lastRow );
-    void IncPos();
+    void IncPos() { IncPosImpl<false>(); }
+    bool IncPosFast() { return IncPosImpl<true>(); }
     void IncBlock() { IncPos(); } // Cannot skip entire block, not linear.
 
     // These members needs to be available already in the base class.
@@ -111,6 +114,7 @@ protected:
     SCTAB           nTab;
     SCCOL           nCol;
     SCROW           nRow;
+    const ScColumn* pColumn; // matching nCol, set by InitPos()
 
     const ScSortedRangeCache* sortedCache;
     size_t sortedCachePos;
diff --git a/sc/source/core/data/queryiter.cxx b/sc/source/core/data/queryiter.cxx
index 8a7bf2ce6e5b..656fedc3c717 100644
--- a/sc/source/core/data/queryiter.cxx
+++ b/sc/source/core/data/queryiter.cxx
@@ -1083,7 +1083,7 @@ void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >
 void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >::InitPosFinish(
     SCROW beforeRow, SCROW lastRow )
 {
-    const ScColumn& rCol = rDoc.maTabs[nTab]->CreateColumnIfNotExists(nCol);
+    pColumn = &rDoc.maTabs[nTab]->CreateColumnIfNotExists(nCol);
     if(lastRow >= 0)
     {
         sortedCachePos = beforeRow >= 0 ? sortedCache->indexForRow(beforeRow) + 1 : 0;
@@ -1091,35 +1091,43 @@ void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >
         if(sortedCachePos <= sortedCachePosLast)
         {
             nRow = sortedCache->rowForIndex(sortedCachePos);
-            maCurPos = rCol.maCells.position(nRow);
+            maCurPos = pColumn->maCells.position(nRow);
             return;
         }
     }
     // No rows, set to end.
     sortedCachePos = sortedCachePosLast = 0;
-    maCurPos.first = rCol.maCells.end();
+    maCurPos.first = pColumn->maCells.end();
     maCurPos.second = 0;
 }
 
-void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >::IncPos()
+template<bool fast>
+bool ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >::IncPosImpl()
 {
-    const ScColumn& rCol = rDoc.maTabs[nTab]->aCol[nCol];
     if(sortedCachePos < sortedCachePosLast)
     {
         ++sortedCachePos;
         nRow = sortedCache->rowForIndex(sortedCachePos);
-        // Avoid mdds position() call if row is in the same block.
-        if(maCurPos.first != rCol.maCells.end() && o3tl::make_unsigned(nRow) >= maCurPos.first->position
-            && o3tl::make_unsigned(nRow) < maCurPos.first->position + maCurPos.first->size)
-            maCurPos.second = nRow - maCurPos.first->position;
-        else
-            maCurPos = rCol.maCells.position(nRow);
+#ifndef DBG_UTIL
+        if constexpr (!fast)
+#endif
+        {
+            // Avoid mdds position() call if row is in the same block.
+            if(maCurPos.first != pColumn->maCells.end() && o3tl::make_unsigned(nRow) >= maCurPos.first->position
+                && o3tl::make_unsigned(nRow) < maCurPos.first->position + maCurPos.first->size)
+                maCurPos.second = nRow - maCurPos.first->position;
+            else
+                maCurPos = pColumn->maCells.position(nRow);
+        }
+        return true;
     }
     else
     {
         // This will make PerformQuery() go to next column.
-        maCurPos.first = rCol.maCells.end();
+        // Necessary even in fast mode, as GetNext() will call GetThis() in this case.
+        maCurPos.first = pColumn->maCells.end();
         maCurPos.second = 0;
+        return false;
     }
 }
 
@@ -1281,6 +1289,26 @@ bool ScQueryCellIterator< accessType >::GetNext()
     return GetThis();
 }
 
+template<>
+bool ScQueryCellIterator< ScQueryCellIteratorAccess::SortedCache >::GetNext()
+{
+    assert( !nStopOnMismatch );
+    assert( !nTestEqualCondition );
+    // When searching using sorted cache, we should always find cells that match,
+    // because InitPos()/IncPos() select only such rows, so skip GetThis() (and thus
+    // the somewhat expensive PerformQuery) as long as we're not at the end
+    // of a column. As an optimization IncPosFast() returns true if not at the end,
+    // in which case in non-DBG_UTIL mode it doesn't even bother to set maCurPos.
+    if( IncPosFast())
+    {
+#ifdef DBG_UTIL
+        assert(GetThis());
+#endif
+        return true;
+    }
+    return GetThis();
+}
+
 bool ScQueryCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const ScQueryParam& rParam,
     const ScFormulaCell* cell, const ScComplexRefData* refData)
 {