From 798d7e0098da1fa2a4cf3c2b85ac364680670756 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Wed, 1 Apr 2009 19:41:12 +0000 Subject: [PATCH] Fix for bug 46948 - Range operator can now take area-refs for operands git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@761023 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 1 + .../hssf/record/formula/eval/RangeEval.java | 27 ++-- .../record/formula/eval/TestRangeEval.java | 120 ++++++++++++++++-- 4 files changed, 131 insertions(+), 18 deletions(-) diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 52f96225aa..c9f1eab99c 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 e6cc63338a..65923e12c2 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 938241c281..07b143a6d6 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 e98517a732..d30a7b1d38 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); } } -- 2.39.5