Enhancement: add gp_matviews, a schema-qualified view over gp_matview_aux by xinzweb · Pull Request #1970 · apache/cloudberry

xinzweb · 2026-09-07T13:21:38Z

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, Fix REFRESH fast path. #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

Bug fix (non-breaking change)
New feature (non-breaking change)
Breaking change (fix or feature with breaking changes)
Documentation update

Test Plan

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

Followed contribution guide
Added/updated documentation
Reviewed code for security implications
This PR contains AI-assisted code generation
Requested review from 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 mvname column in gp_matview_aux may be confused. #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.

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. Flagged same-day in review on the PR that introduced it (apache#720) but never actioned. Add 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) instead of the stored, non-qualified copy. Because it's resolved live rather than synced, ALTER MATERIALIZED VIEW ... RENAME and SET SCHEMA can never leave it stale - there is no sync code to miss, unlike a second stored column would require (verified: no code anywhere handles gp_matview_aux on ALTER TABLE ... SET SCHEMA today, since mvname never tracked schema in the first place - a new synced column would need to add that from scratch and could reproduce the same staleness bug this fixes). gp_matview_aux.mvname itself is left completely unchanged - still populated, still synced on rename via the existing mvaux_rename() - and marked deprecated via COMMENT ON COLUMN, pointing at gp_matviews. This is additive only: no column removed, no existing behavior changed, nothing breaks for any existing direct consumer of mvname. Fixes apache#726 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PJKwGjLr1ee53Uh8aq1v2H

Captured against a real live 6-segment gpdemo cluster (1 coordinator + 1 standby + 2 primaries + 2 mirrors), built from this branch, via psql -X -a -q -d regression < matview_data.sql (pg_regress's own harness isn't wired up for this ad hoc Docker-built cluster). Pure addition at the exact insertion point in matview_data.sql - confirmed via diff against the pristine upstream expected file that nothing else changed (0 deletions, 66 insertions). Also confirmed live: misc_sanity.sql diffs byte-identical against its checked-in expected output (the new view/comments don't trip any existing sanity check), and aqumv.sql's diff against its checked-in expected output is 100% pre-existing drift unrelated to this patch (GUC-ordering in EXPLAIN VERBOSE, new DISTRIBUTED BY notices, topology-based Motion labels) - grepped for gp_matview/mvname/mvschema in that diff and found zero hits, so aqumv.out is intentionally left untouched here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PJKwGjLr1ee53Uh8aq1v2H

apache/cloudberry#1970 opened against xinzweb:t726-remove-mvname-column. Flips hub status Coding -> Review and records the cross-repo pointer. Also notes the local validation cluster container was torn down now that it's no longer needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PJKwGjLr1ee53Uh8aq1v2H

github-actions

Hi, @xinzweb welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!

yjhjstz · 2026-09-07T13:59:16Z

+
+COMMENT ON VIEW gp_matviews IS 'Schema-qualified view of gp_matview_aux, resolving mvname live from pg_class/pg_namespace instead of a stored copy. Prefer this over gp_matview_aux.mvname, which is not schema-qualified (see https://github.com/apache/cloudberry/issues/726).';
+
+COMMENT ON COLUMN gp_matview_aux.mvname IS 'Deprecated: bare, non-schema-qualified materialized view name, retained for backward compatibility only. Two materialized views with the same name in different schemas are indistinguishable via this column. Use gp_matviews.mvname (with gp_matviews.mvschema) instead. See https://github.com/apache/cloudberry/issues/726.';


this is too long for COMMENT?

Shine Love Jean and others added 2 commits September 3, 2026 13:53

xinzweb mentioned this pull request Sep 7, 2026

docs(tasks): T20260828-134911 status=Review, PR opened xinzweb/apache-skills#30

Merged

github-actions Bot reviewed Sep 7, 2026

View reviewed changes

yjhjstz reviewed Sep 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

Enhancement: add gp_matviews, a schema-qualified view over gp_matview_aux - #1970

Enhancement: add gp_matviews, a schema-qualified view over gp_matview_aux#1970
xinzweb wants to merge 2 commits into
apache:mainfrom
xinzweb:t726-remove-mvname-column

xinzweb commented Sep 7, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

yjhjstz Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		COMMENT ON VIEW gp_matviews IS 'Schema-qualified view of gp_matview_aux, resolving mvname live from pg_class/pg_namespace instead of a stored copy. Prefer this over gp_matview_aux.mvname, which is not schema-qualified (see https://github.com/apache/cloudberry/issues/726).';

		COMMENT ON COLUMN gp_matview_aux.mvname IS 'Deprecated: bare, non-schema-qualified materialized view name, retained for backward compatibility only. Two materialized views with the same name in different schemas are indistinguishable via this column. Use gp_matviews.mvname (with gp_matviews.mvschema) instead. See https://github.com/apache/cloudberry/issues/726.';

Uh oh!

Conversation

xinzweb commented Sep 7, 2026

What does this PR do?

Type of Change

Test Plan

Impact

Checklist

Additional Context

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

yjhjstz Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants