Bug 56958 - [PATCH] XSSFSheet.validateArrayFormulas incorrect and incomplete range intersection check
Summary: [PATCH] XSSFSheet.validateArrayFormulas incorrect and incomplete range inters...
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.11-dev
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2014-09-11 10:09 UTC by Yaniv Kunda
Modified: 2016-06-19 23:06 UTC (History)
0 users


Attachments
fixes incorrect intersection check (1.21 KB, patch)
2014-09-11 10:22 UTC, Yaniv Kunda 		Details | Diff
fixes incorrect and incomplete intersection check (7.65 KB, patch)
2014-09-11 11:53 UTC, Yaniv Kunda 		Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yaniv Kunda 2014-09-11 10:09:44 UTC 
This method checks
if (arrayRange.getNumberOfCells() > 1 &&
( arrayRange.isInRange(region.getFirstRow(), region.getFirstColumn()) ||
  arrayRange.isInRange(region.getFirstRow(), region.getFirstColumn()))  ){
   ....
}

Obviously, check arrayRange.isInRange(region.getFirstRow(), region.getFirstColumn() twice does not look intentional.
This should *probably* be:

if (arrayRange.getNumberOfCells() > 1 &&
( arrayRange.isInRange(region.getFirstRow(), region.getFirstColumn()) ||
  arrayRange.isInRange(region.getLastRow(), region.getLastColumn()))  ){

to check if the region intersects with the region.
Comment 1 Yaniv Kunda 2014-09-11 10:22:31 UTC 
Created attachment 32001 [details]
fixes incorrect intersection check
Comment 2 Nick Burch 2014-09-11 10:22:54 UTC 
Any chance of a small junit unit test which shows the problem? That'll help us verify the fix, and ensure it stays correct in the future
Comment 3 Yaniv Kunda 2014-09-11 11:53:18 UTC 
Created attachment 32003 [details]
fixes incorrect and incomplete intersection check

I realized the intersection check is flawed, as it only detected if one of two corners of the region are in the arrayRange, where there are many cases where two rectangles can intersect.

- Modified BaseTestSheetUpdateArrayFormulas.testModifyArrayCells_mergeCells to test a few more cases (including positive testing)
- Added CellRangeAddressBase.intersects(CellRangeAddressBase other) to correctly check intersection
- Fixed both XSSFSheet and HSSFSheet to use the new method
Comment 4 Javen O'Neal 2016-06-17 10:23:53 UTC 
Quick fix in r1748829.
I will review and commit your patch from comment 3 later.
Comment 5 Javen O'Neal 2016-06-19 23:06:09 UTC 
r1748829 fixed XSSF (and SXSSF)
r1749210 fixed HSSF
r1749212 improve performance when checking array formulas
r1749226 replace java.awt.Rectangle#intersects with Yaniv's native implementation

Thanks for the patch! Sorry it's been neglected for 21 months.