]> source.dussan.org Git - poi.git/commitdiff
Fix for bug noticed while investigating bugzilla 47048. Made INDEX() support missing...
authorJosh Micich <josh@apache.org>
Fri, 15 May 2009 21:36:25 +0000 (21:36 +0000)
committerJosh Micich <josh@apache.org>
Fri, 15 May 2009 21:36:25 +0000 (21:36 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@775360 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/formula/functions/Index.java
src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls
src/testcases/org/apache/poi/hssf/record/formula/functions/TestIndex.java

index 1c343222561294981fc88348189fc5535dc36339..371a06e30ad7824244b77d5b8f2a7ec4e41dbef1 100644 (file)
 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
  * <p>
- * 
+ *
  * Syntax : <br/>
  *  INDEX ( reference, row_num[, column_num [, area_num]])</br>
  *  INDEX ( array, row_num[, column_num])
@@ -40,12 +42,11 @@ import org.apache.poi.hssf.record.formula.eval.ValueEval;
  *      <tr><th>area_num</th><td>used when reference is a union of areas</td></tr>
  *    </table>
  * </p>
- * 
+ *
  * @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 <code>false</code> if the INDEX argument list had just 2 items
+        *            (exactly 1 comma).  If anything is passed for the <tt>column_num</tt> argument
+        *            (including {@link BlankEval} or {@link MissingArgEval}) this parameter will be
+        *            <code>true</code>.  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;
        }
 }
index 9dcde6177295a38037d74dbcf7ee9c5774c44086..bf0b23accb90886870920c5b5416b33f320a4216 100644 (file)
Binary files a/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls and b/src/testcases/org/apache/poi/hssf/data/IndexFunctionTestCaseData.xls differ
index 95355733d245ee5f3b019dfb8b3097f15c7f7274..764e30cd997601cfebc804f74513fd43dc4de650 100755 (executable)
 
 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.</p>
+ *
+ * This class contains just a few specific cases that directly invoke {@link Index},
+ * with minimum overhead.<br/>
+ * Another test: {@link TestIndexFunctionFromSpreadsheet} operates from a higher level
+ * and has far greater coverage of input permutations.<br/>
+ *
  * @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)".<br/>
+        * 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);
+       }
 }