From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18000: Access method used by matview can be dropped leaving broken matview
Date: Wed, 28 Jun 2023 08:11:51 +0900
Message-ID: <ZJtst5BUaRnH6JSj@paquier.xyz> (raw)
In-Reply-To: <20230627182332.uq6afduhvdip7pol@awork3.anarazel.de>
References: <18000-9145c25b1af475ca@postgresql.org>
<ZJlN4DFm4FvVxe6v@paquier.xyz>
<ZJpqh3xoFtlwZS9u@paquier.xyz>
<ZJqCusodr0jYZgMq@paquier.xyz>
<20230627182332.uq6afduhvdip7pol@awork3.anarazel.de>
On Tue, Jun 27, 2023 at 11:23:32AM -0700, Andres Freund wrote:
> Should we check changeDependencyFor()'s return value? On a first glance it
> looks like it would be an error to return 0 in this case?
Agreed. I was wondering if this was worth having in this path (other
code paths are rather lax about that), but yes let's do it as there
should not be more than one record in pg_depend.
--
Michael
Attachments:v2-0001-Fix-dependencies-under-ALTER-TABLE-SET-ACCESS-MET.patch (text/x-diff, 4.8K) download
From 6ef48042e1eefd30b16438c03e584b41733b3e49 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 28 Jun 2023 08:06:31 +0900
Subject: [PATCH v2] Fix dependencies under ALTER TABLE SET ACCESS METHOD
---
src/backend/commands/cluster.c | 21 ++++++++++++++++++
src/test/regress/expected/create_am.out | 29 +++++++++++++++++++++++++
src/test/regress/sql/create_am.sql | 18 +++++++++++++++
3 files changed, 68 insertions(+)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.cindex 3bfabb6d10..e84460a43e 100644--- a/src/backend/commands/cluster.c+++ b/src/backend/commands/cluster.c@@ -1070,6 +1070,8 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
relfilenumber2;
RelFileNumber swaptemp;
char swptmpchr;
+ Oid relam1,+ relam2;
/* We need writable copies of both pg_class tuples. */
relRelation = table_open(RelationRelationId, RowExclusiveLock);
@@ -1086,6 +1088,8 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
relfilenumber1 = relform1->relfilenode;
relfilenumber2 = relform2->relfilenode;
+ relam1 = relform1->relam;+ relam2 = relform2->relam;
if (RelFileNumberIsValid(relfilenumber1) &&
RelFileNumberIsValid(relfilenumber2))
@@ -1257,6 +1261,23 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
CacheInvalidateRelcacheByTuple(reltup2);
}
+ /*+ * Now that pg_class has been updated with its relevant information for+ * the swap, update the dependency of the relation to point to its new+ * table AM, if it has changed.+ */+ if (relam1 != relam2)+ {+ if (changeDependencyFor(RelationRelationId,+ r1,+ AccessMethodRelationId,+ relam1,+ relam2) != 1)+ elog(ERROR, "failed to change access method dependency for relation \"%s.%s\"",+ get_namespace_name(get_rel_namespace(r1)),+ get_rel_name(r1));+ }+
/*
* Post alter hook for modified relations. The change to r2 is always
* internal, but r1 depends on the invocation context.
diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.outindex e9a9283d7a..4eec292e65 100644--- a/src/test/regress/expected/create_am.out+++ b/src/test/regress/expected/create_am.out@@ -240,6 +240,35 @@ SELECT amname FROM pg_class c, pg_am am
heap
(1 row)
+-- Switching to heap2 adds new dependency entry to the AM.+ALTER TABLE heaptable SET ACCESS METHOD heap2;+SELECT pg_describe_object(classid, objid, objsubid) as obj,+ pg_describe_object(refclassid,refobjid,refobjsubid) as objref,+ deptype+ FROM pg_depend+ WHERE classid = 'pg_class'::regclass AND+ objid = 'heaptable'::regclass+ ORDER BY 1, 2;+ obj | objref | deptype +-----------------+---------------------+---------+ table heaptable | access method heap2 | n+ table heaptable | schema public | n+(2 rows)++-- Switching to heap should not have a dependency entry to the AM.+ALTER TABLE heaptable SET ACCESS METHOD heap;+SELECT pg_describe_object(classid, objid, objsubid) as obj,+ pg_describe_object(refclassid,refobjid,refobjsubid) as objref,+ deptype+ FROM pg_depend+ WHERE classid = 'pg_class'::regclass AND+ objid = 'heaptable'::regclass+ ORDER BY 1, 2;+ obj | objref | deptype +-----------------+---------------+---------+ table heaptable | schema public | n+(1 row)+
ALTER TABLE heaptable SET ACCESS METHOD heap2;
SELECT amname FROM pg_class c, pg_am am
WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sqlindex 256884c959..f1c809b8e0 100644--- a/src/test/regress/sql/create_am.sql+++ b/src/test/regress/sql/create_am.sql@@ -166,6 +166,24 @@ CREATE TABLE heaptable USING heap AS
SELECT a, repeat(a::text, 100) FROM generate_series(1,9) AS a;
SELECT amname FROM pg_class c, pg_am am
WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
+-- Switching to heap2 adds new dependency entry to the AM.+ALTER TABLE heaptable SET ACCESS METHOD heap2;+SELECT pg_describe_object(classid, objid, objsubid) as obj,+ pg_describe_object(refclassid,refobjid,refobjsubid) as objref,+ deptype+ FROM pg_depend+ WHERE classid = 'pg_class'::regclass AND+ objid = 'heaptable'::regclass+ ORDER BY 1, 2;+-- Switching to heap should not have a dependency entry to the AM.+ALTER TABLE heaptable SET ACCESS METHOD heap;+SELECT pg_describe_object(classid, objid, objsubid) as obj,+ pg_describe_object(refclassid,refobjid,refobjsubid) as objref,+ deptype+ FROM pg_depend+ WHERE classid = 'pg_class'::regclass AND+ objid = 'heaptable'::regclass+ ORDER BY 1, 2;
ALTER TABLE heaptable SET ACCESS METHOD heap2;
SELECT amname FROM pg_class c, pg_am am
WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
--
2.40.1
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, andres@anarazel.de, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18000: Access method used by matview can be dropped leaving broken matview
In-Reply-To: <ZJtst5BUaRnH6JSj@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox