Bug 65475 - SUMIF function's difference between POI and Micro Excel
Summary: SUMIF function's difference between POI and Micro Excel
Status: NEW
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 5.0.0-FINAL
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-30 03:02 UTC by zhangyajun
Modified: 2021-08-02 00:49 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description zhangyajun 2021-07-30 03:02:49 UTC
e.g. There is a formula cell contains SUMIF function. IF it's addend is #N/A,Micro Excel would result #N/A while in Apache POI it can still get a numberical result because POI would replace a non numberical addend with zero.
So, why not just alter the Sumif class in org.apache.poi.ss.formula.functions package as follows:

private static ValueEval eval(int srcRowIndex, int srcColumnIndex, ValueEval arg1, AreaEval aeRange, AreaEval aeSum) {
        I_MatchPredicate mp = Countif.createCriteriaPredicate(arg1, srcRowIndex, srcColumnIndex);
        if (mp == null) {
            return NumberEval.ZERO;
        } else {
            try {
                double result = sumMatchingCells(aeRange, mp, aeSum);
                return new NumberEval(result);
            } catch (EvaluationException var) {
                return var.getErrorEval();
            }

        }
    }

    private static double sumMatchingCells(AreaEval aeRange, I_MatchPredicate mp, AreaEval aeSum) throws EvaluationException {
        int height = aeRange.getHeight();
        int width = aeRange.getWidth();
        double result = 0.0D;

        for(int r = 0; r < height; ++r) {
            for(int c = 0; c < width; ++c) {
                result += accumulate(aeRange, mp, aeSum, r, c);
            }
        }

        return result;
    }

    private static double accumulate(AreaEval aeRange, I_MatchPredicate mp, AreaEval aeSum, int relRowIndex, int relColIndex) throws EvaluationException {
        if (!mp.matches(aeRange.getRelativeValue(relRowIndex, relColIndex))) {
            return 0.0D;
        } else {
            ValueEval addend = aeSum.getRelativeValue(relRowIndex, relColIndex);
            if (addend instanceof NumberEval) {
                return ((NumberEval) addend).getNumberValue();
            } else {
                throw new EvaluationException(ErrorEval.NA);
            }
        }
    }
Comment 1 PJ Fanning 2021-07-30 09:03:29 UTC
Thanks for raising the issue - could you outline a test scenario? Something like the examples in https://support.microsoft.com/en-us/office/sumif-function-169b8c99-c05c-4483-a712-1697a653039b
Comment 2 PJ Fanning 2021-07-30 09:46:31 UTC
I added a disabled test that appears not to work with your change - r1891893
Comment 3 PJ Fanning 2021-07-30 11:04:13 UTC
I fixed the issue in the test and merged your change (r1891896) - thanks
Comment 4 PJ Fanning 2021-07-30 12:28:03 UTC
looks like we might have the same bug in SUMIFS (not handling #N/A correctly). I also plan to undo part of zhangyajun's change as it treats all non-numbers as NA
Comment 5 zhangyajun 2021-08-02 00:49:11 UTC
(In reply to PJ Fanning from comment #4)
> looks like we might have the same bug in SUMIFS (not handling #N/A
> correctly). I also plan to undo part of zhangyajun's change as it treats all
> non-numbers as NA

this time i only consider the case that program should acheive an error cell when encountering error addend. There might be corresponding methods to handle other kinds of non numberical cell. 
Anyway, thanks for your help.

By the way, is https://github.com/apache/poi the POI's repository? Maybe next time i can pr my code directly