From 07c12e09f0f74dc649bb84ded2eae12809170010 Mon Sep 17 00:00:00 2001
From: Josh Micich
Date: Fri, 15 May 2009 21:36:25 +0000
Subject: [PATCH] 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
---
.../hssf/record/formula/functions/Index.java | 128 +++++++++++-------
.../hssf/data/IndexFunctionTestCaseData.xls | Bin 24576 -> 26112 bytes
.../record/formula/functions/TestIndex.java | 55 ++++++--
3 files changed, 123 insertions(+), 60 deletions(-)
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_num | used 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 9dcde6177295a38037d74dbcf7ee9c5774c44086..bf0b23accb90886870920c5b5416b33f320a4216 100644
GIT binary patch
delta 4815
zcmZ`-Yiu0V6+ScGr|s+}v1>cnY1SAQQs?D5PuGrj9p~ZLC5efXC{>y`*|oiXE%w?J
z+A14B1y!UXbE
z%f_P_?|k#!@0|0^Ju`RrLf*P?-MZ9cZF;r$eaq@pe*1-QbP&-7>>IIvRVWmA-?IFw
zy~n!jORie|yRLVliEGxGFTQ-@xsR_|o7IxXP(!UnVY+G~C7W2Kq1L8G
zx@2kKaLAFZk8H3k+F8o1S1*NDI3nu0mTOWsLONFUR-+ORHdXhOo^seeBX%a-+V*zP
z+CyVBOJ`|;()iR#-=x2jMIpn3j=CB~faUkXAFQ|jz5HZXl@w)FJ?ijUeZ=P9ORVWY*l3h^U
z-9OvaOjYnZijm1b{F(}d5ao$(`N?AVs&6eWhsm}nSVwILN5hygc^nGri|&V7n#rd}
zw~VoHh}8=k^$$=GuhVc8IX%GN-2q8l_-P;ZV03kFY
zKUP3aZVs;q0Tp-@RvN;s-fb4CuHA=ld~|mL$9Lrz?Bmaaa(tq%Z)-C(l!>f%L>Rv6
zYr^Gb_9+mdwkqwD`d42o(rYXm*i;;-I^%0W0D$y}exmQd8bip&?*;*Y5N>M19ft6h
z5C90_C2cZNVF!?#s!6Z{0CB*z2v=Of@kB3X!8)z*XRUywnZPj5f_1c+t6POx;8y3%
zf;SRfxn_##(O0fJbEwsf4%$ARoD;XG8nh~85k#qE$2wciX5DLB)hGwM%yvlTiN{*bD*yj|pvt
zuwySzBmkoDtS;j&LwIa2D*zBey>_YH5Iz$E03poi9KK};JNK~y08b7uSo^G=+Q&KE
zT}HUa5Iz?I0FS~3L)g5Z71nD4PbqGPZyUmSApj5sm(XDd9|{40hj79WS`Kgs#Pkq0
z8p5;?0C)(S4B-tS0PqkngIpiNgB$|$%|keD2qy+R(2re0AUdjV7K~#BhPfZR@Q%>^
z$nO+>ZmAasJD|K(53E1yJhHYG2de)JaM*gqSAmFocUj03gY`
z?S@ozZtBy6-0}cC)gfz_aK}&=`mslc_(0DzB+UedxgUGzAXgVfKf2X9{g@r3QmhUr!^}|d6;He;P2#*Z&{Q)2f_iKfOA^d5$IWdjqZn1~Z
z&_l90t?fNV=yb8McS&P2HzDnvQ1ktD%kl5EaA!kXVjb48`c16I4so^sgs?>u_87uv
zLIB_?2R>>$)y~6Q&VVMkJ+api9v1=tj{+=q75*ZGePs&!4Wa!AhX5c7ZioYhuqXro
z9s(NK4e^c;2Fr559B>J%M>qrkj{_wjuIfcQkU?>g^+BZhFl5C90_w>tiaAzTvz0I9(3
zocj#n&ZAr+01pAFbBXw;=7n%n6I`1|4IwWC0HWX$ju}F9ltTdU5Ky)o;s-*&v&fUf
zDMR>;5a6Om0S_}*;pXTz-xa(a)V5;>d`Dq}didDuxq!S2o#|iRBr_FTD1y5)jET?h
z!n3eEAn!tau%dz`D`2S#*f?XkvcVG-h?5ntbOmgx0ybR%n=Qd|Zq{=ppo?WHVDrHI
zWl1koz_Jyv#jj&IS}Fs1xx%(aWubB2uBIfGF+62%^ab#WO?vJy);GArb=01sajZ)G
z)Gz>gzQ1ZM`Rx$MyzYrwER*ucq(p{t^b2VS!fBZf_@blNQDdA~O7xHmIcGje%sunt
zjJubkjeDYvJJIMxS)CAcbg-P8HTb2`&awWuMB`rk8AszU1{`g|6AhN&8t=Rg
zA?TEq2D-K=7s6QM(PW9vMw%?sf%Sz(YERRYSUrG=na9D^3SoeeN_(WzI-R6;drC{?
zTrEuK`~b$3njn^^MH7{X328*;vfQ&$%cndM5kZ*$)L*=&%aTX84`aziteksHx40D5T|V%%KOIG!OHEl6>0qKp>2u}0qyMBpwmiWxOORF}($
z2G40uk)s*&Gh&5%jh_oqtmB>Z7d+`Nlo^BQ94t$V5cHvCK`)xJAR(QW&1F40S)KKR
zvhXDQNKmE&`qym87H!}Xvc(eElttGRVpA5i{2493L@VvDJb5mPDT^LcxCYbqOP++6
z$_hsT-oimI7QUnl{{&xECIduqD}t5FEVli456xp6!cy%Bt(wKZ{aC@Brrq{xdObku
z!Q=Nt;5`2LcJQBi<@m_+r>x7T9)9h_7Tfxj>O9%DhF>JL*e_sTf&C)(I_yuY*^|vH
b_=@|h5~-h`+~A)%fS;k%n6aesZZ{K4K-Jond+`4s-?&<8tCTxAJ+qy^Abq~fOYC#f$1`>%R5(rKN
z6A}_8|BztRh++&FKPU}+FlZK09)XF87{6%Z2TjmVd@(8s%yZ5?EC0L1G`DMaem(d6
zU#Hv5En{ZRnC&teKJUI}7|qIMp7lLkSXiK|Dfgw>W4x6uJn7u=cfzN
ze#D=RYM=8Bb3UW)I}aJx)qd9rCkpXXBbmB+`i
zQuV96@}KlB<6rp;4e-Hb697!0EdD|hR>LgbfPXRSwbHIEYBUV!)a6Qb&b!=7qHamL
zQgzQO_bO<6lqKdVE$GtU(ru%Ul4X5?s{02f!RY8ESIV{`wF&g`O_9D1J0g(a$BFV9X^P>(!r|_91r(AN|(4saxYN;x6F_
zx`<;K3$co!LOdFG6L$+Q?0yM3hHc!daSw5iaMfLn9K+049p=6z
z*&oxmkGN0xsz|$0p@t%9e10bq$LC-8xjag*xAF7YXfzN6|B{r&n6e0}aE>AA2T=Z4
zU}q3FrhQnYn!?)W66ShvnO<}b9M1~lkXZnwn1Yt@Uepaid}qz!Wm)SoEdxr*ppM6C
ztAl_M{#@sG)&i~3WSPj@EHT3J&J|0p)#MtH#c|4s5tMYmta-*!=SR6#)n2=V*J-?#c&+f)nPb?Ny;0+J#Os8A%{(NW)|n1yy~abt
zL&6>XR1AhW`yy}9cs=oY;YsEgw(%_*Zy??v{3vs*)yA7N-blPr_*Lc@w((|-HxX|V
z?nzPY7`AcT-uZcNCf+PO#k^$+pVD{>@fP97QVrN=!ps3aEwMHME*qUd`u-W~z;+em=8x~-Cmi-cY4=T{ID9H{ke!+oz?CV)b99U
zq;qA%?bP+~wp_+5(w7oi8F4BjE>*ucLW9IG^KWEN^k_UmJR$rda}3+&1eYqmG(E(7
zEPgUg$0I3v@o4vII!QVy$>Yp1Y)g)dn@`?LyjS>P=6y@D_iMb5c%Seq%rVT_7mG=0
zyq|c#aOWu1j$s?eu9hz*MLZ?Em-)bw+6TdWxVUErhz|&VlR1WM*>NoS?1RJygUM4_Dynx_z0eea3+j(r-jD&@BoZzG)*)unGa`Z{V|M%;9nfP{s0+`
zj}jjhKF=J(%y;;yB>{|SJVQJqymXARW0;3&bq9cPjgJu@6aK;&%{hi`9EVM_!owUV
zJ}&$t<`crL6$O~o_yq9@;lDG-FlS%nn>9X3d{TJLI2D6o8^?*w7qgl8X5l-TV=uSW
z{;bBQh))SW#~i=5Y}xUvA)kE<@h!st7;ksp!nIVD6Dj8rTz|D^;!=DW|8YcT-`*Y4
z`HrwOV)cy#M9gBi$SKZnX}jefYz^&fMD+vflP
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);
+ }
}
--
2.39.5