Skip to content

fix: compact sparse worksheet rows in linear time#2334

Closed
rootsec1 wants to merge 1 commit into
qax-os:masterfrom
rootsec1:codex/fix-trim-row-sparse-save
Closed

fix: compact sparse worksheet rows in linear time#2334
rootsec1 wants to merge 1 commit into
qax-os:masterfrom
rootsec1:codex/fix-trim-row-sparse-save

Conversation

@rootsec1
Copy link
Copy Markdown
Contributor

@rootsec1 rootsec1 commented Jun 4, 2026

What changed

This changes trimRow to compact worksheet rows in place with a write index instead of deleting each empty row from the middle of the backing slice.

The observable behavior is the same: empty rows are removed, cells inside retained rows are still trimmed, and rows with attributes such as hidden are preserved.

Why

Some valid XLSX files have a very sparse worksheet row list, for example when the sheet dimension reaches Excel's maximum row because of a single far-down cell. In that case, many intermediate rows can be empty.

The previous implementation used:

sheetData.Row = append(sheetData.Row[:k], sheetData.Row[k+1:]...)

inside the scan loop. When many empty rows are present, that repeatedly shifts the remaining slice contents and can make saving sparse worksheets take quadratic time.

How this fixes it

The new implementation performs one forward pass:

  • trim cells in each row
  • copy retained rows to the next write position
  • clear the unused tail so discarded rows can be collected
  • return the compacted slice

This avoids repeated large memory moves while preserving row order and the existing trim semantics.

Tests

Added synthetic coverage only; no external or private workbook fixtures are included.

  • TestTrimRowCompactsSparseRows verifies blank rows and blank cells are removed while rows with attributes are retained.
  • BenchmarkTrimRowSparseRows covers a sparse worksheet shape with 50,000 row entries and only a small number of retained rows.

Local verification:

go test ./...
go test ./... -run '^$' -bench '^BenchmarkTrimRowSparseRows$' -benchmem

@rootsec1
Copy link
Copy Markdown
Contributor Author

rootsec1 commented Jun 4, 2026

Closing to reopen from a neutral branch name.

@rootsec1 rootsec1 closed this Jun 4, 2026
@rootsec1 rootsec1 deleted the codex/fix-trim-row-sparse-save branch June 4, 2026 17:01
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.60%. Comparing base (875b959) to head (cf3ea8d).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2334      +/-   ##
==========================================
- Coverage   99.60%   99.60%   -0.01%     
==========================================
  Files          32       32              
  Lines       26866    26864       -2     
==========================================
- Hits        26760    26758       -2     
  Misses         55       55              
  Partials       51       51              
Flag Coverage Δ
unittests 99.60% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xuri xuri added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants