Check for consistent range dimensions in SUMIFS formula

ClosedXML · Apr 12, 2023 · b5cdb1e · b5cdb1e
1 parent 98db952
commit b5cdb1e
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 3 deletions.
diff --git a/ClosedXML.Tests/Excel/CalcEngine/MathTrigTests.cs b/ClosedXML.Tests/Excel/CalcEngine/MathTrigTests.cs
@@ -2063,6 +2063,17 @@ public void SumIfs_ReturnsCorrectValues_ReferenceExampleForSumIf1FromMicrosoft(i
             }
         }
 
+        [Test]
+        [TestCase("SUMIFS(D1:E5,A1:B5,\"A*\",C1:C5,\">2\")")]
+        [TestCase("SUMIFS(H1:I3,A1:B3,1,D1:F2,2)")]
+        [TestCase("SUMIFS(D:E,A:B,\"A*\",C:C,\">2\")")]
+        public void SumIfs_ReturnsErrorWhenRangeDimensionsAreNotSame(string formula)
+        {
+            using var wb = new XLWorkbook();
+            var ws = wb.AddWorksheet();
+            Assert.AreEqual(XLError.IncompatibleValue, ws.Evaluate(formula));
+        }
+
         [Test]
         public void SumProduct()
         {

diff --git a/ClosedXML/Excel/CalcEngine/CalcEngineHelpers.cs b/ClosedXML/Excel/CalcEngine/CalcEngineHelpers.cs
@@ -134,12 +134,26 @@ internal static bool ValueIsBlank(object value)
         /// <param name="rangeExpression">Expression referring to the cell range.</param>
         /// <returns>Total number of cells in the range.</returns>
         internal static long GetTotalCellsCount(XObjectExpression rangeExpression)
+        {
+            var (columnCount, rowCount) = GetRangeDimensions(rangeExpression);
+            return (long)columnCount * (long)rowCount;
+        }
+
+        /// <summary>
+        /// Get dimensions of the specified range without initializing them all
+        /// (which might cause serious performance issues on column-wide calculations).
+        /// </summary>
+        /// <param name="rangeExpression">Expression referring to the cell range.</param>
+        /// <returns>A tuple of column and row counts.</returns>
+        internal static (int columnCount, int rowCount) GetRangeDimensions(XObjectExpression rangeExpression)
         {
             var range = (rangeExpression?.Value as CellRangeReference)?.Range;
             if (range == null)
-                return 0;
-            return (long)(range.LastColumn().ColumnNumber() - range.FirstColumn().ColumnNumber() + 1) *
-                   (long)(range.LastRow().RowNumber() - range.FirstRow().RowNumber() + 1);
+            {
+                return (0, 0);
+            }
+
+            return (range.ColumnCount(), range.RowCount());
         }
 
         internal static bool TryExtractRange(Expression expression, out IXLRange range, out XLError calculationErrorType)

diff --git a/ClosedXML/Excel/CalcEngine/Functions/MathTrig.cs b/ClosedXML/Excel/CalcEngine/Functions/MathTrig.cs
@@ -979,6 +979,7 @@ private static object SumIfs(List<Expression> p)
         {
             // get parameters
             var sumRange = p[0] as IEnumerable;
+            var sumRangeDimensions = CalcEngineHelpers.GetRangeDimensions(p[0] as XObjectExpression);
 
             var sumRangeValues = new List<object>();
             foreach (var value in sumRange)
@@ -991,6 +992,15 @@ private static object SumIfs(List<Expression> p)
 
             int numberOfCriteria = p.Count / 2; // int division returns floor() automatically, that's what we want.
 
+            for (int criteriaPair = 0; criteriaPair < numberOfCriteria; criteriaPair++)
+            {
+                var criterionDimensions = CalcEngineHelpers.GetRangeDimensions(p[criteriaPair * 2 + 1] as XObjectExpression);
+                if (criterionDimensions != sumRangeDimensions)
+                {
+                    return XLError.IncompatibleValue;
+                }
+            }
+
             // prepare criteria-parameters:
             var criteriaRanges = new Tuple<object, IList<object>>[numberOfCriteria];
             for (int criteriaPair = 0; criteriaPair < numberOfCriteria; criteriaPair++)