diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 52f96225a..c9f1eab99 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 46948 - Fixed evaluation of range operator to allow for area-ref operands 46918 - Fixed ExtendedPivotTableViewFieldsRecord(SXVDEX) to allow shorter format 46898 - Fixed formula evaluator to not cache intermediate circular-reference error results 46917 - Fixed PageItemRecord(SXPI) to allow multiple field infos diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index e6cc63338..65923e12c 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 46948 - Fixed evaluation of range operator to allow for area-ref operands 46918 - Fixed ExtendedPivotTableViewFieldsRecord(SXVDEX) to allow shorter format 46898 - Fixed formula evaluator to not cache intermediate circular-reference error results 46917 - Fixed PageItemRecord(SXPI) to allow multiple field infos diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/RangeEval.java b/src/java/org/apache/poi/hssf/record/formula/eval/RangeEval.java index 938241c28..07b143a6d 100644 --- a/src/java/org/apache/poi/hssf/record/formula/eval/RangeEval.java +++ b/src/java/org/apache/poi/hssf/record/formula/eval/RangeEval.java @@ -36,25 +36,36 @@ public final class RangeEval implements OperationEval { } try { - RefEval reA = evaluateRef(args[0]); - RefEval reB = evaluateRef(args[1]); + AreaEval reA = evaluateRef(args[0]); + AreaEval reB = evaluateRef(args[1]); return resolveRange(reA, reB); } catch (EvaluationException e) { return e.getErrorEval(); } } - private static AreaEval resolveRange(RefEval reA, RefEval reB) { + /** + * @return simple rectangular {@link AreaEval} which fully encloses both areas + * aeA and aeB + */ + private static AreaEval resolveRange(AreaEval aeA, AreaEval aeB) { + int aeAfr = aeA.getFirstRow(); + int aeAfc = aeA.getFirstColumn(); - int height = reB.getRow() - reA.getRow(); - int width = reB.getColumn() - reA.getColumn(); + int top = Math.min(aeAfr, aeB.getFirstRow()); + int bottom = Math.max(aeA.getLastRow(), aeB.getLastRow()); + int left = Math.min(aeAfc, aeB.getFirstColumn()); + int right = Math.max(aeA.getLastColumn(), aeB.getLastColumn()); - return reA.offset(0, height, 0, width); + return aeA.offset(top-aeAfr, bottom-aeAfr, left-aeAfc, right-aeAfc); } - private static RefEval evaluateRef(Eval arg) throws EvaluationException { + private static AreaEval evaluateRef(Eval arg) throws EvaluationException { + if (arg instanceof AreaEval) { + return (AreaEval) arg; + } if (arg instanceof RefEval) { - return (RefEval) arg; + return ((RefEval) arg).offset(0, 0, 0, 0); } if (arg instanceof ErrorEval) { throw new EvaluationException((ErrorEval)arg); diff --git a/src/testcases/org/apache/poi/hssf/record/formula/eval/TestRangeEval.java b/src/testcases/org/apache/poi/hssf/record/formula/eval/TestRangeEval.java index e98517a73..d30a7b1d3 100644 --- a/src/testcases/org/apache/poi/hssf/record/formula/eval/TestRangeEval.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/eval/TestRangeEval.java @@ -17,12 +17,28 @@ package org.apache.poi.hssf.record.formula.eval; +import java.lang.reflect.Field; + +import junit.framework.AssertionFailedError; +import junit.framework.TestCase; + +import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate; import org.apache.poi.hssf.record.formula.AreaI; +import org.apache.poi.hssf.record.formula.AttrPtg; +import org.apache.poi.hssf.record.formula.FuncVarPtg; +import org.apache.poi.hssf.record.formula.IntPtg; +import org.apache.poi.hssf.record.formula.Ptg; +import org.apache.poi.hssf.record.formula.RangePtg; +import org.apache.poi.hssf.record.formula.RefPtg; import org.apache.poi.hssf.record.formula.AreaI.OffsetArea; +import org.apache.poi.hssf.usermodel.HSSFCell; +import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator; +import org.apache.poi.hssf.usermodel.HSSFRow; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hssf.util.AreaReference; import org.apache.poi.hssf.util.CellReference; - -import junit.framework.TestCase; +import org.apache.poi.ss.formula.FormulaParser; +import org.apache.poi.ss.usermodel.CellValue; /** * Test for unary plus operator evaluator. @@ -30,22 +46,22 @@ import junit.framework.TestCase; * @author Josh Micich */ public final class TestRangeEval extends TestCase { - + public void testPermutations() { - + confirm("B3", "D7", "B3:D7"); confirm("B1", "B1", "B1:B1"); - + confirm("B7", "D3", "B3:D7"); confirm("D3", "B7", "B3:D7"); confirm("D7", "B3", "B3:D7"); } private static void confirm(String refA, String refB, String expectedAreaRef) { - + Eval[] args = { - createRefEval(refA), - createRefEval(refB), + createRefEval(refA), + createRefEval(refB), }; AreaReference ar = new AreaReference(expectedAreaRef); Eval result = RangeEval.instance.evaluate(args, 0, (short)0); @@ -60,7 +76,7 @@ public final class TestRangeEval extends TestCase { private static Eval createRefEval(String refStr) { CellReference cr = new CellReference(refStr); return new MockRefEval(cr.getRow(), cr.getCol()); - + } private static final class MockRefEval extends RefEvalBase { @@ -89,7 +105,91 @@ public final class TestRangeEval extends TestCase { } public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) { - throw new RuntimeException("not expected to be called during this test"); + AreaI area = new OffsetArea(getFirstRow(), getFirstColumn(), + relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx); + + return new MockAreaEval(area); } } + + public void testRangeUsingOffsetFunc_bug46948() { + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFRow row = wb.createSheet("Sheet1").createRow(0); + HSSFCell cellA1 = row.createCell(0); + HSSFCell cellB1 = row.createCell(1); + row.createCell(2).setCellValue(5.0); // C1 + row.createCell(3).setCellValue(7.0); // D1 + row.createCell(4).setCellValue(9.0); // E1 + + + try { + cellA1.setCellFormula("SUM(C1:OFFSET(C1,0,B1))"); + } catch (RuntimeException e) { + // TODO fix formula parser to handle ':' as a proper operator + if (!e.getClass().getName().startsWith(FormulaParser.class.getName())) { + throw e; + } + // FormulaParseException is expected until the parser is fixed up + // Poke the formula in directly: + pokeInOffsetFormula(cellA1); + } + + + cellB1.setCellValue(1.0); // range will be C1:D1 + + HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb); + CellValue cv; + try { + cv = fe.evaluate(cellA1); + } catch (IllegalArgumentException e) { + if (e.getMessage().equals("Unexpected ref arg class (org.apache.poi.ss.formula.LazyAreaEval)")) { + throw new AssertionFailedError("Identified bug 46948"); + } + throw e; + } + + assertEquals(12.0, cv.getNumberValue(), 0.0); + + cellB1.setCellValue(2.0); // range will be C1:E1 + fe.notifyUpdateCell(cellB1); + cv = fe.evaluate(cellA1); + assertEquals(21.0, cv.getNumberValue(), 0.0); + + cellB1.setCellValue(0.0); // range will be C1:C1 + fe.notifyUpdateCell(cellB1); + cv = fe.evaluate(cellA1); + assertEquals(5.0, cv.getNumberValue(), 0.0); + } + + /** + * Directly sets the formula "SUM(C1:OFFSET(C1,0,B1))" in the specified cell. + * This hack can be removed when the formula parser can handle functions as + * operands to the range (:) operator. + * + */ + private static void pokeInOffsetFormula(HSSFCell cell) { + cell.setCellFormula("1"); + FormulaRecordAggregate fr; + try { + Field field = HSSFCell.class.getDeclaredField("_record"); + field.setAccessible(true); + fr = (FormulaRecordAggregate) field.get(cell); + } catch (IllegalArgumentException e) { + throw new RuntimeException(e); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } catch (NoSuchFieldException e) { + throw new RuntimeException(e); + } + Ptg[] ptgs = { + new RefPtg("C1"), + new RefPtg("C1"), + new IntPtg(0), + new RefPtg("B1"), + new FuncVarPtg("OFFSET", (byte)3), + RangePtg.instance, + AttrPtg.SUM, + }; + fr.setParsedExpression(ptgs); + } }