fix ByValue lookups with ScSortedRangeCache

My fix for tdf#149071 actually disabled the optimization for all
ByValue lookups, because in fact all such lookups have maString set.
So lookups where the cells are a mix of numeric and string values
need different handling. A simple solution is detecting such a mix
when collecting the values for ScSortedRangeCache and disabling
the optimization in such a case. But it turns out that queries
containing such a mix are not that rare, as documents may e.g.
do COUNTIF($C:$C) where the given column has numeric values that
start with a textual header. So bail out only if the string cell
actually could affect the numeric query.
Also fix ScSortedRangeCache usage depending on query parameters,
different instances are needed for e.g. different ScQueryOp,
because the ScQueryEvaluator functions may return different
results (isQueryByString() is automatically true for SC_EQUAL).

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

4 files changed, 104 insertions, 67 deletions

diff --git a/sc/source/core/data/queryevaluator.cxx b/sc/source/core/data/queryevaluator.cxx
index 27968cbf22b3..b5ff8a354c84 100644
--- a/sc/source/core/data/queryevaluator.cxx
+++ b/sc/source/core/data/queryevaluator.cxx
@@ -35,9 +35,9 @@
 #include <svl/zformat.hxx>
 #include <unotools/collatorwrapper.hxx>
 
-bool ScQueryEvaluator::isPartialTextMatchOp(const ScQueryEntry& rEntry)
+bool ScQueryEvaluator::isPartialTextMatchOp(ScQueryOp eOp)
 {
-    switch (rEntry.eOp)
+    switch (eOp)
     {
         // these operators can only be used with textural comparisons.
         case SC_CONTAINS:
@@ -52,12 +52,12 @@ bool ScQueryEvaluator::isPartialTextMatchOp(const ScQueryEntry& rEntry)
     return false;
 }
 
-bool ScQueryEvaluator::isTextMatchOp(const ScQueryEntry& rEntry)
+bool ScQueryEvaluator::isTextMatchOp(ScQueryOp eOp)
 {
-    if (isPartialTextMatchOp(rEntry))
+    if (isPartialTextMatchOp(eOp))
         return true;
 
-    switch (rEntry.eOp)
+    switch (eOp)
     {
         // these operators can be used for either textural or value comparison.
         case SC_EQUAL:
@@ -68,23 +68,23 @@ bool ScQueryEvaluator::isTextMatchOp(const ScQueryEntry& rEntry)
     return false;
 }
 
-bool ScQueryEvaluator::isMatchWholeCellHelper(bool docMatchWholeCell, const ScQueryEntry& rEntry)
+bool ScQueryEvaluator::isMatchWholeCellHelper(bool docMatchWholeCell, ScQueryOp eOp)
 {
     bool bMatchWholeCell = docMatchWholeCell;
-    if (isPartialTextMatchOp(rEntry))
+    if (isPartialTextMatchOp(eOp))
         // may have to do partial textural comparison.
         bMatchWholeCell = false;
     return bMatchWholeCell;
 }
 
-bool ScQueryEvaluator::isMatchWholeCell(const ScQueryEntry& rEntry) const
+bool ScQueryEvaluator::isMatchWholeCell(ScQueryOp eOp) const
 {
-    return isMatchWholeCellHelper(mbMatchWholeCell, rEntry);
+    return isMatchWholeCellHelper(mbMatchWholeCell, eOp);
 }
 
-bool ScQueryEvaluator::isMatchWholeCell(const ScDocument& rDoc, const ScQueryEntry& rEntry)
+bool ScQueryEvaluator::isMatchWholeCell(const ScDocument& rDoc, ScQueryOp eOp)
 {
-    return isMatchWholeCellHelper(rDoc.GetDocOptions().IsMatchWholeCell(), rEntry);
+    return isMatchWholeCellHelper(rDoc.GetDocOptions().IsMatchWholeCell(), eOp);
 }
 
 void ScQueryEvaluator::setupTransliteratorIfNeeded()
@@ -133,7 +133,7 @@ bool ScQueryEvaluator::isRealWildOrRegExp(const ScQueryEntry& rEntry) const
     if (mrParam.eSearchType == utl::SearchParam::SearchType::Normal)
         return false;
 
-    return isTextMatchOp(rEntry);
+    return isTextMatchOp(rEntry.eOp);
 }
 
 bool ScQueryEvaluator::isTestWildOrRegExp(const ScQueryEntry& rEntry) const
@@ -147,10 +147,10 @@ bool ScQueryEvaluator::isTestWildOrRegExp(const ScQueryEntry& rEntry) const
     return (rEntry.eOp == SC_LESS_EQUAL || rEntry.eOp == SC_GREATER_EQUAL);
 }
 
-bool ScQueryEvaluator::isQueryByValue(const ScQueryEntry& rEntry, const ScQueryEntry::Item& rItem,
+bool ScQueryEvaluator::isQueryByValue(ScQueryOp eOp, ScQueryEntry::QueryType eType,
                                       const ScRefCellValue& rCell)
 {
-    if (rItem.meType == ScQueryEntry::ByString || isPartialTextMatchOp(rEntry))
+    if (eType == ScQueryEntry::ByString || isPartialTextMatchOp(eOp))
         return false;
 
     return isQueryByValueForCell(rCell);
@@ -166,13 +166,13 @@ bool ScQueryEvaluator::isQueryByValueForCell(const ScRefCellValue& rCell)
     return rCell.hasNumeric();
 }
 
-bool ScQueryEvaluator::isQueryByString(const ScQueryEntry& rEntry, const ScQueryEntry::Item& rItem,
+bool ScQueryEvaluator::isQueryByString(ScQueryOp eOp, ScQueryEntry::QueryType eType,
                                        const ScRefCellValue& rCell)
 {
-    if (isTextMatchOp(rEntry))
+    if (isTextMatchOp(eOp))
         return true;
 
-    if (rItem.meType != ScQueryEntry::ByString)
+    if (eType != ScQueryEntry::ByString)
         return false;
 
     return rCell.hasString();
@@ -298,8 +298,7 @@ std::pair<bool, bool> ScQueryEvaluator::compareByValue(const ScRefCellValue& rCe
     return std::pair<bool, bool>(bOk, bTestEqual);
 }
 
-OUString ScQueryEvaluator::getCellString(const ScRefCellValue& rCell, SCROW nRow,
-                                         const ScQueryEntry& rEntry,
+OUString ScQueryEvaluator::getCellString(const ScRefCellValue& rCell, SCROW nRow, SCCOL nCol,
                                          const svl::SharedString** sharedString)
 {
     if (rCell.getType() == CELLTYPE_FORMULA
@@ -326,10 +325,8 @@ OUString ScQueryEvaluator::getCellString(const ScRefCellValue& rCell, SCROW nRow
     else
     {
         sal_uInt32 nFormat
-            = mpContext
-                  ? mrTab.GetNumberFormat(*mpContext, ScAddress(static_cast<SCCOL>(rEntry.nField),
-                                                                nRow, mrTab.GetTab()))
-                  : mrTab.GetNumberFormat(static_cast<SCCOL>(rEntry.nField), nRow);
+            = mpContext ? mrTab.GetNumberFormat(*mpContext, ScAddress(nCol, nRow, mrTab.GetTab()))
+                        : mrTab.GetNumberFormat(nCol, nRow);
         SvNumberFormatter* pFormatter
             = mpContext ? mpContext->GetFormatTable() : mrDoc.GetFormatTable();
         return ScCellFormat::GetInputString(rCell, nFormat, *pFormatter, mrDoc, sharedString, true);
@@ -346,7 +343,7 @@ bool ScQueryEvaluator::isFastCompareByString(const ScQueryEntry& rEntry) const
     const bool bTestWildOrRegExp = isTestWildOrRegExp(rEntry);
     // SC_EQUAL is part of isTextMatchOp(rEntry)
     return rEntry.eOp == SC_EQUAL && !bRealWildOrRegExp && !bTestWildOrRegExp
-           && isMatchWholeCell(rEntry);
+           && isMatchWholeCell(rEntry.eOp);
 }
 
 // The value is placed inside one parameter: [pValueSource1] or [pValueSource2] but never in both.
@@ -363,7 +360,7 @@ std::pair<bool, bool> ScQueryEvaluator::compareByString(const ScQueryEntry& rEnt
     if (bFast)
         bMatchWholeCell = true;
     else
-        bMatchWholeCell = isMatchWholeCell(rEntry);
+        bMatchWholeCell = isMatchWholeCell(rEntry.eOp);
     const bool bRealWildOrRegExp = !bFast && isRealWildOrRegExp(rEntry);
     const bool bTestWildOrRegExp = !bFast && isTestWildOrRegExp(rEntry);
 
@@ -431,7 +428,7 @@ std::pair<bool, bool> ScQueryEvaluator::compareByString(const ScQueryEntry& rEnt
     if (bFast || !bRealWildOrRegExp)
     {
         // Simple string matching i.e. no regexp match.
-        if (bFast || isTextMatchOp(rEntry))
+        if (bFast || isTextMatchOp(rEntry.eOp))
         {
             // Check this even with bFast.
             if (rItem.meType != ScQueryEntry::ByString && rItem.maString.isEmpty())
@@ -779,7 +776,7 @@ std::pair<bool, bool> ScQueryEvaluator::processEntry(SCROW nRow, SCCOL nCol, ScR
         // and simple matching is used, see compareByString()
         if (!cellStringSet)
         {
-            cellString = getCellString(aCell, nRow, rEntry, &cellSharedString);
+            cellString = getCellString(aCell, nRow, rEntry.nField, &cellSharedString);
             cellStringSet = true;
         }
         // Allow also checking ScQueryEntry::ByValue if the cell is not numeric,
@@ -853,17 +850,17 @@ std::pair<bool, bool> ScQueryEvaluator::processEntry(SCROW nRow, SCCOL nCol, ScR
             aRes.first |= aThisRes.first;
             aRes.second |= aThisRes.second;
         }
-        else if (isQueryByValue(rEntry, rItem, aCell))
+        else if (isQueryByValue(rEntry.eOp, rItem.meType, aCell))
         {
             std::pair<bool, bool> aThisRes = compareByValue(aCell, nCol, nRow, rEntry, rItem);
             aRes.first |= aThisRes.first;
             aRes.second |= aThisRes.second;
         }
-        else if (isQueryByString(rEntry, rItem, aCell))
+        else if (isQueryByString(rEntry.eOp, rItem.meType, aCell))
         {
             if (!cellStringSet)
             {
-                cellString = getCellString(aCell, nRow, rEntry, &cellSharedString);
+                cellString = getCellString(aCell, nRow, rEntry.nField, &cellSharedString);
                 cellStringSet = true;
             }
             std::pair<bool, bool> aThisRes;
diff --git a/sc/source/core/data/queryiter.cxx b/sc/source/core/data/queryiter.cxx
index 5156385c4b8f..e712b24d4443 100644
--- a/sc/source/core/data/queryiter.cxx
+++ b/sc/source/core/data/queryiter.cxx
@@ -1219,8 +1219,9 @@ ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >::Mak
     return SortedCacheIndexer(rCells, nStartRow, nEndRow, sortedCache);
 }
 
-static bool CanBeUsedForSorterCache(const ScDocument& rDoc, const ScQueryParam& rParam,
-    const ScFormulaCell* cell, const ScComplexRefData* refData)
+static bool CanBeUsedForSorterCache(ScDocument& rDoc, const ScQueryParam& rParam,
+    SCTAB nTab, const ScFormulaCell* cell, const ScComplexRefData* refData,
+    ScInterpreterContext& context)
 {
     if(!rParam.GetEntry(0).bDoQuery || rParam.GetEntry(1).bDoQuery
         || rParam.GetEntry(0).GetQueryItems().size() != 1 )
@@ -1237,19 +1238,12 @@ static bool CanBeUsedForSorterCache(const ScDocument& rDoc, const ScQueryParam&
     if(rParam.mbRangeLookup)
         return false;
     if(rParam.GetEntry(0).GetQueryItem().meType == ScQueryEntry::ByString
-        && !ScQueryEvaluator::isMatchWholeCell(rDoc, rParam.GetEntry(0)))
+        && !ScQueryEvaluator::isMatchWholeCell(rDoc, rParam.GetEntry(0).eOp))
         return false; // substring matching cannot be sorted
     if(rParam.GetEntry(0).eOp != SC_LESS && rParam.GetEntry(0).eOp != SC_LESS_EQUAL
         && rParam.GetEntry(0).eOp != SC_GREATER && rParam.GetEntry(0).eOp != SC_GREATER_EQUAL
         && rParam.GetEntry(0).eOp != SC_EQUAL)
         return false;
-    // tdf#149071 - numbers entered as string can be compared both as numbers
-    // and as strings, depending on the cell content, and that makes it hard to pre-sort
-    // the data; such queries are ScQueryEntry::ByValue but have maString set too
-    // (see ScQueryParamBase::FillInExcelSyntax())
-    if(rParam.GetEntry(0).GetQueryItem().meType == ScQueryEntry::ByValue
-        && rParam.GetEntry(0).GetQueryItem().maString.isValid())
-        return false;
     // For unittests allow inefficient caching, in order for the code to be checked.
     static bool inUnitTest = getenv("LO_TESTNAME") != nullptr;
     if(refData == nullptr || refData->Ref1.IsRowRel() || refData->Ref2.IsRowRel())
@@ -1270,6 +1264,15 @@ static bool CanBeUsedForSorterCache(const ScDocument& rDoc, const ScQueryParam&
         if(!inUnitTest)
             return false;
     }
+    // Check that all the relevant caches would be valid (may not be the case when mixing
+    // numeric and string cells for ByValue lookups).
+    for(SCCOL col : rDoc.GetAllocatedColumnsRange(nTab, rParam.nCol1, rParam.nCol2))
+    {
+        ScRange aSortedRangeRange( col, rParam.nRow1, nTab, col, rParam.nRow2, nTab);
+        ScSortedRangeCache& cache = rDoc.GetSortedRangeCache( aSortedRangeRange, rParam, &context );
+        if(!cache.isValid())
+            return false;
+    }
     return true;
 }
 
@@ -1329,10 +1332,11 @@ bool ScQueryCellIterator< ScQueryCellIteratorAccess::SortedCache >::GetNext()
     return GetThis();
 }
 
-bool ScQueryCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const ScQueryParam& rParam,
-    const ScFormulaCell* cell, const ScComplexRefData* refData)
+bool ScQueryCellIteratorSortedCache::CanBeUsed(ScDocument& rDoc, const ScQueryParam& rParam,
+    SCTAB nTab, const ScFormulaCell* cell, const ScComplexRefData* refData,
+    ScInterpreterContext& context)
 {
-    return CanBeUsedForSorterCache(rDoc, rParam, cell, refData);
+    return CanBeUsedForSorterCache(rDoc, rParam, nTab, cell, refData, context);
 }
 
 // Countifs implementation.
@@ -1359,10 +1363,11 @@ sal_uInt64 ScCountIfCellIterator< accessType >::GetCount()
 }
 
 
-bool ScCountIfCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const ScQueryParam& rParam,
-    const ScFormulaCell* cell, const ScComplexRefData* refData)
+bool ScCountIfCellIteratorSortedCache::CanBeUsed(ScDocument& rDoc, const ScQueryParam& rParam,
+    SCTAB nTab, const ScFormulaCell* cell, const ScComplexRefData* refData,
+    ScInterpreterContext& context)
 {
-    return CanBeUsedForSorterCache(rDoc, rParam, cell, refData);
+    return CanBeUsedForSorterCache(rDoc, rParam, nTab, cell, refData, context);
 }
 
 template<>
diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
index dbffd939d53d..957d93005375 100644
--- a/sc/source/core/tool/interpr1.cxx
+++ b/sc/source/core/tool/interpr1.cxx
@@ -5803,7 +5803,8 @@ void ScInterpreter::ScCountIf()
             }
             else
             {
-                if(ScCountIfCellIteratorSortedCache::CanBeUsed(mrDoc, rParam, pMyFormulaCell, refData))
+                if(ScCountIfCellIteratorSortedCache::CanBeUsed(mrDoc, rParam, nTab1, pMyFormulaCell,
+                        refData, mrContext))
                 {
                     ScCountIfCellIteratorSortedCache aCellIter(mrDoc, mrContext, nTab1, rParam, false);
                     fCount += aCellIter.GetCount();
@@ -6198,7 +6199,8 @@ void ScInterpreter::IterateParametersIfs( double(*ResultFunc)( const sc::ParamIf
             }
             else
             {
-                if( ScQueryCellIteratorSortedCache::CanBeUsed( mrDoc, rParam, pMyFormulaCell, refData ))
+                if( ScQueryCellIteratorSortedCache::CanBeUsed( mrDoc, rParam, nTab1, pMyFormulaCell,
+                        refData, mrContext ))
                 {
                     ScQueryCellIteratorSortedCache aCellIter(mrDoc, mrContext, nTab1, rParam, false);
                     // Increment Entry.nField in iterator when switching to next column.
@@ -10045,7 +10047,7 @@ static bool lcl_LookupQuery( ScAddress & o_rResultPos, ScDocument& rDoc, ScInter
     }
     else // EQUAL
     {
-        if( ScQueryCellIteratorSortedCache::CanBeUsed( rDoc, rParam, cell, refData ))
+        if( ScQueryCellIteratorSortedCache::CanBeUsed( rDoc, rParam, rParam.nTab, cell, refData, rContext ))
         {
             ScQueryCellIteratorSortedCache aCellIter( rDoc, rContext, rParam.nTab, rParam, false);
             if (aCellIter.GetFirst())
diff --git a/sc/source/core/tool/rangecache.cxx b/sc/source/core/tool/rangecache.cxx
index 6a80ca786082..7f1e9cfe8235 100644
--- a/sc/source/core/tool/rangecache.cxx
+++ b/sc/source/core/tool/rangecache.cxx
@@ -25,18 +25,16 @@
 #include <queryparam.hxx>
 
 #include <sal/log.hxx>
+#include <svl/numformat.hxx>
 #include <unotools/collatorwrapper.hxx>
 
-static bool needsDescending(const ScQueryParam& param)
+static bool needsDescending(ScQueryOp op)
 {
-    assert(param.GetEntry(0).bDoQuery && !param.GetEntry(1).bDoQuery
-           && param.GetEntry(0).GetQueryItems().size() == 1);
-    assert(param.GetEntry(0).eOp == SC_GREATER || param.GetEntry(0).eOp == SC_GREATER_EQUAL
-           || param.GetEntry(0).eOp == SC_LESS || param.GetEntry(0).eOp == SC_LESS_EQUAL
-           || param.GetEntry(0).eOp == SC_EQUAL);
+    assert(op == SC_GREATER || op == SC_GREATER_EQUAL || op == SC_LESS || op == SC_LESS_EQUAL
+           || op == SC_EQUAL);
     // We want all matching values to start in the sort order,
     // since the data is searched from start until the last matching one.
-    return param.GetEntry(0).eOp == SC_GREATER || param.GetEntry(0).eOp == SC_GREATER_EQUAL;
+    return op == SC_GREATER || op == SC_GREATER_EQUAL;
 }
 
 static ScSortedRangeCache::ValueType toValueType(const ScQueryParam& param)
@@ -55,8 +53,8 @@ ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange,
                                        const ScQueryParam& param, ScInterpreterContext* context)
     : maRange(rRange)
     , mpDoc(pDoc)
-    , mDescending(needsDescending(param))
-    , mValues(toValueType(param))
+    , mValid(false)
+    , mValueType(toValueType(param))
 {
     assert(maRange.aStart.Col() == maRange.aEnd.Col());
     assert(maRange.aStart.Tab() == maRange.aEnd.Tab());
@@ -66,6 +64,8 @@ ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange,
            && param.GetEntry(0).GetQueryItems().size() == 1);
     const ScQueryEntry& entry = param.GetEntry(0);
     const ScQueryEntry::Item& item = entry.GetQueryItem();
+    mQueryOp = entry.eOp;
+    mQueryType = item.meType;
 
     SCROW startRow = maRange.aStart.Row();
     SCROW endRow = maRange.aEnd.Row();
@@ -73,9 +73,9 @@ ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange,
     SCCOL endCol = maRange.aEnd.Col();
     if (!item.mbMatchEmpty)
         if (!pDoc->ShrinkToDataArea(nTab, startCol, startRow, endCol, endRow))
-            return;
+            return; // no data cells, no need for a cache
 
-    if (mValues == ValueType::Values)
+    if (mValueType == ValueType::Values)
     {
         struct RowData
         {
@@ -86,12 +86,32 @@ ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange,
         for (SCROW nRow = startRow; nRow <= endRow; ++nRow)
         {
             ScRefCellValue cell(pDoc->GetRefCellValue(ScAddress(nCol, nRow, nTab)));
-            if (ScQueryEvaluator::isQueryByValue(entry, item, cell))
+            if (ScQueryEvaluator::isQueryByValue(mQueryOp, mQueryType, cell))
                 rowData.push_back(RowData{ nRow, cell.getValue() });
+            else if (ScQueryEvaluator::isQueryByString(mQueryOp, mQueryType, cell))
+            {
+                // Make sure that other possibilities in the generic handling
+                // in ScQueryEvaluator::processEntry() do not alter the results.
+                // (ByTextColor/ByBackgroundColor are blocked by CanBeUsedForSorterCache(),
+                // but isQueryByString() is possible if the cell content is a string.
+                // And including strings here would be tricky, as the string comparison
+                // may possibly(?) be different than a numeric one. So check if the string
+                // may possibly match a number, by converting it to one. If it can't match,
+                // then it's fine to ignore it (and it can happen e.g. if the query uses
+                // the whole column which includes a textual header). But if it can possibly
+                // match, then bail out and leave it to the unoptimized case.
+                // TODO Maybe it would actually work to use the numeric value obtained here?
+                if (!ScQueryEvaluator::isMatchWholeCell(*pDoc, mQueryOp))
+                    return; // substring matching cannot be sorted
+                sal_uInt32 format = 0;
+                double value;
+                if (context->GetFormatTable()->IsNumberFormat(cell.getString(pDoc), format, value))
+                    return;
+            }
         }
         std::stable_sort(rowData.begin(), rowData.end(),
                          [](const RowData& d1, const RowData& d2) { return d1.value < d2.value; });
-        if (mDescending)
+        if (needsDescending(entry.eOp))
             for (auto it = rowData.rbegin(); it != rowData.rend(); ++it)
                 mSortedRows.emplace_back(it->row);
         else
@@ -113,31 +133,40 @@ ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange,
         for (SCROW nRow = startRow; nRow <= endRow; ++nRow)
         {
             ScRefCellValue cell(pDoc->GetRefCellValue(ScAddress(nCol, nRow, nTab)));
-            if (ScQueryEvaluator::isQueryByString(entry, item, cell))
+            // This should be used only with ScQueryEntry::ByString, and that
+            // means that ScQueryEvaluator::isQueryByString() should be the only
+            // possibility in the generic handling in ScQueryEvaluator::processEntry()
+            // (ByTextColor/ByBackgroundColor are blocked by CanBeUsedForSorterCache(),
+            // and isQueryByValue() is blocked by ScQueryEntry::ByString).
+            assert(mQueryType == ScQueryEntry::ByString);
+            assert(!ScQueryEvaluator::isQueryByValue(mQueryOp, mQueryType, cell));
+            if (ScQueryEvaluator::isQueryByString(mQueryOp, mQueryType, cell))
             {
                 const svl::SharedString* sharedString = nullptr;
-                OUString string = evaluator.getCellString(cell, nRow, entry, &sharedString);
+                OUString string = evaluator.getCellString(cell, nRow, nCol, &sharedString);
                 if (sharedString)
                     string = sharedString->getString();
                 rowData.push_back(RowData{ nRow, string });
             }
         }
         CollatorWrapper& collator
-            = ScGlobal::GetCollator(mValues == ValueType::StringsCaseSensitive);
+            = ScGlobal::GetCollator(mValueType == ValueType::StringsCaseSensitive);
         std::stable_sort(rowData.begin(), rowData.end(),
                          [&collator](const RowData& d1, const RowData& d2) {
                              return collator.compareString(d1.string, d2.string) < 0;
                          });
-        if (mDescending)
+        if (needsDescending(entry.eOp))
             for (auto it = rowData.rbegin(); it != rowData.rend(); ++it)
                 mSortedRows.emplace_back(it->row);
         else
             for (const RowData& d : rowData)
                 mSortedRows.emplace_back(d.row);
     }
+
     mRowToIndex.resize(maRange.aEnd.Row() - maRange.aStart.Row() + 1, mSortedRows.max_size());
     for (size_t i = 0; i < mSortedRows.size(); ++i)
         mRowToIndex[mSortedRows[i] - maRange.aStart.Row()] = i;
+    mValid = true;
 }
 
 void ScSortedRangeCache::Notify(const SfxHint& rHint)
@@ -157,7 +186,11 @@ void ScSortedRangeCache::Notify(const SfxHint& rHint)
 ScSortedRangeCache::HashKey ScSortedRangeCache::makeHashKey(const ScRange& range,
                                                             const ScQueryParam& param)
 {
-    return { range, needsDescending(param), toValueType(param) };
+    assert(param.GetEntry(0).bDoQuery && !param.GetEntry(1).bDoQuery
+           && param.GetEntry(0).GetQueryItems().size() == 1);
+    const ScQueryEntry& entry = param.GetEntry(0);
+    const ScQueryEntry::Item& item = entry.GetQueryItem();
+    return { range, toValueType(param), entry.eOp, item.meType };
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */