Bug 56958

Summary: [PATCH] XSSFSheet.validateArrayFormulas incorrect and incomplete range intersection check
Product: POI Reporter: Yaniv Kunda <yaniv>
Component: XSSFAssignee: POI Developers List <dev>
Severity: normal Keywords: PatchAvailable
Priority: P2    
Version: 3.11-dev   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: fixes incorrect intersection check
fixes incorrect and incomplete intersection check

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.