From 7df009dfd998e34286aef2d33ca10de94dfe51ce Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Mon, 2 Feb 2009 23:53:22 +0000 Subject: [PATCH] Modified formula parser to encode SUM taking a single argument as tAttrSum (from comment 7 of bugzilla 46643) git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@740159 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/record/formula/AttrPtg.java | 72 +++++++------------ .../record/formula/eval/FunctionEval.java | 7 +- .../function/FunctionMetadataRegistry.java | 9 +-- .../apache/poi/ss/formula/FormulaParser.java | 9 ++- .../ss/formula/OperandClassTransformer.java | 17 +++++ .../poi/hssf/model/TestFormulaParserEval.java | 9 ++- 6 files changed, 62 insertions(+), 61 deletions(-) diff --git a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java b/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java index 7a256bf2f7..1fe6fa3d72 100644 --- a/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java +++ b/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java @@ -79,8 +79,7 @@ public final class AttrPtg extends ControlPtg { _chooseFuncOffset = -1; } - public AttrPtg(LittleEndianInput in) - { + public AttrPtg(LittleEndianInput in) { field_1_options = in.readByte(); field_2_data = in.readShort(); if (isOptimizedChoose()) { @@ -113,44 +112,30 @@ public final class AttrPtg extends ControlPtg { return new AttrPtg(space.set(0), data, null, -1); } - public void setOptions(byte options) - { - field_1_options = options; + public static AttrPtg getSumSingle() { + return new AttrPtg(sum.set(0), 0, null, -1); } - public byte getOptions() - { - return field_1_options; - } - public boolean isSemiVolatile() - { - return semiVolatile.isSet(getOptions()); + public boolean isSemiVolatile() { + return semiVolatile.isSet(field_1_options); } - public boolean isOptimizedIf() - { - return optiIf.isSet(getOptions()); + public boolean isOptimizedIf() { + return optiIf.isSet(field_1_options); } - public boolean isOptimizedChoose() - { - return optiChoose.isSet(getOptions()); + public boolean isOptimizedChoose() { + return optiChoose.isSet(field_1_options); } // lets hope no one uses this anymore - public boolean isGoto() - { - return optGoto.isSet(getOptions()); - } - - public boolean isSum() - { - return sum.isSet(getOptions()); + public boolean isGoto() { + return optGoto.isSet(field_1_options); } - public void setSum(boolean bsum) { - field_1_options=sum.setByteBoolean(field_1_options,bsum); + public boolean isSum() { + return sum.isSet(field_1_options); } public void setOptimizedIf(boolean bif) { @@ -166,24 +151,20 @@ public final class AttrPtg extends ControlPtg { } // lets hope no one uses this anymore - public boolean isBaxcel() - { - return baxcel.isSet(getOptions()); + private boolean isBaxcel() { + return baxcel.isSet(field_1_options); } // biff3&4 only shouldn't happen anymore - public boolean isSpace() - { - return space.isSet(getOptions()); + public boolean isSpace() { + return space.isSet(field_1_options); } - public void setData(short data) - { + public void setData(short data) { field_2_data = data; } - public short getData() - { + public short getData() { return field_2_data; } @@ -227,8 +208,7 @@ public final class AttrPtg extends ControlPtg { } } - public int getSize() - { + public int getSize() { if (_jumpTable != null) { return SIZE + (_jumpTable.length + 1) * LittleEndian.SHORT_SIZE; } @@ -239,22 +219,20 @@ public final class AttrPtg extends ControlPtg { if(space.isSet(field_1_options)) { return operands[ 0 ]; } else if (optiIf.isSet(field_1_options)) { - return toFormulaString() + "(" + operands[ 0 ] +")"; + return toFormulaString() + "(" + operands[0] + ")"; } else if (optGoto.isSet(field_1_options)) { return toFormulaString() + operands[0]; //goto isn't a real formula element should not show up } else { - return toFormulaString() + "(" + operands[ 0 ] + ")"; + return toFormulaString() + "(" + operands[0] + ")"; } } - public int getNumberOfOperands() - { + public int getNumberOfOperands() { return 1; } - public int getType() - { + public int getType() { return -1; } @@ -288,7 +266,7 @@ public final class AttrPtg extends ControlPtg { if (_jumpTable == null) { jt = null; } else { - jt = (int[]) _jumpTable.clone(); + jt = _jumpTable.clone(); } return new AttrPtg(field_1_options, field_2_data, jt, _chooseFuncOffset); } diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java b/src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java index 2940af6869..12da871e6d 100644 --- a/src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java +++ b/src/java/org/apache/poi/hssf/record/formula/eval/FunctionEval.java @@ -20,6 +20,7 @@ package org.apache.poi.hssf.record.formula.eval; import java.util.HashMap; import java.util.Map; +import org.apache.poi.hssf.record.formula.function.FunctionMetadataRegistry; import org.apache.poi.hssf.record.formula.functions.*; /** @@ -31,12 +32,14 @@ public abstract class FunctionEval implements OperationEval { * Some function IDs that require special treatment */ private static final class FunctionID { + /** 4 */ + public static final int SUM = FunctionMetadataRegistry.FUNCTION_INDEX_SUM; /** 78 */ public static final int OFFSET = 78; /** 148 */ public static final int INDIRECT = 148; /** 255 */ - public static final int EXTERNAL_FUNC = 255; + public static final int EXTERNAL_FUNC = FunctionMetadataRegistry.FUNCTION_INDEX_EXTERNAL; } // convenient access to namespace private static final FunctionID ID = null; @@ -75,7 +78,7 @@ public abstract class FunctionEval implements OperationEval { retval[1] = new If(); // IF retval[2] = new IsNa(); // ISNA retval[3] = new IsError(); // ISERROR - retval[4] = AggregateFunction.SUM; + retval[ID.SUM] = AggregateFunction.SUM; retval[5] = AggregateFunction.AVERAGE; retval[6] = AggregateFunction.MIN; retval[7] = AggregateFunction.MAX; diff --git a/src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java b/src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java index fe243bf011..1b78cc44df 100644 --- a/src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java +++ b/src/java/org/apache/poi/hssf/record/formula/function/FunctionMetadataRegistry.java @@ -30,11 +30,12 @@ public final class FunctionMetadataRegistry { */ public static final String FUNCTION_NAME_IF = "IF"; + public static final short FUNCTION_INDEX_SUM = 4; public static final short FUNCTION_INDEX_EXTERNAL = 255; private static FunctionMetadataRegistry _instance; private final FunctionMetadata[] _functionDataByIndex; - private final Map _functionDataByName; + private final Map _functionDataByName; private static FunctionMetadataRegistry getInstance() { if (_instance == null) { @@ -43,12 +44,12 @@ public final class FunctionMetadataRegistry { return _instance; } - /* package */ FunctionMetadataRegistry(FunctionMetadata[] functionDataByIndex, Map functionDataByName) { + /* package */ FunctionMetadataRegistry(FunctionMetadata[] functionDataByIndex, Map functionDataByName) { _functionDataByIndex = functionDataByIndex; _functionDataByName = functionDataByName; } - /* package */ Set getAllFunctionNames() { + /* package */ Set getAllFunctionNames() { return _functionDataByName.keySet(); } @@ -75,7 +76,7 @@ public final class FunctionMetadataRegistry { } private FunctionMetadata getFunctionByNameInternal(String name) { - return (FunctionMetadata) _functionDataByName.get(name); + return _functionDataByName.get(name); } diff --git a/src/java/org/apache/poi/ss/formula/FormulaParser.java b/src/java/org/apache/poi/ss/formula/FormulaParser.java index c05ef4475c..e67e6b3023 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaParser.java +++ b/src/java/org/apache/poi/ss/formula/FormulaParser.java @@ -27,6 +27,7 @@ import org.apache.poi.hssf.record.formula.AddPtg; import org.apache.poi.hssf.record.formula.Area3DPtg; import org.apache.poi.hssf.record.formula.AreaPtg; import org.apache.poi.hssf.record.formula.ArrayPtg; +import org.apache.poi.hssf.record.formula.AttrPtg; import org.apache.poi.hssf.record.formula.BoolPtg; import org.apache.poi.hssf.record.formula.ConcatPtg; import org.apache.poi.hssf.record.formula.DividePtg; @@ -592,9 +593,11 @@ public final class FormulaParser { } boolean isVarArgs = !fm.hasFixedArgsLength(); int funcIx = fm.getIndex(); - if (false && funcIx == 4 && args.length == 1) { - // TODO - make POI behave more like Excel when summing a single argument: - // return new ParseNode(AttrPtg.getSumSingle(), args); + if (funcIx == FunctionMetadataRegistry.FUNCTION_INDEX_SUM && args.length == 1) { + // Excel encodes the sum of a single argument as tAttrSum + // POI does the same for consistency, but this is not critical + return new ParseNode(AttrPtg.getSumSingle(), args); + // The code below would encode tFuncVar(SUM) which seems to do no harm } validateNumArgs(args.length, fm); diff --git a/src/java/org/apache/poi/ss/formula/OperandClassTransformer.java b/src/java/org/apache/poi/ss/formula/OperandClassTransformer.java index 8403400432..6a6e676a1f 100644 --- a/src/java/org/apache/poi/ss/formula/OperandClassTransformer.java +++ b/src/java/org/apache/poi/ss/formula/OperandClassTransformer.java @@ -18,7 +18,9 @@ package org.apache.poi.ss.formula; import org.apache.poi.hssf.record.formula.AbstractFunctionPtg; +import org.apache.poi.hssf.record.formula.AttrPtg; import org.apache.poi.hssf.record.formula.ControlPtg; +import org.apache.poi.hssf.record.formula.FuncVarPtg; import org.apache.poi.hssf.record.formula.MemFuncPtg; import org.apache.poi.hssf.record.formula.Ptg; import org.apache.poi.hssf.record.formula.RangePtg; @@ -101,6 +103,13 @@ final class OperandClassTransformer { return; } + if (isSingleArgSum(token)) { + // Need to process the argument of SUM with transformFunctionNode below + // so make a dummy FuncVarPtg for that call. + token = new FuncVarPtg("SUM", (byte)1); + // Note - the tAttrSum token (node.getToken()) is a base + // token so does not need to have its operand class set + } if (token instanceof ValueOperatorPtg || token instanceof ControlPtg || token instanceof MemFuncPtg || token instanceof UnionPtg) { @@ -135,6 +144,14 @@ final class OperandClassTransformer { token.setClass(transformClass(token.getPtgClass(), desiredOperandClass, callerForceArrayFlag)); } + private static boolean isSingleArgSum(Ptg token) { + if (token instanceof AttrPtg) { + AttrPtg attrPtg = (AttrPtg) token; + return attrPtg.isSum(); + } + return false; + } + private static boolean isSimpleValueFunction(Ptg token) { if (token instanceof AbstractFunctionPtg) { AbstractFunctionPtg aptg = (AbstractFunctionPtg) token; diff --git a/src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java b/src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java index 48be912292..fc93a34782 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java +++ b/src/testcases/org/apache/poi/hssf/model/TestFormulaParserEval.java @@ -20,7 +20,7 @@ package org.apache.poi.hssf.model; import junit.framework.AssertionFailedError; import junit.framework.TestCase; -import org.apache.poi.hssf.record.formula.FuncVarPtg; +import org.apache.poi.hssf.record.formula.AttrPtg; import org.apache.poi.hssf.record.formula.NamePtg; import org.apache.poi.hssf.record.formula.Ptg; import org.apache.poi.hssf.usermodel.HSSFCell; @@ -58,9 +58,8 @@ public final class TestFormulaParserEval extends TestCase { confirmParseFormula(workbook); // And make it non-contiguous - if (false) { // TODO (Nov 2008) - make the formula parser support area unions - name.setReference("A1:A2,C3"); - } + // using area unions + name.setReference("A1:A2,C3"); confirmParseFormula(workbook); } @@ -72,7 +71,7 @@ public final class TestFormulaParserEval extends TestCase { Ptg[] ptgs = HSSFFormulaParser.parse("SUM(testName)", workbook); assertTrue("two tokens expected, got "+ptgs.length,ptgs.length == 2); assertEquals(NamePtg.class, ptgs[0].getClass()); - assertEquals(FuncVarPtg.class, ptgs[1].getClass()); + assertEquals(AttrPtg.class, ptgs[1].getClass()); } public void testEvaluateFormulaWithRowBeyond32768_Bug44539() { -- 2.39.5