From: Josh Micich Date: Fri, 15 May 2009 21:36:25 +0000 (+0000) Subject: Fix for bug noticed while investigating bugzilla 47048. Made INDEX() support missing... X-Git-Tag: REL_3_5_BETA6~37 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=07c12e09f0f74dc649bb84ded2eae12809170010;p=poi.git Fix for bug noticed while investigating bugzilla 47048. Made INDEX() support missing args. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@775360 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/hssf/record/formula/functions/Index.java b/src/java/org/apache/poi/hssf/record/formula/functions/Index.java index 1c34322256..371a06e30a 100644 --- a/src/java/org/apache/poi/hssf/record/formula/functions/Index.java +++ b/src/java/org/apache/poi/hssf/record/formula/functions/Index.java @@ -18,9 +18,11 @@ package org.apache.poi.hssf.record.formula.functions; import org.apache.poi.hssf.record.formula.eval.AreaEval; +import org.apache.poi.hssf.record.formula.eval.BlankEval; import org.apache.poi.hssf.record.formula.eval.ErrorEval; import org.apache.poi.hssf.record.formula.eval.Eval; import org.apache.poi.hssf.record.formula.eval.EvaluationException; +import org.apache.poi.hssf.record.formula.eval.MissingArgEval; import org.apache.poi.hssf.record.formula.eval.OperandResolver; import org.apache.poi.hssf.record.formula.eval.RefEval; import org.apache.poi.hssf.record.formula.eval.ValueEval; @@ -28,7 +30,7 @@ import org.apache.poi.hssf.record.formula.eval.ValueEval; /** * Implementation for the Excel function INDEX *

- * + * * Syntax :
* INDEX ( reference, row_num[, column_num [, area_num]])
* INDEX ( array, row_num[, column_num]) @@ -40,12 +42,11 @@ import org.apache.poi.hssf.record.formula.eval.ValueEval; * area_numused when reference is a union of areas * *

- * + * * @author Josh Micich */ public final class Index implements Function { - // TODO - javadoc for interface method public Eval evaluate(Eval[] args, int srcCellRow, short srcCellCol) { int nArgs = args.length; if(nArgs < 2) { @@ -58,112 +59,141 @@ public final class Index implements Function { firstArg = ((RefEval)firstArg).offset(0, 0, 0, 0); } if(!(firstArg instanceof AreaEval)) { - + // else the other variation of this function takes an array as the first argument // it seems like interface 'ArrayEval' does not even exist yet - + throw new RuntimeException("Incomplete code - cannot handle first arg of type (" + firstArg.getClass().getName() + ")"); } AreaEval reference = (AreaEval) firstArg; - + int rowIx = 0; int columnIx = 0; - int areaIx = 0; - try { + boolean colArgWasPassed = false; + try { switch(nArgs) { case 4: - areaIx = convertIndexArgToZeroBase(args[3], srcCellRow, srcCellCol); throw new RuntimeException("Incomplete code" + " - don't know how to support the 'area_num' parameter yet)"); // Excel expression might look like this "INDEX( (A1:B4, C3:D6, D2:E5 ), 1, 2, 3) // In this example, the 3rd area would be used i.e. D2:E5, and the overall result would be E2 // Token array might be encoded like this: MemAreaPtg, AreaPtg, AreaPtg, UnionPtg, UnionPtg, ParenthesesPtg // The formula parser doesn't seem to support this yet. Not sure if the evaluator does either - + case 3: - columnIx = convertIndexArgToZeroBase(args[2], srcCellRow, srcCellCol); + columnIx = resolveIndexArg(args[2], srcCellRow, srcCellCol); + colArgWasPassed = true; case 2: - rowIx = convertIndexArgToZeroBase(args[1], srcCellRow, srcCellCol); + rowIx = resolveIndexArg(args[1], srcCellRow, srcCellCol); break; default: // too many arguments return ErrorEval.VALUE_INVALID; } - return getValueFromArea(reference, rowIx, columnIx, nArgs); + return getValueFromArea(reference, rowIx, columnIx, colArgWasPassed, srcCellRow, srcCellCol); } catch (EvaluationException e) { return e.getErrorEval(); } } - + /** - * @param nArgs - needed because error codes are slightly different when only 2 args are passed + * @param colArgWasPassed false if the INDEX argument list had just 2 items + * (exactly 1 comma). If anything is passed for the column_num argument + * (including {@link BlankEval} or {@link MissingArgEval}) this parameter will be + * true. This parameter is needed because error codes are slightly + * different when only 2 args are passed. */ - private static ValueEval getValueFromArea(AreaEval ae, int pRowIx, int pColumnIx, int nArgs) throws EvaluationException { + private static ValueEval getValueFromArea(AreaEval ae, int pRowIx, int pColumnIx, + boolean colArgWasPassed, int srcRowIx, int srcColIx) throws EvaluationException { + boolean rowArgWasEmpty = pRowIx == 0; + boolean colArgWasEmpty = pColumnIx == 0; int rowIx; int columnIx; - + // when the area ref is a single row or a single column, // there are special rules for conversion of rowIx and columnIx if (ae.isRow()) { if (ae.isColumn()) { - rowIx = pRowIx == -1 ? 0 : pRowIx; - columnIx = pColumnIx == -1 ? 0 : pColumnIx; + // single cell ref + rowIx = rowArgWasEmpty ? 0 : pRowIx-1; + columnIx = colArgWasEmpty ? 0 : pColumnIx-1; } else { - if (nArgs == 2) { - rowIx = 0; - columnIx = pRowIx; + if (colArgWasPassed) { + rowIx = rowArgWasEmpty ? 0 : pRowIx-1; + columnIx = pColumnIx-1; } else { - rowIx = pRowIx == -1 ? 0 : pRowIx; - columnIx = pColumnIx; + // special case - row arg seems to get used as the column index + rowIx = 0; + // transfer both the index value and the empty flag from 'row' to 'column': + columnIx = pRowIx-1; + colArgWasEmpty = rowArgWasEmpty; } } - if (rowIx < -1 || columnIx < -1) { - throw new EvaluationException(ErrorEval.VALUE_INVALID); - } } else if (ae.isColumn()) { - if (nArgs == 2) { - rowIx = pRowIx; - columnIx = 0; + if (rowArgWasEmpty) { + rowIx = srcRowIx - ae.getFirstRow(); } else { - rowIx = pRowIx; - columnIx = pColumnIx == -1 ? 0 : pColumnIx; + rowIx = pRowIx-1; } - if (rowIx < -1 || columnIx < -1) { - throw new EvaluationException(ErrorEval.VALUE_INVALID); + if (colArgWasEmpty) { + columnIx = 0; + } else { + columnIx = colArgWasEmpty ? 0 : pColumnIx-1; } } else { - if (nArgs == 2) { + // ae is an area (not single row or column) + if (!colArgWasPassed) { // always an error with 2-D area refs - if (pRowIx < -1) { - throw new EvaluationException(ErrorEval.VALUE_INVALID); - } - throw new EvaluationException(ErrorEval.REF_INVALID); + // Note - the type of error changes if the pRowArg is negative + throw new EvaluationException(pRowIx < 0 ? ErrorEval.VALUE_INVALID : ErrorEval.REF_INVALID); } // Normal case - area ref is 2-D, and both index args were provided - rowIx = pRowIx; - columnIx = pColumnIx; + // if either arg is missing (or blank) the logic is similar to OperandResolver.getSingleValue() + if (rowArgWasEmpty) { + rowIx = srcRowIx - ae.getFirstRow(); + } else { + rowIx = pRowIx-1; + } + if (colArgWasEmpty) { + columnIx = srcColIx - ae.getFirstColumn(); + } else { + columnIx = pColumnIx-1; + } } - + int width = ae.getWidth(); int height = ae.getHeight(); // Slightly irregular logic for bounds checking errors - if (rowIx >= height || columnIx >= width) { + if (!rowArgWasEmpty && rowIx >= height || !colArgWasEmpty && columnIx >= width) { + // high bounds check fail gives #REF! if arg was explicitly passed throw new EvaluationException(ErrorEval.REF_INVALID); } - if (rowIx < 0 || columnIx < 0) { + if (rowIx < 0 || columnIx < 0 || rowIx >= height || columnIx >= width) { throw new EvaluationException(ErrorEval.VALUE_INVALID); } return ae.getRelativeValue(rowIx, columnIx); } + /** - * takes a NumberEval representing a 1-based index and returns the zero-based int value + * @param arg a 1-based index. + * @return the resolved 1-based index. Zero if the arg was missing or blank + * @throws EvaluationException if the arg is an error value evaluates to a negative numeric value */ - private static int convertIndexArgToZeroBase(Eval arg, int srcCellRow, short srcCellCol) throws EvaluationException { - + private static int resolveIndexArg(Eval arg, int srcCellRow, short srcCellCol) throws EvaluationException { + ValueEval ev = OperandResolver.getSingleValue(arg, srcCellRow, srcCellCol); - int oneBasedVal = OperandResolver.coerceValueToInt(ev); - return oneBasedVal - 1; + if (ev == MissingArgEval.instance) { + return 0; + } + if (ev == BlankEval.INSTANCE) { + return 0; + } + int result = OperandResolver.coerceValueToInt(ev); + if (result < 0) { + throw new EvaluationException(ErrorEval.VALUE_INVALID); + } + return result; } } diff --git a/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls b/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls index 9dcde61772..bf0b23accb 100644 Binary files a/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls and b/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls differ diff --git a/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java index 95355733d2..764e30cd99 100755 --- a/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java @@ -17,20 +17,28 @@ package org.apache.poi.hssf.record.formula.functions; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.record.formula.eval.AreaEval; import org.apache.poi.hssf.record.formula.eval.Eval; +import org.apache.poi.hssf.record.formula.eval.MissingArgEval; import org.apache.poi.hssf.record.formula.eval.NumberEval; import org.apache.poi.hssf.record.formula.eval.ValueEval; /** - * Tests for the INDEX() function - * + * Tests for the INDEX() function.

+ * + * This class contains just a few specific cases that directly invoke {@link Index}, + * with minimum overhead.
+ * Another test: {@link TestIndexFunctionFromSpreadsheet} operates from a higher level + * and has far greater coverage of input permutations.
+ * * @author Josh Micich */ public final class TestIndex extends TestCase { + private static final Index FUNC_INST = new Index(); private static final double[] TEST_VALUES0 = { 1, 2, 3, 4, @@ -39,44 +47,69 @@ public final class TestIndex extends TestCase { 9, 10, 11, 12, }; - + /** * For the case when the first argument to INDEX() is an area reference */ public void testEvaluateAreaReference() { - + double[] values = TEST_VALUES0; confirmAreaEval("C1:D6", values, 4, 1, 7); confirmAreaEval("C1:D6", values, 6, 2, 12); confirmAreaEval("C1:D6", values, 3, 1, 5); - + // now treat same data as 3 columns, 4 rows - confirmAreaEval("C10:E13", values, 2, 2, 5); + confirmAreaEval("C10:E13", values, 2, 2, 5); confirmAreaEval("C10:E13", values, 4, 1, 10); } - + /** * @param areaRefString in Excel notation e.g. 'D2:E97' * @param dValues array of evaluated values for the area reference * @param rowNum 1-based * @param colNum 1-based, pass -1 to signify argument not present */ - private static void confirmAreaEval(String areaRefString, double[] dValues, + private static void confirmAreaEval(String areaRefString, double[] dValues, int rowNum, int colNum, double expectedResult) { ValueEval[] values = new ValueEval[dValues.length]; for (int i = 0; i < values.length; i++) { values[i] = new NumberEval(dValues[i]); } AreaEval arg0 = EvalFactory.createAreaEval(areaRefString, values); - + Eval[] args; if (colNum > 0) { args = new Eval[] { arg0, new NumberEval(rowNum), new NumberEval(colNum), }; } else { args = new Eval[] { arg0, new NumberEval(rowNum), }; } - - double actual = NumericFunctionInvoker.invoke(new Index(), args); + + double actual = NumericFunctionInvoker.invoke(FUNC_INST, args); assertEquals(expectedResult, actual, 0D); } + + /** + * Tests expressions like "INDEX(A1:C1,,2)".
+ * This problem was found while fixing bug 47048 and is observable up to svn r773441. + */ + public void testMissingArg() { + ValueEval[] values = { + new NumberEval(25.0), + new NumberEval(26.0), + new NumberEval(28.0), + }; + AreaEval arg0 = EvalFactory.createAreaEval("A10:C10", values); + Eval[] args = new Eval[] { arg0, MissingArgEval.instance, new NumberEval(2), }; + Eval actualResult; + try { + actualResult = FUNC_INST.evaluate(args, 1, (short)1); + } catch (RuntimeException e) { + if (e.getMessage().equals("Unexpected arg eval type (org.apache.poi.hssf.record.formula.eval.MissingArgEval")) { + throw new AssertionFailedError("Identified bug 47048b - INDEX() should support missing-arg"); + } + throw e; + } + assertEquals(NumberEval.class, actualResult.getClass()); + assertEquals(26.0, ((NumberEval)actualResult).getNumberValue(), 0.0); + } }