From: Greg Woolsey Date: Mon, 18 Mar 2019 19:09:11 +0000 (+0000) Subject: #60724 - Partial implementation for SUBTOTAL() 'ignore hidden rows' variations X-Git-Tag: REL_4_1_0~41 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=82f9c292589bc9dd84c4b9b699129cc191513a03;p=poi.git #60724 - Partial implementation for SUBTOTAL() 'ignore hidden rows' variations The function still doesn't deal with auto-filtering, but it now handles variations that should skip hidden rows. Taught the evaluation framework to know about hidden rows similar to what was already there for skipping subtotals within subtotal ranges. Added unit test cases. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1855789 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java index d2d6927c82..04eed2d30c 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java @@ -45,7 +45,17 @@ final class HSSFEvaluationSheet implements EvaluationSheet { public int getLastRowNum() { return _hs.getLastRowNum(); } - + + /* (non-Javadoc) + * @see org.apache.poi.ss.formula.EvaluationSheet#isRowHidden(int) + * @since POI 4.0.2 + */ + public boolean isRowHidden(int rowIndex) { + HSSFRow row = _hs.getRow(rowIndex); + if (row == null) return false; + return row.getZeroHeight(); + } + @Override public EvaluationCell getCell(int rowIndex, int columnIndex) { HSSFRow row = _hs.getRow(rowIndex); diff --git a/src/java/org/apache/poi/ss/formula/EvaluationSheet.java b/src/java/org/apache/poi/ss/formula/EvaluationSheet.java index 82f36a971e..27606ba694 100644 --- a/src/java/org/apache/poi/ss/formula/EvaluationSheet.java +++ b/src/java/org/apache/poi/ss/formula/EvaluationSheet.java @@ -48,4 +48,12 @@ public interface EvaluationSheet { * @since POI 4.0.0 */ public int getLastRowNum(); + + /** + * Used by SUBTOTAL and similar functions that have options to ignore hidden rows + * @param rowIndex + * @return true if the row is hidden, false if not + * @since POI 4.0.2 + */ + public boolean isRowHidden(int rowIndex); } diff --git a/src/java/org/apache/poi/ss/formula/LazyAreaEval.java b/src/java/org/apache/poi/ss/formula/LazyAreaEval.java index 4bfbf41d30..f10cfe6de9 100644 --- a/src/java/org/apache/poi/ss/formula/LazyAreaEval.java +++ b/src/java/org/apache/poi/ss/formula/LazyAreaEval.java @@ -94,4 +94,14 @@ final class LazyAreaEval extends AreaEvalBase { SheetRefEvaluator _sre = _evaluator.getSheetEvaluator(_evaluator.getFirstSheetIndex()); return _sre.isSubTotal(getFirstRow() + rowIndex, getFirstColumn() + columnIndex); } + + /** + * @return whether the row at rowIndex is hidden + * @see org.apache.poi.ss.formula.eval.AreaEvalBase#isRowHidden(int) + */ + public boolean isRowHidden(int rowIndex) { + // delegate the query to the sheet evaluator which has access to internal ptgs + SheetRefEvaluator _sre = _evaluator.getSheetEvaluator(_evaluator.getFirstSheetIndex()); + return _sre.isRowHidden(getFirstRow() + rowIndex); + } } diff --git a/src/java/org/apache/poi/ss/formula/LazyRefEval.java b/src/java/org/apache/poi/ss/formula/LazyRefEval.java index c71b0e9779..4456c23a62 100644 --- a/src/java/org/apache/poi/ss/formula/LazyRefEval.java +++ b/src/java/org/apache/poi/ss/formula/LazyRefEval.java @@ -47,10 +47,22 @@ public final class LazyRefEval extends RefEvalBase { return new LazyAreaEval(area, _evaluator); } + /** + * @return true if the cell is a subtotal + */ public boolean isSubTotal() { SheetRefEvaluator sheetEvaluator = _evaluator.getSheetEvaluator(getFirstSheetIndex()); return sheetEvaluator.isSubTotal(getRow(), getColumn()); } + + /** + * @return whether the row at rowIndex is hidden + */ + public boolean isRowHidden() { + // delegate the query to the sheet evaluator which has access to internal ptgs + SheetRefEvaluator _sre = _evaluator.getSheetEvaluator(_evaluator.getFirstSheetIndex()); + return _sre.isRowHidden(getRow()); + } public String toString() { CellReference cr = new CellReference(getRow(), getColumn()); diff --git a/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java b/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java index c9b408fa51..2324eda75b 100644 --- a/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java @@ -56,6 +56,8 @@ final class SheetRefEvaluator { } /** + * @param rowIndex + * @param columnIndex * @return whether cell at rowIndex and columnIndex is a subtotal * @see org.apache.poi.ss.formula.functions.Subtotal */ @@ -77,4 +79,14 @@ final class SheetRefEvaluator { return subtotal; } + /** + * Used by functions that calculate differently depending on row visibility, like some + * variations of SUBTOTAL() + * @see org.apache.poi.ss.formula.functions.Subtotal + * @param rowIndex + * @return true if the row is hidden + */ + public boolean isRowHidden(int rowIndex) { + return getSheet().isRowHidden(rowIndex); + } } diff --git a/src/java/org/apache/poi/ss/formula/TwoDEval.java b/src/java/org/apache/poi/ss/formula/TwoDEval.java index c52d0f3eff..f3c8c974f5 100644 --- a/src/java/org/apache/poi/ss/formula/TwoDEval.java +++ b/src/java/org/apache/poi/ss/formula/TwoDEval.java @@ -19,6 +19,7 @@ package org.apache.poi.ss.formula; import org.apache.poi.ss.formula.eval.AreaEval; import org.apache.poi.ss.formula.eval.ValueEval; +import org.apache.poi.ss.formula.functions.Subtotal; /** * Common interface of {@link AreaEval} and {@link org.apache.poi.ss.formula.eval.AreaEvalBase}, @@ -64,5 +65,13 @@ public interface TwoDEval extends ValueEval { * @return true if the cell at row and col is a subtotal */ boolean isSubTotal(int rowIndex, int columnIndex); + + /** + * + * @param rowIndex + * @return true if the row is hidden + * @see Subtotal + */ + boolean isRowHidden(int rowIndex); } diff --git a/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java b/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java index edb8af7a1d..96633318d9 100644 --- a/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java +++ b/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java @@ -146,4 +146,11 @@ public abstract class AreaEvalBase implements AreaEval { return false; } + /** + * @return false by default, meaning all rows are calculated + * @see org.apache.poi.ss.formula.TwoDEval#isRowHidden(int) + */ + public boolean isRowHidden(int rowIndex) { + return false; + } } diff --git a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java index a223c81dfb..354531de15 100644 --- a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java +++ b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java @@ -62,6 +62,14 @@ final class ForkedEvaluationSheet implements EvaluationSheet { return _masterSheet.getLastRowNum(); } + /* (non-Javadoc) + * @see org.apache.poi.ss.formula.EvaluationSheet#isRowHidden(int) + * @since POI 4.0.2 + */ + public boolean isRowHidden(int rowIndex) { + return _masterSheet.isRowHidden(rowIndex); + } + @Override public EvaluationCell getCell(int rowIndex, int columnIndex) { RowColKey key = new RowColKey(rowIndex, columnIndex); diff --git a/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java b/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java index 3d8eb4f83d..5eb90c3417 100644 --- a/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java +++ b/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java @@ -162,7 +162,7 @@ public abstract class AggregateFunction extends MultiOperandNumericFunction { * @param func the function to wrap * @return wrapped instance. The actual math is delegated to the argument function. */ - /*package*/ static Function subtotalInstance(Function func) { + /*package*/ static Function subtotalInstance(Function func, boolean countHiddenRows) { final AggregateFunction arg = (AggregateFunction)func; return new AggregateFunction() { @Override @@ -178,6 +178,9 @@ public abstract class AggregateFunction extends MultiOperandNumericFunction { return false; } + public boolean isHiddenRowCounted() { + return countHiddenRows; + } }; } diff --git a/src/java/org/apache/poi/ss/formula/functions/Count.java b/src/java/org/apache/poi/ss/formula/functions/Count.java index b8de432edc..25c0799054 100644 --- a/src/java/org/apache/poi/ss/formula/functions/Count.java +++ b/src/java/org/apache/poi/ss/formula/functions/Count.java @@ -86,6 +86,9 @@ public final class Count implements Function { } }; + /** + * matches hidden rows but not subtotals + */ private static final I_MatchPredicate subtotalPredicate = new I_MatchAreaPredicate() { public boolean matches(ValueEval valueEval) { return defaultPredicate.matches(valueEval); @@ -99,16 +102,34 @@ public final class Count implements Function { } }; + /** + * matches nither hidden rows or subtotals + */ + private static final I_MatchPredicate subtotalVisibleOnlyPredicate = new I_MatchAreaPredicate() { + public boolean matches(ValueEval valueEval) { + return defaultPredicate.matches(valueEval); + } + + /** + * don't count cells that are subtotals + */ + public boolean matches(TwoDEval areEval, int rowIndex, int columnIndex) { + return !areEval.isSubTotal(rowIndex, columnIndex) && !areEval.isRowHidden(rowIndex); + } + }; + /** * Create an instance of Count to use in {@link Subtotal} *

* If there are other subtotals within argument refs (or nested subtotals), * these nested subtotals are ignored to avoid double counting. *

+ * @param includeHiddenRows true to include hidden rows in the aggregate, false to skip them + * @return function * * @see Subtotal */ - public static Count subtotalInstance() { - return new Count(subtotalPredicate ); + public static Count subtotalInstance(boolean includeHiddenRows) { + return new Count(includeHiddenRows ? subtotalPredicate : subtotalVisibleOnlyPredicate); } } diff --git a/src/java/org/apache/poi/ss/formula/functions/Counta.java b/src/java/org/apache/poi/ss/formula/functions/Counta.java index 5a106be0a9..c725f012a2 100644 --- a/src/java/org/apache/poi/ss/formula/functions/Counta.java +++ b/src/java/org/apache/poi/ss/formula/functions/Counta.java @@ -80,7 +80,8 @@ public final class Counta implements Function { return true; } }; - private static final I_MatchPredicate subtotalPredicate = new I_MatchAreaPredicate() { + + private static final I_MatchPredicate subtotalPredicate = new I_MatchAreaPredicate() { public boolean matches(ValueEval valueEval) { return defaultPredicate.matches(valueEval); } @@ -93,8 +94,21 @@ public final class Counta implements Function { } }; - public static Counta subtotalInstance() { - return new Counta(subtotalPredicate); + private static final I_MatchPredicate subtotalVisibleOnlyPredicate = new I_MatchAreaPredicate() { + public boolean matches(ValueEval valueEval) { + return defaultPredicate.matches(valueEval); + } + + /** + * don't count cells in rows that are hidden or subtotal cells + */ + public boolean matches(TwoDEval areEval, int rowIndex, int columnIndex) { + return !areEval.isSubTotal(rowIndex, columnIndex) && ! areEval.isRowHidden(rowIndex); + } + }; + + public static Counta subtotalInstance(boolean includeHiddenRows) { + return new Counta(includeHiddenRows ? subtotalPredicate : subtotalVisibleOnlyPredicate); } } diff --git a/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java b/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java index 7ec5a5fcf0..89e103ba43 100644 --- a/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java +++ b/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java @@ -152,6 +152,14 @@ public abstract class MultiOperandNumericFunction implements Function { return true; } + /** + * @return true if values in hidden rows are counted + * @see Subtotal + */ + public boolean isHiddenRowCounted() { + return true; + } + /** * Collects values from a single argument */ @@ -165,6 +173,7 @@ public abstract class MultiOperandNumericFunction implements Function { for (int rcIx = 0; rcIx < width; rcIx++) { ValueEval ve = ae.getValue(sIx, rrIx, rcIx); if (!isSubtotalCounted() && ae.isSubTotal(rrIx, rcIx)) continue; + if (!isHiddenRowCounted() && ae.isRowHidden(rrIx)) continue; collectValue(ve, true, temp); } } diff --git a/src/java/org/apache/poi/ss/formula/functions/Subtotal.java b/src/java/org/apache/poi/ss/formula/functions/Subtotal.java index 9b5bff897f..8727353ae2 100644 --- a/src/java/org/apache/poi/ss/formula/functions/Subtotal.java +++ b/src/java/org/apache/poi/ss/formula/functions/Subtotal.java @@ -56,7 +56,17 @@ import java.util.List; * 9SUM * 10VAR * * 11VARP * - * 101-111* + * 101AVERAGE + * 102COUNT + * 103COUNTA + * 104MAX + * 105MIN + * 106PRODUCT + * 107STDEV + * 108STDEVP * + * 109SUM + * 110VAR * + * 111VARP * *
* * Not implemented in POI yet. Functions 101-111 are the same as functions 1-11 but with * the option 'ignore hidden values'. @@ -68,20 +78,28 @@ public class Subtotal implements Function { private static Function findFunction(int functionCode) throws EvaluationException { switch (functionCode) { - case 1: return subtotalInstance(AggregateFunction.AVERAGE); - case 2: return Count.subtotalInstance(); - case 3: return Counta.subtotalInstance(); - case 4: return subtotalInstance(AggregateFunction.MAX); - case 5: return subtotalInstance(AggregateFunction.MIN); - case 6: return subtotalInstance(AggregateFunction.PRODUCT); - case 7: return subtotalInstance(AggregateFunction.STDEV); + case 1: return subtotalInstance(AggregateFunction.AVERAGE, true); + case 2: return Count.subtotalInstance(true); + case 3: return Counta.subtotalInstance(true); + case 4: return subtotalInstance(AggregateFunction.MAX, true); + case 5: return subtotalInstance(AggregateFunction.MIN, true); + case 6: return subtotalInstance(AggregateFunction.PRODUCT, true); + case 7: return subtotalInstance(AggregateFunction.STDEV, true); case 8: throw new NotImplementedFunctionException("STDEVP"); - case 9: return subtotalInstance(AggregateFunction.SUM); + case 9: return subtotalInstance(AggregateFunction.SUM, true); case 10: throw new NotImplementedFunctionException("VAR"); case 11: throw new NotImplementedFunctionException("VARP"); - } - if (functionCode > 100 && functionCode < 112) { - throw new NotImplementedException("SUBTOTAL - with 'exclude hidden values' option"); + case 101: return subtotalInstance(AggregateFunction.AVERAGE, false); + case 102: return Count.subtotalInstance(false); + case 103: return Counta.subtotalInstance(false); + case 104: return subtotalInstance(AggregateFunction.MAX, false); + case 105: return subtotalInstance(AggregateFunction.MIN, false); + case 106: return subtotalInstance(AggregateFunction.PRODUCT, false); + case 107: return subtotalInstance(AggregateFunction.STDEV, false); + case 108: throw new NotImplementedFunctionException("STDEVP SUBTOTAL with 'exclude hidden values' option"); + case 109: return subtotalInstance(AggregateFunction.SUM, false); + case 110: throw new NotImplementedFunctionException("VAR SUBTOTAL with 'exclude hidden values' option"); + case 111: throw new NotImplementedFunctionException("VARP SUBTOTAL with 'exclude hidden values' option"); } throw EvaluationException.invalidValue(); } @@ -93,9 +111,10 @@ public class Subtotal implements Function { } final Function innerFunc; + int functionCode = 0; try { ValueEval ve = OperandResolver.getSingleValue(args[0], srcRowIndex, srcColumnIndex); - int functionCode = OperandResolver.coerceValueToInt(ve); + functionCode = OperandResolver.coerceValueToInt(ve); innerFunc = findFunction(functionCode); } catch (EvaluationException e) { return e.getErrorEval(); @@ -116,6 +135,9 @@ public class Subtotal implements Function { if(lazyRefEval.isSubTotal()) { it.remove(); } + if (functionCode > 100 && lazyRefEval.isRowHidden()) { + it.remove(); + } } } diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java index bb1d1f1698..21e2dab8be 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java @@ -45,6 +45,16 @@ final class SXSSFEvaluationSheet implements EvaluationSheet { return _xs.getLastRowNum(); } + /* (non-Javadoc) + * @see org.apache.poi.ss.formula.EvaluationSheet#isRowHidden(int) + * @since POI 4.0.2 + */ + public boolean isRowHidden(int rowIndex) { + SXSSFRow row = _xs.getRow(rowIndex); + if (row == null) return false; + return row.getZeroHeight(); + } + @Override public EvaluationCell getCell(int rowIndex, int columnIndex) { SXSSFRow row = _xs.getRow(rowIndex); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java index 74ac23215a..3d71f08825 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java @@ -52,6 +52,16 @@ final class XSSFEvaluationSheet implements EvaluationSheet { return _xs.getLastRowNum(); } + /* (non-Javadoc) + * @see org.apache.poi.ss.formula.EvaluationSheet#isRowHidden(int) + * @since POI 4.0.2 + */ + public boolean isRowHidden(int rowIndex) { + final XSSFRow row = _xs.getRow(rowIndex); + if (row == null) return false; + return row.getZeroHeight(); + } + /* (non-JavaDoc), inherit JavaDoc from EvaluationWorkbook * @since POI 3.15 beta 3 */ diff --git a/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java b/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java index 41ca4b8e98..aa16314379 100644 --- a/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java +++ b/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java @@ -376,6 +376,15 @@ public final class TestSubtotal { confirmExpectedResult(evaluator, "SUBTOTAL(COUNT;B2:B8,C2:C8)", cellC2, 3.0); confirmExpectedResult(evaluator, "SUBTOTAL(COUNTA;B2:B8,C2:C8)", cellC3, 5.0); + // test same functions ignoring hidden rows over a copy of the same data + cellC1 = sheet.getRow(11).getCell(3); + cellC2 = sheet.getRow(12).getCell(3); + cellC3 = sheet.getRow(13).getCell(3); + confirmExpectedResult(evaluator, "SUBTOTAL(SUM NO HIDDEN;B22:B28;C22:C28)", cellC1, 17.0); + confirmExpectedResult(evaluator, "SUBTOTAL(COUNT NO HIDDEN;B22:B28,C22:C28)", cellC2, 2.0); + confirmExpectedResult(evaluator, "SUBTOTAL(COUNTA NO HIDDEN;B22:B28,C22:C28)", cellC3, 4.0); + + workbook.close(); } @@ -393,7 +402,6 @@ public final class TestSubtotal { { "SUBTOTAL(8,B2:B3)", NotImplementedException.class.getName() }, { "SUBTOTAL(10,B2:B3)", NotImplementedException.class.getName() }, { "SUBTOTAL(11,B2:B3)", NotImplementedException.class.getName() }, - { "SUBTOTAL(107,B2:B3)", NotImplementedException.class.getName() }, { "SUBTOTAL(0,B2:B3)", null }, { "SUBTOTAL(9)", FormulaParseException.class.getName() }, { "SUBTOTAL()", FormulaParseException.class.getName() }, @@ -404,7 +412,7 @@ public final class TestSubtotal { try { a3.setCellFormula(f[0]); fe.evaluateAll(); - assertEquals(FormulaError.VALUE.getCode(), a3.getErrorCellValue()); + assertEquals(f[0], FormulaError.VALUE.getCode(), a3.getErrorCellValue()); } catch (Exception e) { actualEx = e; } diff --git a/test-data/spreadsheet/SubtotalsNested.xls b/test-data/spreadsheet/SubtotalsNested.xls index a19571d849..830aa5c34c 100644 Binary files a/test-data/spreadsheet/SubtotalsNested.xls and b/test-data/spreadsheet/SubtotalsNested.xls differ