xinzweb opened a new pull request, #1970:
URL: https://github.com/apache/cloudberry/pull/1970
Fixes #726
### What does this PR do?
`gp_matview_aux.mvname` is populated from a bare, non-schema-qualified
relation name (`InsertMatviewAuxEntry()`, `gp_matview_aux.c`), so two
materialized views sharing a name in different schemas produce identical,
indistinguishable `mvname` values.
This PR adds `gp_matviews`, modeled on `pg_matviews`, which live-joins
`gp_matview_aux` → `pg_class` → `pg_namespace` for an always-correct
schema-qualified name (`mvschema`/`mvname`). `gp_matview_aux.mvname` is
left completely unchanged — still populated, still synced on rename via
the existing `mvaux_rename()` — and marked deprecated via `COMMENT ON
COLUMN`, pointing callers at the new view. This is purely additive: no
column removed, no existing behavior changed.
I considered (and rejected) two alternatives:
- **Prefix schema into the existing `mvname` column** (the maintainer's
literal suggestion on the introducing PR, #720) — `mvname` is
`NameData` (63 usable bytes); a schema prefix could silently truncate
or re-collide for long identifiers.
- **Add a new stored, synced `mvschema` column** instead of a view — I
verified (`grep`) that no code anywhere syncs a matview's schema on
`ALTER ... SET SCHEMA` today, since `mvname` never tracked schema. A
new synced column would need that sync code added from scratch, and if
ever missed, would reproduce the exact staleness bug this issue is
about. A live-resolved view can't go stale by construction.
Root cause: `namestrcpy(&mvname, get_rel_name(mvoid))` at
`gp_matview_aux.c:253` never schema-qualifies. Introduced in #720
(merged 2024-12-02); flagged same-day in that PR's own review by
@yjhnupt, with @yjhjstz suggesting the schema-prefix approach above —
the follow-up sat unactioned for ~20 months.
### Type of Change
- [x] Bug fix (non-breaking change)
- [ ] New feature (non-breaking change)
- [ ] Breaking change (fix or feature with breaking changes)
- [ ] Documentation update
### Test Plan
- [x] Unit tests added/updated — new self-contained schema-collision
test in `matview_data.sql` (create `mv0` in two schemas, confirm
`gp_matview_aux.mvname` can't distinguish them but
`gp_matviews.mvschema` can; confirm `ALTER ... RENAME` resolves
live)
- [ ] Integration tests added/updated
- [ ] Passed `make installcheck`
- [ ] Passed `make -C src/test installcheck-cbdb-parallel`
Neither `make installcheck` command was run as literally stated — see
Additional Context for what was actually run and why.
### Impact
**Performance:** None — purely additive, no existing code path changed.
**User-facing changes:** New `gp_matviews` view available;
`gp_matview_aux.mvname` now documented (via `COMMENT ON COLUMN`) as
deprecated, but unchanged in behavior.
**Dependencies:** None.
### Checklist
- [x] Followed [contribution
guide](https://cloudberry.apache.org/contribute/code)
- [ ] Added/updated documentation
- [x] Reviewed code for security implications
- [x] This PR contains AI-assisted code generation
- [ ] Requested review from [cloudberry
committers](https://github.com/orgs/apache/teams/cloudberry-committers)
### Additional Context
**AI disclosure:** this change was drafted end-to-end by Claude Code
under my direction — design (including a design pivot from an initial
breaking-change approach after I asked about it), implementation, and
this PR description. I reviewed the diff and the live-cluster evidence
below before submitting; I'm accountable for what's in this PR.
**What was actually run, honestly:**
- `mvn apache-rat:check` — passed clean (0 unapproved).
- Built from source and validated against a real, live 6-segment
`gpdemo` cluster via a Docker-based local toolchain, not the official
CI matrix. Confirmed live: the #726 scenario (`gp_matview_aux.mvname`
can't distinguish two same-named matviews in different schemas;
`gp_matviews.mvschema` can), `ALTER ... RENAME` resolves live, the new
`COMMENT ON` deprecation notices are queryable
(`col_description()`/`obj_description()`), catalog version bootstraps
cleanly, `-Werror` compiles clean.
- `matview_data.sql` was piped through `psql` directly against the live
cluster (not `pg_regress`/`make installcheck` — not wired up for this
ad hoc cluster) and the new test's expected output spliced into
`matview_data.out` at its exact insertion point — confirmed via diff
against the pristine upstream file that this is a pure addition (0
deletions).
- `misc_sanity.sql` diffed byte-identical against its checked-in
expected output.
- This design doesn't touch `aqumv.sql`, `pax_storage`, or
`singlenode_regress` at all (only the now-superseded removal design
needed to) — nothing to test there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
