Skip to content

Incremental persistence: single MERGE per fact table + fix stale-row & ordering bugs - #66

Merged
tombonfert merged 4 commits into
mainfrom
feature/single_merge_for_inc_persistence
Jul 24, 2026
Merged

Incremental persistence: single MERGE per fact table + fix stale-row & ordering bugs#66
tombonfert merged 4 commits into
mainfrom
feature/single_merge_for_inc_persistence

Conversation

@tombonfert

Copy link
Copy Markdown
Collaborator

Summary

Collapses the incremental fact-write from two Delta transactions (replaceWhere + N× upsert) into a single MERGE per fact table, and fixes two bugs in the incremental workflow: stale gold rows left by shrinking containers, and a persist-ordering bug that silently skipped reprocessing of updated containers' calculated channels.

Changes

  • New sink primitive UnityCatalogSink.merge_incremental — unions changed + unchanged rows into one MERGE with a scoped whenNotMatchedBySourceDelete; removes replace_by_ids.
  • persist_facts_incremental now writes one MERGE per table. Delete scope = updated containers (_identify_updated_containers) ∪ changed-definition ids; write gate = new + updated containers (so new containers still insert).
  • Fix — stale rows: a modified container producing fewer rows now has its orphaned gold rows deleted in the same transaction.
  • Fix — ordering: measurement_dimension is written last in _persist_incremental (matching _persist_full), so lazy container detection isn't invalidated mid-run.
  • detect_updated_containers added to ContainerUpsertDetector; Report threads separate processed (new+updated) and updated-only id sets.
  • Tests: rewrote incremental persist unit tests for the single-MERGE API; added new detector tests and an end-to-end shrink regression test (calculated_channel_test.py).

Test Plan

  • Unit tests added/updated
  • Manual testing completed
  • Documentation updated (if applicable)

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • No new linter warnings introduced

…se `merge_incremental`

- Updated `persist_facts_incremental` to consolidate the handling of changed and unchanged DataFrames into a single `merge_incremental` call, improving efficiency and clarity.
- Introduced new parameters for processed and updated container IDs to scope delete operations correctly, ensuring stale rows are pruned during incremental updates.
- Enhanced documentation to reflect the new behavior and added tests to validate the changes, ensuring correct functionality across various scenarios.
- Modified related components in the `Report` class to accommodate the new persistence logic, streamlining the overall data handling process.
@tombonfert
tombonfert requested a review from a team as a code owner July 23, 2026 18:14
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.72%. Comparing base (63c6308) to head (5c37062).

Files with missing lines Patch % Lines
src/impulse_reporting/core/report.py 83.33% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
+ Coverage   88.66%   88.72%   +0.06%     
==========================================
  Files          59       59              
  Lines        4791     4826      +35     
  Branches      547      552       +5     
==========================================
+ Hits         4248     4282      +34     
  Misses        445      445              
- Partials       98       99       +1     
Files with missing lines Coverage Δ
src/impulse_reporting/core/report_utils.py 96.83% <100.00%> (+0.08%) ⬆️
...mpulse_reporting/incremental/container_detector.py 100.00% <100.00%> (+8.00%) ⬆️
src/impulse_reporting/persist/report_storage.py 84.75% <100.00%> (+0.28%) ⬆️
src/impulse_reporting/core/report.py 91.49% <83.33%> (-0.45%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ntal` and `Report` class

- Changed the parameter `processed_container_ids` to `has_processed_containers` in `persist_facts_incremental` to simplify the logic for determining if any containers were recomputed during the run.
- Updated the `Report` class to reflect this change, ensuring consistent handling of processed container states across various persistence calls.
- Adjusted related tests to use the new boolean flag, enhancing clarity and maintainability in the test suite.
- Updated the documentation in `index.md` to clarify the persistence strategy for unchanged and changed definitions, emphasizing the atomic nature of Delta `MERGE` operations.
- Improved the `test_incremental_modified_container_fewer_rows_deletes_stale` test by utilizing a new helper function to streamline the setup of modified containers for incremental tests.
- Added a new test, `test_incremental_modified_container_multi_family_shrink_deletes_stale`, to validate that modified containers producing fewer rows under unchanged definitions correctly delete stale rows across all relevant fact tables while preserving the integrity of non-reprocessed containers.
- Refactored the `clone_silver_with_shrunk_container` helper to enhance clarity and maintainability in test setups.
- Updated documentation in `index.md` to clarify the behavior of the `MERGE` operation with schema evolution, detailing how it handles added and dropped columns during incremental runs.
- Modified the `UnityCatalogSink` class in `report_storage.py` to implement `withSchemaEvolution()`, allowing for seamless integration of schema changes without breaking incremental processing.
- Added a new test case in `histogram_bin_change_test.py` to validate that a fact-schema change (such as adding a new column) correctly evolves the gold table during an incremental MERGE, ensuring data integrity and consistency.
@tombonfert
tombonfert merged commit f654d22 into main Jul 24, 2026
5 checks passed
@tombonfert
tombonfert deleted the feature/single_merge_for_inc_persistence branch July 24, 2026 13:54
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.

1 participant