Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class support for capturing, serializing, diffing, and emitting indexes on materialized views, fixing a regression where DROP MATERIALIZED VIEW would silently remove indexes during view recreation (issue #235). This extends the dump/comparer pipeline so matview indexes are preserved across dumps and correctly reconciled both when a matview is recreated and when it remains unchanged.
Changes:
- Dump: fetch and store per-materialized-view index metadata alongside the matview object (backwards-compatible with older dumps).
- Compare/emit: recreate matview indexes on view rebuilds; reconcile index adds/drops/redefinitions/comment-only changes in-place when the matview definition is unchanged, including production-mode concurrent index operations.
- Tests/docs/release: add regression schemas + Rust tests, document behavior in test README, and bump version to v1.0.26 with changelog entry.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| data/test/schema_a.sql | Adds FROM-side regression fixtures for matview index recreation/reconciliation paths. |
| data/test/schema_b.sql | Adds TO-side regression fixtures covering recreated, stable-with-index-churn, and TO-only matviews. |
| data/test/README.md | Documents issue #235 regression scenario and expected behaviors (including production-mode concurrency). |
| CHANGELOG | Adds v1.0.26 release notes describing the fix and compatibility behavior. |
| app/src/dump/view.rs | Adds View.indexes, scripts for matviews including indexes, and an index diff plan for unchanged matviews. |
| app/src/dump/view_tests.rs | Unit tests for matview index scripting, hashing behavior, backward-compatible deserialization, and index diff classification. |
| app/src/dump/core.rs | Adds a dedicated query to collect matview indexes and attaches them to dumped materialized views. |
| app/src/dump/core_tests.rs | Validates the matview index query targets relkind m and exposes raw join keys. |
| app/src/comparer/core.rs | Emits matview index changes in-place for unchanged matviews; splits matview index builds/drops for production mode. |
| app/src/comparer/core_tests.rs | End-to-end comparer tests for recreated matviews, in-place index reconciliation, use_drop behavior, and production-mode concurrency. |
| app/Cargo.toml | Bumps crate version to 1.0.26. |
| app/Cargo.lock | Updates lockfile version entry for 1.0.26. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+209
to
+219
| /// Returns a string to create the view, including the indexes of a | ||
| /// materialized view. A materialized view is always dropped and recreated | ||
| /// rather than replaced in place, and `DROP MATERIALIZED VIEW` takes its | ||
| /// indexes with it, so the CREATE has to put them back (issue #235). | ||
| pub fn get_script(&self) -> String { | ||
| let mut script = self.get_script_without_indexes(); | ||
| for index in &self.indexes { | ||
| script.push_str(&index.get_script()); | ||
| } | ||
| script | ||
| } |
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.
Bug fixes:
recreation did not restore its indexes). A
materialized view can be indexed, but the index
rows PostgreSQL reports for one were only ever
distributed into the dump's table list, which
holds ordinary and partitioned tables alone, so
they matched nothing and never reached the
dump. Nothing re-emitted them either: a
materialized view is always dropped and
recreated when its definition changes, and DROP
MATERIALIZED VIEW takes the indexes with it, so
the migration silently left the view unindexed.
Because neither dump carried the indexes the
second diff was empty, and the loss went
unreported. The dump now captures the indexes
of every materialized view alongside the view
itself. A view that is recreated has them
rebuilt with it, and one whose definition did
not change has its index set reconciled in
place (created, dropped, redefined or
re-commented) so that adding an index no longer
forces a full rebuild of the view's contents.
A view restored after DROP FUNCTION ... CASCADE
gets the same treatment, and with
--output-for-production the builds and drops
run CONCURRENTLY after the transaction commits,
exactly as a table's do. Dumps written by older
versions carry no index data; they stay
readable and simply report no indexes.