From d1fa09977ae393f91e9bd67ad52589ea89c837c8 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Sat, 6 Sep 2008 05:30:31 +0000 Subject: [PATCH] Fixes for special cases of lookup functions (test cases added) git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@692612 13f79535-47bb-0310-9956-ffa450edef68 --- .../record/formula/functions/Hlookup.java | 10 ++- .../record/formula/functions/LookupUtils.java | 61 +++++++++++------- .../record/formula/functions/Vlookup.java | 10 ++- .../hssf/data/LookupFunctionsTestCaseData.xls | Bin 39936 -> 41984 bytes 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/src/java/org/apache/poi/hssf/record/formula/functions/Hlookup.java b/src/java/org/apache/poi/hssf/record/formula/functions/Hlookup.java index e6efaaee4d..2bbf6f5cd5 100644 --- a/src/java/org/apache/poi/hssf/record/formula/functions/Hlookup.java +++ b/src/java/org/apache/poi/hssf/record/formula/functions/Hlookup.java @@ -60,8 +60,7 @@ public final class Hlookup implements Function { AreaEval tableArray = LookupUtils.resolveTableArrayArg(args[1]); boolean isRangeLookup = LookupUtils.resolveRangeLookupArg(arg3, srcCellRow, srcCellCol); int colIndex = LookupUtils.lookupIndexOfValue(lookupValue, LookupUtils.createRowVector(tableArray, 0), isRangeLookup); - ValueEval veRowIndex = OperandResolver.getSingleValue(args[2], srcCellRow, srcCellCol); - int rowIndex = LookupUtils.resolveRowOrColIndexArg(veRowIndex); + int rowIndex = LookupUtils.resolveRowOrColIndexArg(args[2], srcCellRow, srcCellCol); ValueVector resultCol = createResultColumnVector(tableArray, rowIndex); return resultCol.getItem(colIndex); } catch (EvaluationException e) { @@ -73,12 +72,11 @@ public final class Hlookup implements Function { /** * Returns one column from an AreaEval * - * @throws EvaluationException (#VALUE!) if colIndex is negative, (#REF!) if colIndex is too high + * @param rowIndex assumed to be non-negative + * + * @throws EvaluationException (#REF!) if colIndex is too high */ private ValueVector createResultColumnVector(AreaEval tableArray, int rowIndex) throws EvaluationException { - if(rowIndex < 0) { - throw EvaluationException.invalidValue(); - } if(rowIndex >= tableArray.getHeight()) { throw EvaluationException.invalidRef(); } diff --git a/src/java/org/apache/poi/hssf/record/formula/functions/LookupUtils.java b/src/java/org/apache/poi/hssf/record/formula/functions/LookupUtils.java index e6a3ec81c2..ee67ef8ec5 100644 --- a/src/java/org/apache/poi/hssf/record/formula/functions/LookupUtils.java +++ b/src/java/org/apache/poi/hssf/record/formula/functions/LookupUtils.java @@ -322,30 +322,45 @@ final class LookupUtils { * <blank> #VALUE! *
* - * * Note - out of range errors (both too high and too low) are handled by the caller. - * @return column or row index as a zero-based value - * + * Note - out of range errors (result index too high) are handled by the caller. + * @return column or row index as a zero-based value, never negative. + * @throws EvaluationException when the specified arg cannot be coerced to a non-negative integer */ - public static int resolveRowOrColIndexArg(ValueEval veRowColIndexArg) throws EvaluationException { - if(veRowColIndexArg == null) { + public static int resolveRowOrColIndexArg(Eval rowColIndexArg, int srcCellRow, int srcCellCol) throws EvaluationException { + if(rowColIndexArg == null) { throw new IllegalArgumentException("argument must not be null"); } - if(veRowColIndexArg instanceof BlankEval) { - throw EvaluationException.invalidValue(); + + ValueEval veRowColIndexArg; + try { + veRowColIndexArg = OperandResolver.getSingleValue(rowColIndexArg, srcCellRow, (short)srcCellCol); + } catch (EvaluationException e) { + // All errors get translated to #REF! + throw EvaluationException.invalidRef(); } - if(veRowColIndexArg instanceof StringEval) { - StringEval se = (StringEval) veRowColIndexArg; - String strVal = se.getStringValue(); - Double dVal = OperandResolver.parseDouble(strVal); - if(dVal == null) { - // String does not resolve to a number. Raise #VALUE! error. - throw EvaluationException.invalidRef(); - // This includes text booleans "TRUE" and "FALSE". They are not valid. + int oneBasedIndex; + if(veRowColIndexArg instanceof BlankEval) { + oneBasedIndex = 0; + } else { + if(veRowColIndexArg instanceof StringEval) { + StringEval se = (StringEval) veRowColIndexArg; + String strVal = se.getStringValue(); + Double dVal = OperandResolver.parseDouble(strVal); + if(dVal == null) { + // String does not resolve to a number. Raise #REF! error. + throw EvaluationException.invalidRef(); + // This includes text booleans "TRUE" and "FALSE". They are not valid. + } + // else - numeric value parses OK } - // else - numeric value parses OK + // actual BoolEval values get interpreted as FALSE->0 and TRUE->1 + oneBasedIndex = OperandResolver.coerceValueToInt(veRowColIndexArg); + } + if (oneBasedIndex < 1) { + // note this is asymmetric with the errors when the index is too large (#REF!) + throw EvaluationException.invalidValue(); } - // actual BoolEval values get interpreted as FALSE->0 and TRUE->1 - return OperandResolver.coerceValueToInt(veRowColIndexArg) - 1; + return oneBasedIndex - 1; // convert to zero based } @@ -583,11 +598,13 @@ final class LookupUtils { return maxIx - 1; } - public static LookupValueComparer createLookupComparer(ValueEval lookupValue) throws EvaluationException { + public static LookupValueComparer createLookupComparer(ValueEval lookupValue) { - if (lookupValue instanceof BlankEval) { - // blank eval can never be found in a lookup array - throw new EvaluationException(ErrorEval.NA); + if (lookupValue == BlankEval.INSTANCE) { + // blank eval translates to zero + // Note - a blank eval in the lookup column/row never matches anything + // empty string in the lookup column/row can only be matched by explicit emtpty string + return new NumberLookupComparer(NumberEval.ZERO); } if (lookupValue instanceof StringEval) { return new StringLookupComparer((StringEval) lookupValue); diff --git a/src/java/org/apache/poi/hssf/record/formula/functions/Vlookup.java b/src/java/org/apache/poi/hssf/record/formula/functions/Vlookup.java index 54f7d465e5..28923c0f37 100644 --- a/src/java/org/apache/poi/hssf/record/formula/functions/Vlookup.java +++ b/src/java/org/apache/poi/hssf/record/formula/functions/Vlookup.java @@ -60,8 +60,7 @@ public final class Vlookup implements Function { AreaEval tableArray = LookupUtils.resolveTableArrayArg(args[1]); boolean isRangeLookup = LookupUtils.resolveRangeLookupArg(arg3, srcCellRow, srcCellCol); int rowIndex = LookupUtils.lookupIndexOfValue(lookupValue, LookupUtils.createColumnVector(tableArray, 0), isRangeLookup); - ValueEval veColIndex = OperandResolver.getSingleValue(args[2], srcCellRow, srcCellCol); - int colIndex = LookupUtils.resolveRowOrColIndexArg(veColIndex); + int colIndex = LookupUtils.resolveRowOrColIndexArg(args[2], srcCellRow, srcCellCol); ValueVector resultCol = createResultColumnVector(tableArray, colIndex); return resultCol.getItem(rowIndex); } catch (EvaluationException e) { @@ -73,12 +72,11 @@ public final class Vlookup implements Function { /** * Returns one column from an AreaEval * - * @throws EvaluationException (#VALUE!) if colIndex is negative, (#REF!) if colIndex is too high + * @param colIndex assumed to be non-negative + * + * @throws EvaluationException (#REF!) if colIndex is too high */ private ValueVector createResultColumnVector(AreaEval tableArray, int colIndex) throws EvaluationException { - if(colIndex < 0) { - throw EvaluationException.invalidValue(); - } if(colIndex >= tableArray.getWidth()) { throw EvaluationException.invalidRef(); } diff --git a/src/testcases/org/apache/poi/hssf/data/LookupFunctionsTestCaseData.xls b/src/testcases/org/apache/poi/hssf/data/LookupFunctionsTestCaseData.xls index f4b35fb93504540b85455ff37341318cbcd74e1e..94f16e9840536697981a8c31ce59900625c6236f 100755 GIT binary patch delta 5460 zcma)Adr*|u6+d_R_Qh*~UEGD`u_{RkiArdrqJSHLM3J{f0S&Og3MjD1LlBLNIz~-V z(Q7-2X4JGzCv7?zn{Co0lSWKCjqRkJq>nL^&U8#V(`lP#N~ZmzV;i^UobT?k>+na} z+5PT4kKcLRd(S-|({C82ZyILHMBNX|-xH!SWHh+N(#-%+gWoOqt;O%%a5zlghRFC_ z)AjROPx`0N-6+^GdsBGs-2BsR+b=}D9M_H+M!pfcqsN1HAm=$Qs0Po5ZiRm%or6Y$zjHGc@~sJ)S!MaY%#K4MeiQ_SzL)zR~{fiE!TU}VhSLjd1BNMs9aiUQbR_0vQ%gCbqDxZt7% zK(2qJHYN8n0sn<7NnXtHMCigJ|1^ba${h*fhCEsJq8O7a%5#H8fRQ%;NFV{9D@yR! z*VSzTSYEYx`>Nt3fG%`@BrxIejr3*!bO#1J{X;$eNzYgy;OXn{?Mp+E0pHMG&p;rs zcYN5h-#3sBFgiBUKhztcp%(hM6u>_?Ja)jNn1jBt?!KUZ)DufBya%o)fC58-F%L_} ziQ0AN7F-(kjraz`FohYqy$T>1Z59**G~#m(pMS?^c?m#2?vn|8ejA^k*Ww@xqptrd zz!W~6UjwMr05-1y7&oKxT7W~C&!_SEDn4Jw=coAmGd?q{0I!q+e2mZEt^)|m54}fB z{&Ij%D&&o0KL}|XpBG|}e4%nrDvIEr04siQbq4!3q+&kcwJJ(a$z02Kx=G6og(Ch!xlUn5f|i*y7jvs;}}c~fTe%|ObTGVmy%;v ziF=$S%`Axpf+fvLvW!j9Hlf(Bs%WhXruyrvS54~(7Dm9M#RwLKP|gUb!3ZxQf*fjh z%1*ygrZzmbax)vZvMQcFR<$bC3Rb0EK*q0;>X{Cwe5Ijm`&={YC9)nJJPIgL>22`R zEJ=m8jU|(i>xEGznWQAWY=QPLXHx#PBS${oSg?lllXZQsuAj`7uxzrWWLz8%r$R9VRmN@3fV?cDH!2&6mYTWri8z(g)i)>mor1Qp+=|{8WM6XT$tJ9J3hRSo= zLY`-u)(wz8l28ZWeZw)_!xp?~BCC#D$buVBFZy&8sF2wBnCPbMD#)tBRXOb5g>|4fJ@>5Gh+ZV;{Y=~?4y$rGh+}D zK}VGDbmau~=$6E(E@90A)?5;!>5OQqbYVq`lnqLkGsYKZ6p&WM$-xbxnl87np@J@j z8L0zV@t7{g)YIZ(%pyUEE_QK9A-ZCmL;wyi0*FN|*2yddNNbt}#vsblA7N(2VP-LA zk)UmxtuRTJ76HrST~#kBplrsp@jl6p2Cj1!LPe#xmCPr{f_l{@S+`2xPjHJ;iPjyX zUzb-uiEb4pIh%`g8;szu3D!YgT+pb2*ac9P7jrkDP7+Y{F31DZ9J0)FSh<6|B6%t) z1e}MLkPXHWm{NqYx+7k45tVpGCeeMJgOXk^kPBooe^Hb9WHMhj$+-D!5f6!%>svk> z9YZK`0Wo5#9qQr?|JVftVY9bjBe+rkS`0;ygS}M|ya+#s349)fS^O5^xDG!p;KQob z6#Z<)(N?HNPAX>4A|E=xVucIyt)UewUV7`Xo5r;9ScEGK8SpyYD!K>w%#;79%*@Qj z{M#TQ8{Njf2R8tY61aPM00}R*0Q_t>z#sZl_jFYY{bt7BFEI5)^JBvkm z=)=y<1}%-|dEMJJ7Y!!6do&LXZg=0X7UF<5ZH~O^cgh<*CDvj^$(2uaJEh%UVqL2! zc`W1imuOz3gf98RLc2{qT<#sP(o=%W$*UBwrrB zq@AQ`&Aw_|n*z4*?(@mZ`z~p-#4I1FwmqSkR>eFwa$0+qn9D}1ZP*&KgVeFvpewY0 zbg!^=Dx!^J=$7+iZm~rskJE4A_yf7vG4b9{M?M~FynZR-biRyh@{fqoAnzbXgObaP!^mU|dhwvMof+pxCSx?p`u%RvB=_uh zmo_OmXB>u;F(?6q;f%v@GDfp}2QjwFj}e1jBlS`n8%q{rY>V<@W65HSZF1#=TRb6a zCfud85`^K7^TW*;+oQZ#3^!wJmzNP^hkOe$b||@A9Ro`$3=#(>qFi2}yxkQGzQMThK~ zI+f@`xwj7i96gSwc22%_@GapA4IjEs$RD+qMPGp&($$tGpF80aJ)w7=Y7rtVox$bW z8+1@Af)n|3IH2N#uWxhM{CV`V;8{2E>D=Gt;@u}vMtg0AE3ALL+2HqzA5j&g887)En+jjI+9G06dkSHH#=Ywscss)jh?px)lu z1c9mcPJzwee_cn5w02~j+D;o@VF0(_Ay|oTB!}ePqq)_oeC6%LOYdD#fO1B>?({0| z;vCS}j0VVG9AlHf7a!s*Tzqz_@);R=mCwLJO;sMfl6(3#^8XAPPXNqZ0GPkP8)mA1 zy_`Q-7qT9I$zbb6X>ySMBUAwfd`EVMZk-qq!Y`{%J*Zi}f~!1wYD!C^!561aYAzZ) z^~~p*hXzAmzu>_86t_(&zS!t}MrOYCy^8Xk z)2)s^1);b70GkLR{Y-mWqb@`%8(t1o4%u{esl0k-Lt2YsP}hx&OlG8=U4!78&NiAm z75J62t&V;LuJ?)neFUC7+io6KLi=X>&68^U$t?L-d9Kksttc1IUB^BImlQXUzBD;l o@bsQU^&x0rVL9Bi9ENO==g!Z)df`oj>HFjKAJEYCZ$4`LAMP8~^#A|> delta 3500 zcma)9dr*|u6+idm+x>P|V0U4IiwhP6Vl+`v$5D&G5Ez*L$Yat>?bu8+nc()EyPpV|{iDn5 zz2~0uJHLDGx%ZyC^uDR|URV5^#MW2S|0G1dZc;qLc|Q;c@OK*kNcmjxt@CQNvn}tF z{l95>Q}q7%`X48$e+t{xhoD=V_kD~byiDV>q&@)!AjOHU~T&|t<5cM zHPuY;b{nfLuw|CO4%m&&>wFP)9}D;fZrN5-4iD(>t-EIy6F8lD4F|mO`qU;@*s%b# zkn6}QfN&!D2lYkNA5I1MC-whX4bVVYTe$|{$JGCX`q(f4(^`N|>iYJ%wz7gO#>UUFri~4)i*V_Q9EHU(@3GjYayE$hQz(hJu^}Vd8WM2^? z9Vc@dBNvbvK+t+w!qoD0c(c!Di=u%)8{lu-$l;0}yZd<+_YD=G`*;E_t%zF6c1{vy zL~&5d1POeENVH3xU>!btJi7SXmaq_LHH6L-LT8q&zYoD`34ykT(Ah%h!X>NoMD((6 z#~K*|ZI@`@iD)5kwLb}U-x|EvuTbqSiLqcGbNkAJV7Q6hYO!Z zf0ZMP88}6Dy2{Img{P;)FoQWHgP9r3pvlOdRBQ2xqO@aqGA^@>OCy#Z#YUIOD&yKJ zZJM<4(c&37^&Io3M$D9Up3xF=Mhnkqc{n3ziix6ymbIJ=3yJta(OP`8#wl0BTgcpZ z4Q|>#=C+39wlcT%AvYEl@5Eo%1oOfh&rDw#Otk6DWDCh;VCB33A+yg%bZ#_sf|w^V zBu^ysM9$$U+_5?{gZzWJNjMrjvc!3xIOtf;9@&Sd&dwb6IUHAZe7Br8&j~jx@+nqTpBL58#z8@u4I4kwV#N=aaC7UICNIQ-&OUf<6YBX zy0_XX)P(NoFw81mh)p%IA`|;-_NYZYq?)ctrD06%9<}!{4c{fiVC{cZ{}CFTs!J6a z`t`c4in@WZrp8jWjD?EEYU^pD4g@T)2+ti@h#xgxQ-8(;g9i%K>+JkK3D2{T+Eido zC4s?74A!qUeI!&b6PkMX>B}E8WhpMw zMV_1<7yT&;r6)5og%m%n0_bZ2$ZDe$O2iQd3F>@qdfde>yY{(9U9!*RwB0Gc-xrzDGv+VOT;#Ov3*Rv%fqZg zCG=Zm_|Ta86%W@QE)iR?|L~X!lsQZ}l55{%5L;-)cBsUTBV($AjqZ+Id#Pcx8pfuM zA=S;s88YrLj5cYk(u+F}2)oZv+T|Lm@$I9Ycz*40=oo_Nj`N3zI3mrL{_ zi7v&g9*_7Q?v-r^q00<(+`LHQB)SY=C0seaBiqjiz1u)9o`+s6(YrD6m`CivblH|4 zdqX^p@x2~uH}-ns_Zpsw^E?x!=U#l4aQkpbwl5Ky?z3D&(mZsML|5P)!tKWy*~a!I zk(}OlhJL~V+-V2+vP+-p>lDJJZ#$kM@SS`I_I_VQuiqtV^qCW7LfpsF{v>q^pP%mj z!`~?)f$yV6b(3l*Kc6?%HFWhHKsCvV(Wl;0%NTs;)J}Cj3mZ>Ys`Vu3&z(*a>Je7G zej93FPn5t$h_($CRg;rFuP#_&GvAPJNWTzsKHyKBg2FZhH7?_Ma<5)4h=#d24J zK@n|GTxunX*DmhU9y36(BMq9vkk5=T#pfeMj!Que{m>SHB*