Repository navigation
Conversation
Merged cells only hold a value in the first cell of the range, and the reader had no way to discover the ranges. They are stored in <mergeCells>, which follows <sheetData> in the sheet XML. RowSpliterator now reads the ranges in the same pass once it reaches the end of sheetData, so there is no second parse of the sheet and no per-cell lookup. Sheet.getMergedCells() returns them once the rows have been read to the end, and throws IllegalStateException before then. FastExcelReaderTest compares the ranges with POI's getMergedRegions() for every test file, and MergedCellsTest covers a sheet without merged cells and reading the ranges before or partway through the rows. Fixes dhatim#633
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #633
This change adds
Sheet.getMergedCells(). This method returns the merged cell ranges of a sheet asList<CellRangeAddress>.How it works
In the sheet XML,
<mergeCells>is immediately after<sheetData>. WhenRowSpliteratorgets to the end ofsheetData, it continues with the same reader. It reads each<mergeCell ref="...">range and gives the list to theSheet. The results are:getMergedCells()returns the ranges after all rows are read. UseSheet.read()or read all of a stream fromopenStream().getMergedCells()throwsIllegalStateException.This method prevents the performance problems that #323 has.
Performance
I ran
ReaderBenchmarkine2e(mvn test -Pbench) on master and on this branch. Each run has 15 single-shot samples.The fastexcel difference is 2.6%. The POI difference between the two runs is 6.3%, but this change has no effect on POI. Thus, the fastexcel difference is less than the variation between runs.
calendar_stress_test.xlsxhas no merged cells. Thus, this benchmark measures the usual case.I also tested a sheet with 20,000 rows, 20 columns and 2,000 merged ranges. I did not use JMH for this test. To read all rows took 147.3 ms before the change. It took 151.0 ms after the change, and this time includes the merged ranges.
Possibly, you do not want extra work at the end of each sheet. If so, I can make this feature optional with
ReadingOptions.Tests
FastExcelReaderTest.testFilecomparesgetMergedCells()withgetMergedRegions()from POI. It does this for all test files, which includemerge_cells.xlsx.MergedCellsTesttests these conditions:getMergedCells()before the rows are readgetMergedCells()after only some rows are read