tdf#133858 reduce the double-ref range to data content

in certain matrix formulas like SUM(IF(A1=G:G, H:H)*B1/B2) where whole
columns are used for comparison in the condition of IF ultimately
followed by a reducer like SUM. In such cases we can safely reduce the
double-refs involved in the comparison to the sheet area where there is
data before converting the data to ScMatrix.

This is a more restricted version of Noel's fix in
37ffe509ef011357123642577c04ff296d59ce68

Change-Id: I1c2e8985adedb3f4c4648f541fb0e8e7d0fae033
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/109050
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110580
Reviewed-by: Andras Timar <andras.timar@collabora.com>

10 files changed, 247 insertions, 4 deletions

diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx
index 51849f8caeb0..03af9a280942 100644
--- a/formula/source/core/api/FormulaCompiler.cxx
+++ b/formula/source/core/api/FormulaCompiler.cxx
@@ -1671,18 +1671,25 @@ void FormulaCompiler::Factor()
                     HandleIIOpCode(pFacToken, pArgArray,
                                    std::min(nSepCount, static_cast<sal_uInt32>(FORMULA_MAXPARAMSII)));
             }
+            bool bDone = false;
             if (bBadName)
                 ;   // nothing, keep current token for return
             else if (eOp != ocClose)
                 SetError( FormulaError::PairExpected);
             else
+            {
                 NextToken();
+                bDone = true;
+            }
             // Jumps are just normal functions for the FunctionAutoPilot tree view
             if (!mbJumpCommandReorder && pFacToken->GetType() == svJump)
                 pFacToken = new FormulaFAPToken( pFacToken->GetOpCode(), nSepCount, pFacToken );
             else
                 pFacToken->SetByte( nSepCount );
             PutCode( pFacToken );
+
+            if (bDone)
+                AnnotateOperands();
         }
         else if (IsOpCodeJumpCommand(eOp))
         {
diff --git a/include/formula/FormulaCompiler.hxx b/include/formula/FormulaCompiler.hxx
index fef6e82e39e1..65765b84d104 100644
--- a/include/formula/FormulaCompiler.hxx
+++ b/include/formula/FormulaCompiler.hxx
@@ -337,6 +337,8 @@ protected:
     // Called from CompileTokenArray() after RPN code generation is done.
     virtual void PostProcessCode() {}
 
+    virtual void AnnotateOperands() {}
+
     OUString            aCorrectedFormula;      // autocorrected Formula
     OUString            aCorrectedSymbol;       // autocorrected Symbol
 
diff --git a/sc/inc/compiler.hxx b/sc/inc/compiler.hxx
index 5bd9068da8d2..1faac8362638 100644
--- a/sc/inc/compiler.hxx
+++ b/sc/inc/compiler.hxx
@@ -508,10 +508,12 @@ private:
     bool HandleIIOpCodeInternal(formula::FormulaToken* token, formula::FormulaToken*** pppToken, sal_uInt8 nNumParams);
     bool SkipImplicitIntersectionOptimization(const formula::FormulaToken* token) const;
     virtual void PostProcessCode() override;
+    virtual void AnnotateOperands() override;
     static bool ParameterMayBeImplicitIntersection(const formula::FormulaToken* token, int parameter);
     void ReplaceDoubleRefII(formula::FormulaToken** ppDoubleRefTok);
     bool AdjustSumRangeShape(const ScComplexRefData& rBaseRange, ScComplexRefData& rSumRange);
     void CorrectSumRange(const ScComplexRefData& rBaseRange, ScComplexRefData& rSumRange, formula::FormulaToken** ppSumRangeToken);
+    void AnnotateTrimOnDoubleRefs();
 };
 
 #endif
diff --git a/sc/inc/refdata.hxx b/sc/inc/refdata.hxx
index 7edac9f4bb01..4b23bb23b843 100644
--- a/sc/inc/refdata.hxx
+++ b/sc/inc/refdata.hxx
@@ -119,6 +119,11 @@ struct ScComplexRefData
 {
     ScSingleRefData Ref1;
     ScSingleRefData Ref2;
+    bool bTrimToData;
+
+    ScComplexRefData() :
+        bTrimToData(false)
+    {}
 
     void InitFlags()
         { Ref1.InitFlags(); Ref2.InitFlags(); }
@@ -191,6 +196,9 @@ struct ScComplexRefData
 
     bool IsDeleted() const;
 
+    bool IsTrimToData() const { return bTrimToData; }
+    void SetTrimToData(bool bSet) { bTrimToData = bSet; }
+
 #if DEBUG_FORMULA_COMPILER
     void Dump( int nIndent = 0 ) const;
 #endif
diff --git a/sc/qa/unit/helper/qahelper.cxx b/sc/qa/unit/helper/qahelper.cxx
index 64f41b033ff0..4530a075ad63 100644
--- a/sc/qa/unit/helper/qahelper.cxx
+++ b/sc/qa/unit/helper/qahelper.cxx
@@ -406,8 +406,6 @@ ScRangeList getChartRanges(ScDocument& rDoc, const SdrOle2Obj& rChartObj)
     return aRanges;
 }
 
-namespace {
-
 ScTokenArray* getTokens(ScDocument& rDoc, const ScAddress& rPos)
 {
     ScFormulaCell* pCell = rDoc.GetFormulaCell(rPos);
@@ -421,8 +419,6 @@ ScTokenArray* getTokens(ScDocument& rDoc, const ScAddress& rPos)
     return pCell->GetCode();
 }
 
-}
-
 bool checkFormula(ScDocument& rDoc, const ScAddress& rPos, const char* pExpected)
 {
     ScTokenArray* pCode = getTokens(rDoc, rPos);
diff --git a/sc/qa/unit/helper/qahelper.hxx b/sc/qa/unit/helper/qahelper.hxx
index 21a57002f1a6..76540601b94f 100644
--- a/sc/qa/unit/helper/qahelper.hxx
+++ b/sc/qa/unit/helper/qahelper.hxx
@@ -225,6 +225,8 @@ SCQAHELPER_DLLPUBLIC void checkFormula(ScDocument& rDoc, const ScAddress& rPos,
 
 SCQAHELPER_DLLPUBLIC void testFormats(ScBootstrapFixture* pTest, ScDocument* pDoc, sal_Int32 nFormat);
 
+SCQAHELPER_DLLPUBLIC ScTokenArray* getTokens(ScDocument& rDoc, const ScAddress& rPos);
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/qa/unit/ucalc.hxx b/sc/qa/unit/ucalc.hxx
index 45b46a9fdc10..12340323e609 100644
--- a/sc/qa/unit/ucalc.hxx
+++ b/sc/qa/unit/ucalc.hxx
@@ -149,6 +149,7 @@ public:
     void testFormulaCompilerImplicitIntersection1ParamWithChange();
     void testFormulaCompilerImplicitIntersection1NoGroup();
     void testFormulaCompilerImplicitIntersectionOperators();
+    void testFormulaAnnotateTrimOnDoubleRefs();
     void testFormulaRefUpdate();
     void testFormulaRefUpdateRange();
     void testFormulaRefUpdateSheets();
@@ -586,6 +587,7 @@ public:
     CPPUNIT_TEST(testFormulaCompilerImplicitIntersection1ParamWithChange);
     CPPUNIT_TEST(testFormulaCompilerImplicitIntersection1NoGroup);
     CPPUNIT_TEST(testFormulaCompilerImplicitIntersectionOperators);
+    CPPUNIT_TEST(testFormulaAnnotateTrimOnDoubleRefs);
     CPPUNIT_TEST(testFormulaRefUpdate);
     CPPUNIT_TEST(testFormulaRefUpdateRange);
     CPPUNIT_TEST(testFormulaRefUpdateSheets);
diff --git a/sc/qa/unit/ucalc_formula.cxx b/sc/qa/unit/ucalc_formula.cxx
index 3c1b462933c4..f634c25c049d 100644
--- a/sc/qa/unit/ucalc_formula.cxx
+++ b/sc/qa/unit/ucalc_formula.cxx
@@ -1446,6 +1446,115 @@ void Test::testFormulaCompilerImplicitIntersectionOperators()
     m_pDoc->DeleteTab(0);
 }
 
+void Test::testFormulaAnnotateTrimOnDoubleRefs()
+{
+    m_pDoc->InsertTab(0, "Test");
+    sc::AutoCalcSwitch aACSwitch(*m_pDoc, true); // turn auto calc on.
+
+    constexpr sal_Int32 nCols = 2;
+    constexpr sal_Int32 nRows = 5;
+
+    // Values in A1:B5
+    constexpr sal_Int32 aMat[nRows][nCols] = {
+        {4, 50},
+        {5, 30},
+        {4, 40},
+        {0, 70},
+        {5, 90}
+    };
+
+    for (sal_Int32 nCol = 0; nCol < nCols; ++nCol)
+    {
+        for (sal_Int32 nRow = 0; nRow < nRows; ++nRow)
+            m_pDoc->SetValue(nCol, nRow, 0, aMat[nRow][nCol]);
+    }
+
+    m_pDoc->SetValue(2, 0, 0, 4); // C1 = 4
+    m_pDoc->SetValue(3, 0, 0, 5); // D1 = 5
+
+    ScMarkData aMark;
+    aMark.SelectOneTable(0);
+
+    struct TestCase
+    {
+        OUString aFormula;
+        ScRange aTrimmableRange;
+        double fResult;
+        bool bMatrixFormula;
+    };
+
+    constexpr sal_Int32 nTestCases = 5;
+    TestCase aTestCases[nTestCases] = {
+        {
+            "=SUM(IF($C$1=A:A;B:B)/10*D1)",
+            ScRange(0, 0, 0, 0, 1048575, 0),
+            45.0,
+            true
+        },
+
+        {
+            "=SUM(IF(A:A=5;B:B)/10*D1)",
+            ScRange(0, 0, 0, 0, 1048575, 0),
+            60.0,
+            true
+        },
+
+        {
+            "=SUM(IF($C$1=A:A;B:B;B:B)/10*D1)",  // IF has else clause
+            ScRange(-1, -1, -1, -1, -1, -1),     // Has no trimmable double-ref.
+            140.0,
+            true
+        },
+
+        {
+            "=SUM(IF($C$1=A:A;B:B)/10*D1)",
+            ScRange(-1, -1, -1, -1, -1, -1),     // Has no trimmable double-ref.
+            25,
+            false                                // Not in matrix mode.
+        },
+
+        {
+            "=SUMPRODUCT(A:A=$C$1; 1-(A:A=$C$1))",
+            ScRange(-1, -1, -1, -1, -1, -1),     // Has no trimmable double-ref.
+            0.0,
+            false                                // Not in matrix mode.
+        },
+    };
+
+    for (sal_Int32 nTestIdx = 0; nTestIdx < nTestCases; ++nTestIdx)
+    {
+        TestCase& rTestCase = aTestCases[nTestIdx];
+        if (rTestCase.bMatrixFormula)
+            m_pDoc->InsertMatrixFormula(4, 0, 4, 0, aMark, rTestCase.aFormula); // Formula in E1
+        else
+            m_pDoc->SetString(ScAddress(4, 0, 0), rTestCase.aFormula);          // Formula in E1
+
+        std::string aMsgStart = "TestCase#" + std::to_string(nTestIdx + 1) + " : ";
+        CPPUNIT_ASSERT_EQUAL_MESSAGE(aMsgStart + "Incorrect formula result", rTestCase.fResult, m_pDoc->GetValue(ScAddress(4, 0, 0)));
+
+        ScTokenArray* pCode = getTokens(*m_pDoc, ScAddress(4, 0, 0));
+        sal_Int32 nLen = pCode->GetCodeLen();
+        FormulaToken** pRPNArray = pCode->GetCode();
+
+        for (sal_Int32 nIdx = 0; nIdx < nLen; ++nIdx)
+        {
+            FormulaToken* pTok = pRPNArray[nIdx];
+            if (pTok && pTok->GetType() == svDoubleRef)
+            {
+                ScRange aRange = pTok->GetDoubleRef()->toAbs(ScAddress(4, 0, 0));
+                if (aRange == rTestCase.aTrimmableRange)
+                    CPPUNIT_ASSERT_MESSAGE(aMsgStart + "Double ref is incorrectly flagged as not trimmable to data",
+                        pTok->GetDoubleRef()->IsTrimToData());
+                else
+                    CPPUNIT_ASSERT_MESSAGE(aMsgStart + "Double ref is incorrectly flagged as trimmable to data",
+                        !pTok->GetDoubleRef()->IsTrimToData());
+            }
+        }
+    }
+
+    m_pDoc->DeleteTab(0);
+}
+
 void Test::testFormulaRefUpdate()
 {
     m_pDoc->InsertTab(0, "Formula");
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index e4620b1f14a1..e13757a6b18d 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -6023,6 +6023,11 @@ void ScCompiler::PostProcessCode()
     mPendingImplicitIntersectionOptimizations.clear();
 }
 
+void ScCompiler::AnnotateOperands()
+{
+    AnnotateTrimOnDoubleRefs();
+}
+
 void ScCompiler::ReplaceDoubleRefII(FormulaToken** ppDoubleRefTok)
 {
     const ScComplexRefData* pRange = (*ppDoubleRefTok)->GetDoubleRef();
@@ -6194,4 +6199,97 @@ void ScCompiler::CorrectSumRange(const ScComplexRefData& rBaseRange,
     pNewSumRangeTok->IncRef();
 }
 
+void ScCompiler::AnnotateTrimOnDoubleRefs()
+{
+    if (!pCode || !(pCode -1) || !(*(pCode - 1)))
+        return;
+
+    // OpCode of the "root" operator (which is already in RPN array).
+    OpCode eOpCode = (*(pCode - 1))->GetOpCode();
+    // eOpCode can be some operator which does not change with operands with or contains zero values.
+    if (eOpCode != ocSum)
+        return;
+
+    FormulaToken** ppTok = pCode - 2; // exclude the root operator.
+    // The following loop runs till a "pattern" is found or there is a mismatch
+    // and marks the push DoubleRef arguments as trimmable when there is a match.
+    // The pattern is
+    // SUM(IF(<reference|double>=<reference|double>, <then-clause>)<a some operands with operators / or *>)
+    // such that one of the operands of ocEqual is a double-ref.
+    // Examples of formula that matches this are:
+    //   SUM(IF(D:D=$A$1,F:F)*$H$1*2.3/$G$2)
+    //   SUM((IF(D:D=$A$1,F:F)*$H$1*2.3/$G$2)*$H$2*5/$G$3)
+    //   SUM(IF(E:E=16,F:F)*$H$1*100)
+    bool bTillClose = true;
+    bool bCloseTillIf = false;
+    sal_Int16 nToksTillIf = 0;
+    constexpr sal_Int16 MAXDIST_IF = 15;
+    while (*(ppTok))
+    {
+        FormulaToken* pTok = *ppTok;
+        OpCode eCurrOp = pTok->GetOpCode();
+        ++nToksTillIf;
+
+        // TODO : Is there a better way to handle this ?
+        // ocIf is too far off from the sum opcode.
+        if (nToksTillIf > MAXDIST_IF)
+            return;
+
+        switch (eCurrOp)
+        {
+            case ocDiv:
+            case ocMul:
+                if (!bTillClose)
+                    return;
+                break;
+            case ocPush:
+
+                break;
+            case ocClose:
+                if (bTillClose)
+                {
+                    bTillClose = false;
+                    bCloseTillIf = true;
+                }
+                else
+                    return;
+                break;
+            case ocIf:
+                {
+                    if (!bCloseTillIf)
+                        return;
+
+                    if (!pTok->IsInForceArray())
+                        return;
+
+                    const short nJumpCount = pTok->GetJump()[0];
+                    if (nJumpCount != 2) // Should have THEN but no ELSE.
+                        return;
+
+                    OpCode eCompOp = (*(ppTok - 1))->GetOpCode();
+                    if (eCompOp != ocEqual)
+                        return;
+
+                    FormulaToken* pLHS = *(ppTok - 2);
+                    FormulaToken* pRHS = *(ppTok - 3);
+                    if (((pLHS->GetType() == svSingleRef || pLHS->GetType() == svDouble) && pRHS->GetType() == svDoubleRef) ||
+                        ((pRHS->GetType() == svSingleRef || pRHS->GetType() == svDouble) && pLHS->GetType() == svDoubleRef))
+                    {
+                        if (pLHS->GetType() == svDoubleRef)
+                            pLHS->GetDoubleRef()->SetTrimToData(true);
+                        else
+                            pRHS->GetDoubleRef()->SetTrimToData(true);
+                        return;
+                    }
+                }
+                break;
+            default:
+                return;
+        }
+        --ppTok;
+    }
+
+    return;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/tool/interpr5.cxx b/sc/source/core/tool/interpr5.cxx
index aefb26c920e3..e56ed48f8290 100644
--- a/sc/source/core/tool/interpr5.cxx
+++ b/sc/source/core/tool/interpr5.cxx
@@ -325,6 +325,23 @@ ScMatrixRef ScInterpreter::CreateMatrixFromDoubleRef( const FormulaToken* pToken
         return nullptr;
     }
 
+    if (nTab1 == nTab2 && pToken)
+    {
+        const ScComplexRefData& rCRef = *pToken->GetDoubleRef();
+        if (rCRef.IsTrimToData())
+        {
+            // Clamp the size of the matrix area to rows which actually contain data.
+            // For e.g. SUM(IF over an entire column, this can make a big difference,
+            // But lets not trim the empty space from the top or left as this matters
+            // at least in matrix formulas involving IF().
+            // Refer ScCompiler::AnnotateTrimOnDoubleRefs() where double-refs are
+            // flagged for trimming.
+            SCCOL nTempCol = nCol1;
+            SCROW nTempRow = nRow1;
+            pDok->ShrinkToDataArea(nTab1, nTempCol, nTempRow, nCol2, nRow2);
+        }
+    }
+
     SCSIZE nMatCols = static_cast<SCSIZE>(nCol2 - nCol1 + 1);
     SCSIZE nMatRows = static_cast<SCSIZE>(nRow2 - nRow1 + 1);