Bug 66433 - [PATCH] Improve boolean functions empty cell handling
Summary: [PATCH] Improve boolean functions empty cell handling
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2023-01-18 16:08 UTC by Patrick Böker
Modified: 2023-01-24 12:23 UTC (History)
0 users



Attachments
boolean-blank-eval.patch (1.15 KB, patch)
2023-01-18 16:08 UTC, Patrick Böker
Details | Diff
BooleanFunctionsTestCaseData.xls (30.00 KB, application/octet-stream)
2023-01-19 08:25 UTC, Patrick Böker
Details
LookupFunctionsTestCaseData.xls (69.00 KB, application/octet-stream)
2023-01-19 08:26 UTC, Patrick Böker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Böker 2023-01-18 16:08:14 UTC
Created attachment 38467 [details]
boolean-blank-eval.patch

The attached patch fixes AND() and OR() when passing a function. Previously BlankEvals have been ignored. Now they are treated as FALSE. This matches Excels behavior (tested on Excel 2016).


This implementation works by mapping BlankEval to FALSE in class BooleanFunction. This is my best guess at what Excel is doing behind the scenes. But I am not sure as I didn't manage to come up with tests to unambiguously differentiate the possibilities.

Anyways, I did add tests for this case, they match Excels behavior. All tests are passing.
Comment 1 Patrick Böker 2023-01-18 16:14:04 UTC
Uploading the test files currently fails with a 500 server error. I'll try again tomorrow.
Comment 2 Patrick Böker 2023-01-19 08:25:23 UTC
Created attachment 38469 [details]
BooleanFunctionsTestCaseData.xls

BooleanFunctionsTestCaseData.xls is to be placed in poi/test-data/spreadsheet
Comment 3 Patrick Böker 2023-01-19 08:26:44 UTC
Created attachment 38470 [details]
LookupFunctionsTestCaseData.xls

LookupFunctionsTestCaseData.xls is to be placed in poi\test-data\spreadsheet
Comment 4 PJ Fanning 2023-01-20 11:53:52 UTC
what are the xls files for? there are no tests in the patch that use the test files
Comment 5 PJ Fanning 2023-01-20 12:06:58 UTC
I found out how those xls files are used. I dislike those tests. Depending on xls files provided by non-ASF users means we get no patch diff support and the files could be malicious.

I checked the files in the web version of Excel and they seem ok.

I added r1906836 nonetheless.