Skip to content

Preserve sparse edits when selecting DataFrame rows - #332

Merged
vincentlaucsb merged 1 commit into
vincentlaucsb:masterfrom
fhgffy:fix/selected-rows-preserve-edits
Oct 6, 2026
Merged

vincentlaucsb merged 1 commit into
vincentlaucsb:masterfrom
fhgffy:fix/selected-rows-preserve-edits

Conversation

@fhgffy

@fhgffy fhgffy commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve existing sparse cell edits when DataFrame::selected_rows() builds a filtered, unkeyed frame. The current implementation shares parsed CSVRow storage and carries the visible column mapping, but omits the selected rows' edit overlays, silently reverting edited cells to their original parsed values.

The fix snapshots each selected nonempty overlay under its existing lock and stores it independently in the result. Parsed row bytes remain shared; physical column indices and column-name policy are preserved. Later edits to either frame remain independent.

Regression coverage

  • Keyed and unkeyed sources, sparse edits on included/excluded rows, non-identity and non-binary masks
  • Independent source/result edits and repeated selection
  • Column soft deletion plus row erase/insert with physical-index overlays
  • Empty-string edits, unchanged cells, shared raw bytes, column lookup, JSON, exact writer bytes, const mutation rejection
  • Empty/all/invalid masks and empty-frame schema preservation

Validation

The new tests fail on unmodified production base 5a7cc8ff97d23f475adb220f6d268cfd4f843ea2 for the intended loss-of-edits symptom. Validation on GCC 14.2.0 / Linux x86_64:

  • Release C++20 full CTest: 3/3 passed
  • C++11 library configuration full CTest: 3/3 passed; Catch2 v3 test translation units use C++14
  • Native C++11 consumers for regular and officially generated headers passed
  • Generated-header full DataFrame suites: 58 cases passed each in C++14 and C++20
  • C++17 AddressSanitizer + UndefinedBehaviorSanitizer full CTest: 3/3 passed with leak detection disabled because this environment's ptrace blocks LeakSanitizer
  • Independent native C++11/C++20 probes, concurrent snapshots, and focused sanitizers passed
  • Offline clean-base reconstruction, final patch application, source-hash verification, fresh full C++20 suite, and native C++11 consumers passed

MSVC, Clang, other operating systems, ThreadSanitizer and successful leak detection were not tested. Independent review approved the exact final tree.

@fhgffy
fhgffy marked this pull request as ready for review October 5, 2026 05:47
@fhgffy

fhgffy commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

CI is done: 31 checks passed. Both failed jobs passed their tests and generated coverage, then hit TLS errors in the Codecov step: Python and C++. The workflow files are unchanged from the base commit. Both failures are in the coverage-reporting steps.

@vincentlaucsb

Copy link
Copy Markdown
Owner

Thanks for the PR

@vincentlaucsb
vincentlaucsb merged commit 9dbbd2e into vincentlaucsb:master Oct 6, 2026
31 of 33 checks passed
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.

2 participants