Skip to content

fix: preserve sparse map row values during export - #1299

Open
qiaolezi001 wants to merge 2 commits into
apache:mainfrom
qiaolezi001:fix/fesod-1296-sparse-map
Open

qiaolezi001 wants to merge 2 commits into
apache:mainfrom
qiaolezi001:fix/fesod-1296-sparse-map

Conversation

@qiaolezi001

Copy link
Copy Markdown
Contributor

Purpose of the pull request

Fixes #1296.

A sparse row such as {0: "FIRST", 2: "THIRD"} silently loses its third-column value because MapRowData.size() reports entry count while the writer consumes numeric data positions. Preserve the complete logical row span without requiring explicit null entries for gaps.

What's changed?

  • Compute the indexed row extent once when wrapping the map, including gaps up to the final integer key. Keep value lookups and the existing head-to-column mapping.
  • Preserve collection rows, dense maps and indexed head behavior. Row callbacks execute before wrapping and their added values are covered by regression tests. An unrepresentable integer span fails explicitly instead of overflowing.
  • Clarify the RowData.size() contract and add English/Chinese sparse-map examples.
  • Cover XLS, XLSX and CSV, both memory modes, headers, absent column zero, reversed map iteration order, explicit/trailing nulls, indexed heads and row callbacks.

The row extent is captured when the map is wrapped; adding/removing input columns from a later per-cell callback does not recalculate it. This matches collection rows' fixed extent and avoids rescanning all keys for every head entry.

Validation

JDK 25, project Java 8 source/target:

  • Baseline: 15 of 18 public write/read regression cases fail; the three collection controls pass.
  • Fix: all 26 new cases pass, including five row-data boundary tests and row-handler coverage.
  • ./mvnw clean package -pl fesod-common,fesod-shaded,fesod-sheet: 1,875 tests, zero failures/errors/skips, no test groups excluded; normal fuzz seed regressions included.
  • Independent POI/Commons CSV reopen of all 12 original sparse/dense format/header probes passes.
  • Repository-wide spotless:check, Checkstyle, Hawkeye 7.0.0, cached/offline Apache RAT 0.15 and git diff --check pass.

No local five-JDK matrix, website build or timed fuzzing campaign is claimed; the project CI covers the JDK matrix. RAT's initial online dependency lookup failed on DNS and the same goal was then run successfully across all six reactor projects using the existing local cache.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Generated-by: AI

@yebuguntang

yebuguntang commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

I independently verified this fix.

Reproduction of #1296 on my local pre-fix main (b87181c8): writing {0: "FIRST", 2: "THIRD"} yields A1=FIRST, C1=null. Against this PR's head (4b6d5a17), the same case returns C1=THIRD, with the middle column correctly left blank.

I also ran a full reactor build on the PR branch (./mvnw clean package): BUILD SUCCESS across all six modules — parent, BOM, Common, Shaded, Spreadsheet and Benchmark — with 0 failures / 0 errors / 0 skipped. The PR's own regression tests (SparseMapDataTest, MapRowDataTest) were included in that build and passed as well; fuzz tests were included too.

Environment: Windows, Microsoft OpenJDK 21.0.10, Maven 3.6.3.

Happy to help verify anything else if useful.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the sparse-map export fix.

No open issues 🎉

What changed in this PR

Fixes #1296 by preserving values after gaps in integer-keyed map rows during spreadsheet export, while retaining existing collection-row and indexed-head behavior.

Changes:

  • Capture each map row’s indexed extent once, including gaps, and reject spans that would overflow.
  • Add boundary and round-trip regression tests across XLS, XLSX and CSV, including headers, memory modes and row callbacks.
  • Clarify the row-size contract and document sparse-map usage in English and Chinese.
File Description
website/​i18n/​zh-cn/​docusaurus-plugin-content-docs/​current/​sheet/​write/​pojo.md Adds Chinese sparse-map guidance and an example.
website/​docs/​sheet/​write/​pojo.md Adds English sparse-map guidance and an example.
fesod-sheet/​src/​test/​java/​org/​apache/​fesod/​sheet/​write/​metadata/​MapRowDataTest.java Tests sparse extents, null values and integer boundaries.
fesod-sheet/​src/​test/​java/​org/​apache/​fesod/​sheet/​core/​SparseMapDataTest.java Adds cross-format export regressions and compatibility checks.
fesod-sheet/​src/​main/​java/​org/​apache/​fesod/​sheet/​write/​metadata/​RowData.java Documents size as the exclusive upper bound of data indexes.
fesod-sheet/​src/​main/​java/​org/​apache/​fesod/​sheet/​write/​metadata/​MapRowData.java Caches the indexed row extent and guards against overflow.

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Sparse column-indexed maps silently lose values during export

4 participants