Continue a full XLSX sheet on another sheet instead of dropping its rows - #362
Open
dchaudhari7177 wants to merge 1 commit into
Open
dchaudhari7177 wants to merge 1 commit into
dchaudhari7177 wants to merge 1 commit into
Conversation
xlsxwriter returns -1 for a write past row 1,048,576 and leaves the cell out of the file without raising, so a Timeline or Storage sheet filled from a large run silently lost everything past the limit. Every XLSX sheet now goes through RolloverWorksheet, which maps the writer's running row number onto the sheet with room for it and, when one fills, adds "Name (2)" with the same title bar, headers, column widths and frozen panes. Each sheet gets its own autofilter, the rollover is logged, and tab ordering keeps continuations next to the sheet they continue. Closes RyanDFIR#359
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.
Closes #359.
What changed
This takes the preferred fix from the issue: a sheet that fills up continues on another sheet, so nothing is dropped.
RolloverWorksheetinanalysis.pywraps a worksheet and takes the same calls the writers already make:write,write_string,write_number,merge_range,set_column,freeze_panesandautofilter. Those seven are every methodgenerate_excelcalls on a sheet. The writers keep one running row number for a whole collection, and the wrapper maps it onto the sheet with room for it:get_unique_sheet_name(Timeline (2),Storage (2), …) gets the recorded setup replayed. The row then goes there and a warning is logged:XLSX sheet "Storage" reached 1,048,576 rows; continuing on "Storage (2)".autofiltergives each full sheet a filter down to its last row, and the last sheet keeps the writer's row, so a single-sheet workbook gets exactly the filter it got before._tab_ordernow strips a trailing(N)before matching, so continuations sort next to the sheet they continue instead of falling to the end.All eight sheets use it, not only Timeline and Storage. Every one of them is filled from a collection with a running row number and uses the same two header rows, so the change is one line per sheet. A merged range in the data has to fit on one sheet, which holds for the single-row group headers on Service Workers, Sessions and Extensions.
I left the per-store IndexedDB cap from #348 as it is. It bounds memory and parse time as well as the sheet, so removing it deserves its own decision.
Checks
At the boundary, stock xlsxwriter 3.2.9 behaves as the issue describes:
tests/test_xlsx_row_limit.py:TestXlsxRowLimitpatchesXLSX_MAX_ROWSto 6 (two header rows plus four data rows) and runsgenerate_excelwith 10 Timeline cookies and 10 Local Storage records. It checks thatTimeline,Timeline (2),Timeline (3)andStorage…Storage (3)sit together in tab order, that every record appears exactly once and in order across the three sheets, that no sheet exceeds the limit, and that continuations repeat the header row, title bar, frozen panes and column width. Each sheet's autofilter must beA2:…6, A2:…6, A2:…5, and four rollovers must be logged.TestXlsxRowLimitBoundaryuses the real limit. Writing at zero-based row 1,048,575 returns 0 on the first sheet, and 1,048,576 returns 0 and lands onStorage (2)under the repeated header. Because the wrapper maps row numbers directly, this needs only three writes, not a million.The existing XLSX tests (
test_xlsx_none_safety, andTestXlsxColumnsintest_hashing, which reads column positions and the autofilter range) pass unchanged. Wrapped sheets are stillNoneSafeWorksheets, because they come fromworkbook.add_worksheetwith the sameworksheet_class.