From 5fc5680d157600ea499990cf4f90270a99964c03 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Wed, 14 Sep 2016 14:59:00 +0000 Subject: [PATCH] Patches from Patrick Zimmermann from bugs #60130 and #60131 - DGET fix for empty cells and D* coding improvements git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1760717 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/ss/formula/functions/DGet.java | 19 ++- .../poi/ss/formula/functions/DStarRunner.java | 142 +++++++----------- test-data/spreadsheet/DGet.xls | Bin 51712 -> 53760 bytes 3 files changed, 67 insertions(+), 94 deletions(-) diff --git a/src/java/org/apache/poi/ss/formula/functions/DGet.java b/src/java/org/apache/poi/ss/formula/functions/DGet.java index 91a9934b5..0bf9cc262 100644 --- a/src/java/org/apache/poi/ss/formula/functions/DGet.java +++ b/src/java/org/apache/poi/ss/formula/functions/DGet.java @@ -17,7 +17,10 @@ package org.apache.poi.ss.formula.functions; +import org.apache.poi.ss.formula.eval.BlankEval; import org.apache.poi.ss.formula.eval.ErrorEval; +import org.apache.poi.ss.formula.eval.EvaluationException; +import org.apache.poi.ss.formula.eval.OperandResolver; import org.apache.poi.ss.formula.eval.ValueEval; /** @@ -46,8 +49,18 @@ public final class DGet implements IDStarAlgorithm { public ValueEval getResult() { if(result == null) { return ErrorEval.VALUE_INVALID; - } else { - return result; - } + } else if(result instanceof BlankEval) { + return ErrorEval.VALUE_INVALID; + } else + try { + if(OperandResolver.coerceValueToString(OperandResolver.getSingleValue(result, 0, 0)).equals("")) { + return ErrorEval.VALUE_INVALID; + } + else { + return result; + } + } catch (EvaluationException e) { + return e.getErrorEval(); + } } } diff --git a/src/java/org/apache/poi/ss/formula/functions/DStarRunner.java b/src/java/org/apache/poi/ss/formula/functions/DStarRunner.java index 6a87a67a6..2901abc95 100644 --- a/src/java/org/apache/poi/ss/formula/functions/DStarRunner.java +++ b/src/java/org/apache/poi/ss/formula/functions/DStarRunner.java @@ -17,13 +17,13 @@ package org.apache.poi.ss.formula.functions; -import org.apache.poi.ss.formula.TwoDEval; +import org.apache.poi.ss.formula.eval.AreaEval; import org.apache.poi.ss.formula.eval.BlankEval; import org.apache.poi.ss.formula.eval.ErrorEval; import org.apache.poi.ss.formula.eval.EvaluationException; import org.apache.poi.ss.formula.eval.NotImplementedException; import org.apache.poi.ss.formula.eval.NumericValueEval; -import org.apache.poi.ss.formula.eval.RefEval; +import org.apache.poi.ss.formula.eval.OperandResolver; import org.apache.poi.ss.formula.eval.StringEval; import org.apache.poi.ss.formula.eval.StringValueEval; import org.apache.poi.ss.formula.eval.ValueEval; @@ -62,11 +62,17 @@ public final class DStarRunner implements Function3Arg { public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval database, ValueEval filterColumn, ValueEval conditionDatabase) { // Input processing and error checks. - if(!(database instanceof TwoDEval) || !(conditionDatabase instanceof TwoDEval)) { + if(!(database instanceof AreaEval) || !(conditionDatabase instanceof AreaEval)) { return ErrorEval.VALUE_INVALID; } - TwoDEval db = (TwoDEval)database; - TwoDEval cdb = (TwoDEval)conditionDatabase; + AreaEval db = (AreaEval)database; + AreaEval cdb = (AreaEval)conditionDatabase; + + try { + filterColumn = OperandResolver.getSingleValue(filterColumn, srcRowIndex, srcColumnIndex); + } catch (EvaluationException e) { + return e.getErrorEval(); + } int fc; try { @@ -100,15 +106,11 @@ public final class DStarRunner implements Function3Arg { } // Filter each entry. if(matches) { - try { - ValueEval currentValueEval = solveReference(db.getValue(row, fc)); - // Pass the match to the algorithm and conditionally abort the search. - boolean shouldContinue = algorithm.processMatch(currentValueEval); - if(! shouldContinue) { - break; - } - } catch (EvaluationException e) { - return e.getErrorEval(); + ValueEval currentValueEval = resolveReference(db, row, fc); + // Pass the match to the algorithm and conditionally abort the search. + boolean shouldContinue = algorithm.processMatch(currentValueEval); + if(! shouldContinue) { + break; } } } @@ -126,56 +128,16 @@ public final class DStarRunner implements Function3Arg { } /** - * Resolve reference(-chains) until we have a normal value. + * * - * @param field a ValueEval which can be a RefEval. - * @return a ValueEval which is guaranteed not to be a RefEval - * @throws EvaluationException If a multi-sheet reference was found along the way. - */ - private static ValueEval solveReference(ValueEval field) throws EvaluationException { - if (field instanceof RefEval) { - RefEval refEval = (RefEval)field; - if (refEval.getNumberOfSheets() > 1) { - throw new EvaluationException(ErrorEval.VALUE_INVALID); - } - return solveReference(refEval.getInnerValueEval(refEval.getFirstSheetIndex())); - } - else { - return field; - } - } - - /** - * Returns the first column index that matches the given name. The name can either be - * a string or an integer, when it's an integer, then the respective column - * (1 based index) is returned. - * @param nameValueEval + * @param nameValueEval Must not be a RefEval or AreaEval. Thus make sure resolveReference() is called on the value first! * @param db - * @return the first column index that matches the given name (or int) + * @return * @throws EvaluationException */ - @SuppressWarnings("unused") - private static int getColumnForTag(ValueEval nameValueEval, TwoDEval db) + private static int getColumnForName(ValueEval nameValueEval, AreaEval db) throws EvaluationException { - int resultColumn = -1; - - // Numbers as column indicator are allowed, check that. - if(nameValueEval instanceof NumericValueEval) { - double doubleResultColumn = ((NumericValueEval)nameValueEval).getNumberValue(); - resultColumn = (int)doubleResultColumn; - // Floating comparisions are usually not possible, but should work for 0.0. - if(doubleResultColumn - resultColumn != 0.0) - throw new EvaluationException(ErrorEval.VALUE_INVALID); - resultColumn -= 1; // Numbers are 1-based not 0-based. - } else { - resultColumn = getColumnForName(nameValueEval, db); - } - return resultColumn; - } - - private static int getColumnForName(ValueEval nameValueEval, TwoDEval db) - throws EvaluationException { - String name = getStringFromValueEval(nameValueEval); + String name = OperandResolver.coerceValueToString(nameValueEval); return getColumnForString(db, name); } @@ -187,16 +149,19 @@ public final class DStarRunner implements Function3Arg { * @return Corresponding column number. * @throws EvaluationException If it's not possible to turn all headings into strings. */ - private static int getColumnForString(TwoDEval db,String name) + private static int getColumnForString(AreaEval db,String name) throws EvaluationException { int resultColumn = -1; final int width = db.getWidth(); for(int column = 0; column < width; ++column) { - ValueEval columnNameValueEval = db.getValue(0, column); - if(solveReference(columnNameValueEval) instanceof BlankEval) { + ValueEval columnNameValueEval = resolveReference(db, 0, column); + if(columnNameValueEval instanceof BlankEval) { continue; } - String columnName = getStringFromValueEval(columnNameValueEval); + if(columnNameValueEval instanceof ErrorEval) { + continue; + } + String columnName = OperandResolver.coerceValueToString(columnNameValueEval); if(name.equals(columnName)) { resultColumn = column; break; @@ -215,7 +180,7 @@ public final class DStarRunner implements Function3Arg { * @throws EvaluationException If references could not be resolved or comparison * operators and operands didn't match. */ - private static boolean fullfillsConditions(TwoDEval db, int row, TwoDEval cdb) + private static boolean fullfillsConditions(AreaEval db, int row, AreaEval cdb) throws EvaluationException { // Only one row must match to accept the input, so rows are ORed. // Each row is made up of cells where each cell is a condition, @@ -229,20 +194,15 @@ public final class DStarRunner implements Function3Arg { // special column that accepts formulas. boolean columnCondition = true; ValueEval condition = null; - try { - // The condition to apply. - condition = solveReference(cdb.getValue(conditionRow, column)); - } catch (java.lang.RuntimeException e) { - // It might be a special formula, then it is ok if it fails. - columnCondition = false; - } + + // The condition to apply. + condition = resolveReference(cdb, conditionRow, column); + // If the condition is empty it matches. if(condition instanceof BlankEval) continue; // The column in the DB to apply the condition to. - ValueEval targetHeader = solveReference(cdb.getValue(0, column)); - targetHeader = solveReference(targetHeader); - + ValueEval targetHeader = resolveReference(cdb, 0, column); if(!(targetHeader instanceof StringValueEval)) { throw new EvaluationException(ErrorEval.VALUE_INVALID); @@ -254,14 +214,14 @@ public final class DStarRunner implements Function3Arg { if(columnCondition == true) { // normal column condition // Should not throw, checked above. - ValueEval value = db.getValue( - row, getColumnForName(targetHeader, db)); + ValueEval value = resolveReference(db, row, getColumnForName(targetHeader, db)); if(!testNormalCondition(value, condition)) { matches = false; break; } } else { // It's a special formula condition. - if(getStringFromValueEval(condition).isEmpty()) { + // TODO: Check whether the condition cell contains a formula and return #VALUE! if it doesn't. + if(OperandResolver.coerceValueToString(condition).isEmpty()) { throw new EvaluationException(ErrorEval.VALUE_INVALID); } throw new NotImplementedException( @@ -328,7 +288,7 @@ public final class DStarRunner implements Function3Arg { if(itsANumber) { return testNumericCondition(value, operator.equal, stringOrNumber); } else { // It's a string. - String valueString = value instanceof BlankEval ? "" : getStringFromValueEval(value); + String valueString = value instanceof BlankEval ? "" : OperandResolver.coerceValueToString(value); return stringOrNumber.equals(valueString); } } else { // It's a text starts-with condition. @@ -336,7 +296,7 @@ public final class DStarRunner implements Function3Arg { return value instanceof StringEval; } else { - String valueString = value instanceof BlankEval ? "" : getStringFromValueEval(value); + String valueString = value instanceof BlankEval ? "" : OperandResolver.coerceValueToString(value); return valueString.startsWith(conditionString); } } @@ -424,20 +384,20 @@ public final class DStarRunner implements Function3Arg { return null; } } - + /** - * Takes a ValueEval and tries to retrieve a String value from it. - * It tries to resolve references if there are any. + * Resolve a ValueEval that's in an AreaEval. * - * @param value ValueEval to retrieve the string from. - * @return String corresponding to the given ValueEval. - * @throws EvaluationException If it's not possible to retrieve a String value. + * @param db AreaEval from which the cell to resolve is retrieved. + * @param dbRow Relative row in the AreaEval. + * @param dbCol Relative column in the AreaEval. + * @return A ValueEval that is a NumberEval, StringEval, BoolEval, BlankEval or ErrorEval. */ - private static String getStringFromValueEval(ValueEval value) - throws EvaluationException { - value = solveReference(value); - if(!(value instanceof StringValueEval)) - throw new EvaluationException(ErrorEval.VALUE_INVALID); - return ((StringValueEval)value).getStringValue(); + private static ValueEval resolveReference(AreaEval db, int dbRow, int dbCol) { + try { + return OperandResolver.getSingleValue(db.getValue(dbRow, dbCol), db.getFirstRow()+dbRow, db.getFirstColumn()+dbCol); + } catch (EvaluationException e) { + return e.getErrorEval(); + } } } diff --git a/test-data/spreadsheet/DGet.xls b/test-data/spreadsheet/DGet.xls index 5d254febeb15659303de31134301f5446aaa2924..729e974e4dfd4bf2e06b10c620cbb17b4b6bf719 100644 GIT binary patch delta 4726 zcmZ`+YitzP75--ZUTp8Oez1Nr2GdfY0z?{;7KjZt)CHRYHjl+%)_AaYz1}r`qyn`} z67rB*wFNF!E5(3$l(d2H$TmqIX|w5{kbhAc5z!Y_f3%gVs>+|Xs!F!!+_^LMtjA-m z@0|19d%kndJ@?+Z&z#SD=33sFHQM0WwRQQ&0NmYmTT5p(UDFl_J+En=|6!5ID8p^zG43+oYBaH4}Jogq_SEJ<$vCS#jX4j_4a{$`>pKrKZRF{O$B9ylS%*M zc-yB8vE9dJ?4BuLyWFS5>68Ab;EsRupHN9d01Y~DL_-S;zKrhH2f?t1J_uc$3Wf`N zFlC&4H$4^nVx>5;Fr@7k3m2}@_Q?O;^uIBEzqvnD_5MCKUxeL8bU^PkUO*{+6A_*3 zZxmgtC?(&2na-~N&O$NO(I`Ibc*!gPjFHia6V#56MvoeKz{Vv@$l)<#d@?biA2kvQ zJvOe(PStbNQC*G-%>l?r4Y0JUwX^*xJ(kobh75foIyPunCc}j7ZQaphwm0b=NqxW= z(<3n>agx}Kd>*&Dl{{M7)zjHjq`I~@RapR=a}8i)+>d-M+Y+@ zz3kbs!vjpP;oa?9n>+#_qm#+WabrL?#>P@(`Vgg?f_OTbm^8*IPfAXvCZxO3iD-Xx z+{ohBD8?Ti2h9qwd_7F?bnRacyhQDAQQDFpUy0$!wi zeD*0|Z!xf|1$d?e*xE|o_<_33z*iOkr>T9N+7GDBQv1g(Kv@}ZzYQoT2R>=1VhaFk zwmt^*(M!)Z;CZUR-&4C_A<+9};9Jz*q4puQLmj~PX_Eg*ZDU2cZ2O(k!iW2TgAwr{ zGL`PzKj5pLAjd!H1KybgE_@rhII*3DaltCQ! z&`~w~pT1Xw~fC#7m}>wJxP`usW#=~vGOiW)|tW$Uo)AbL@VGAzo& z&ZHs50D|KF%NrUZ^Y#d#Lad%TWo|)(_6U9)Bm-?Y&CW?FfX#T8q*Fe|f`rl2VqqbL zSyBaIFFWI^6jtCXrg7DT1(f};3#lPwh4!dhlGVAOdV-qOphg$6m=N3DCWTl|EFpxQ zFT`5(irW|KSGw|1MV_?&9qd#u9WDXoR4?+-il2xJ7y8Vtm{vjbQYyM}*^vTh$McR9 z#5TO(NFf(iLD-aoRk^Tg!Y(*i*oD<#n~6&fSmy%kY2ZZ%Yjk0Y37dAXCKtAZFb;xK za){2ffDHT~vRT?T=G4UkyL7^+5x=`wtGzD%b+I|<%3>Yr#fnR{HEG3om~2{A!jo{A zYVOdb*&C7w&HPCiks7`W@_Uwcm{*wZ_~>ca3%0;1h80?U7l4yQS{4B zt(R+ey-gjgj&=d@HWO9bOmPdBY`R=XuV)(RS*7jL|?x8Wm+CZB!OER=!6#4RL5Qf;2L3?{UCJp{r zj9+cv!OjfIVZ61r$>fq5KAU0~C#yw#Hg$h3MPeNOjGt4>d-33`En!d^d`-M@wZmkC zLpgpB!8`^_Go`cUO6QagNyA)+mc+r|k^!zo%<XY}&4tXUc6?W3yb1Nmp+x z>oE#&W*3Z6ux)tG!Tjiy=P3ITqmRkiQn^bCI+Dpyqk~#2Jm} z1Wt*_30%c(BA5x4d4P`=h3WjA0=xc@Ma9F0`4P||ptnPNg8I~!qtBy`9V!5H1 z6N>K|Q?ZUH)&|mXyyV3^ ztks!nn{>pKtk#Fth-AH^SVtr)f5!0g9g(a7jWe6Hn6x6rM9lLrg)?EBDJhAHWMY$& z=%-9%Ql68NiD#dgl9FkKWJ)QfMT&`-=V7YN)Y?ob$+So^r6kkORY)mG#2;8plwu;2 zK0i4o`o5;w@@Y^^#5@mEeWu=K8kJ0tOrw(NXNqZ5GEFO{Q5lkzWIlWYXjc7>o7AyG z)-$_woI37-m83C=aBVX(CQVpD@iyBo>zt8sLgAR`I!O_2vB(3E89J^p+>~Tzlf&^#_F; zp8LW5vJcY_uN!(N-vjyH$M-zG*L|AJX8GF2*EGJC@imOEUA^?FFD}Ud delta 2383 zcmZ`)ZA_C_6n<{MpcQCg1xjnNL`ckFnGQeL1_%ncITb|tuzb|kR+~66VPR&3SzL4` zYNm35WSIj|KHMA?eX}J?_GA9ruZfA#jkpiDW&5!#+kPxF+oU`9?b}z1FSmJn?tPwf z&$;K^d+zNY*Y^+V&#h+r&aZJ90syY>yvC#$^Dt)P9(}-?wic3x>rC!S1N*5fmsVo2Cs``GR| zqp&M<%&Lxb$2eng^rhO-mrAqL)`;SsPd7+E*eqEU7!b6?r1jAzqk6O zZ3Xp{kV?Ic@BzDe>V$sbAnMzUc8;3W^8Wk8rhyI&6cOosX@fVWeC zZIwVXE&lb5K))F{SOxT0fU0U>>oVX9$r~j9A{is8-wdRu1OKk0jxhtssRcY%poyf9 zWN#gC+y<~Mz_Lu>7|GKlza;rprc|+gCe3uM87OY$=8g-}S#MC66Q+gU>;TTb1{^vK z+9)pdoD6)2%#s4**2bvEpz?qaM_1KXv6?bg7=J}tIK zi%nn=)v?I+V#QVsY>x)!)xg>`u=XdgXKkP~J>j@h1M>+?@(%20%)@VTuik^r{LcXw ze}1r(`ZaR@)Zi;74=Lgi_n)!z-v_HndCgFR(SvfN^9MtDd~~RSuy2OEGWL{=H4T@_ zqa6P2a2}r=E@d8RZrGaQZUKwAP}GKYnCMvM>QUbnl7|^onx>I*cl(kWnaJY4k)x3s zxY;9E(1r|b!ZBLhQJolBQ6pAb73VS)E1#JHIWYbZuWi_+0aY(p$YWwvB!c4YJWqfR!#{~q;4w&0?& z)OHH77RS|*6`y11ga>1*>XWRw zk&o(MA$H6su5o3@e9)sBR}?$Gq;&a6E1s@SlR+6fATaJ3FL|7)PKs1iMf>E#(=7^u$> zrk}yUznCtQ{)qOlf#}z!>W|;J`~7X{yJ*N$F&~S?#1kZ*9Pz}6Cq=v<;>8dzgy6*s vAZ~nd!HWysN8euh{QUf_!{_(f24)}Xx-ap6W_wv9@3|dG6VC{paJl^t&%dF}