From: Michael Paquier <michael@paquier.xyz>
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18674: Partitioned table doesn't depend on access method it uses
Date: Mon, 28 Oct 2024 16:23:33 +0900
Message-ID: <Zx879T_oFkiHv9St@paquier.xyz> (raw)
In-Reply-To: <CALdSSPjGzuo0eodxdP9WZS4Q7nvv-2k3W=9ie2XoMedBjkr1Fw@mail.gmail.com>
References: <18674-1ef01eceec278fab@postgresql.org>
<CALdSSPgvKRX1pZE7GLKpDESw0wbBH4qBZPQSdSKYY2btCrfuSQ@mail.gmail.com>
<CALdSSPjGzuo0eodxdP9WZS4Q7nvv-2k3W=9ie2XoMedBjkr1Fw@mail.gmail.com>
On Mon, Oct 28, 2024 at 01:23:23AM +0500, Kirill Reshke wrote:
> Seems that simply changing RELKIND_HAS_TABLE_AM marco is too invasive.
> However, I will make another try. In v2 version, regression tests
> still fail, i will adjust them, if patching this way is the right
> approach to fix the problem..
>
> script provided by Alexander:
> CREATE ACCESS METHOD ham TYPE TABLE HANDLER heap_tableam_handler;
> CREATE TABLE pt (a int) PARTITION BY LIST (a) USING ham;
> DROP ACCESS METHOD ham;
#define RELKIND_HAS_TABLE_AM(relkind) \
((relkind) == RELKIND_RELATION || \
+ (relkind) == RELKIND_PARTITIONED_TABLE || \
(relkind) == RELKIND_TOASTVALUE || \
(relkind) == RELKIND_MATVIEW)
This is an incorrect approach to the problem. RELKIND_HAS_TABLE_AM()
should be true only for relkinds that have rd_tableam set in their
relcache entries. However, you are breaking the original promise of
this macro because it is not set (and should not be set) for
partitioned tables as these have no physical presence on disk.
pg_class.relam is just a reference to use when creating partitions on
it. Note the comment in pg_class.h explaining the case of partitioned
tables.
Using an ALTER TABLE .. SET ACCESS METHOD on the partitioned table and
attempting a DROP ACCESS METHOD would fail. We are missing a normal
dependency entry in pg_depend for your sequence of commands between
the new partitioned table with the USING clause and its access method
set.
The path of adding the dependency between an AM and a table/matview is
heap_create_with_catalog() when these are defined. We can just expand
that for partitioned tables and simply fix the problem. Attached is a
patch to do that, with tests around create_am.sql based on USING
checking the contents of pg_depend.
--
Michael
Attachments:0001-Fix-dependency-of-partitioned-table-and-its-table-AM.patch (text/x-diff, 3.1K) download
From 3bdc73cb92633cf1651d4c7e30691ac13809ef91 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 28 Oct 2024 16:11:49 +0900
Subject: [PATCH] Fix dependency of partitioned table and its table AM under
USING
---
src/backend/catalog/heap.c | 3 ++-
src/test/regress/expected/create_am.out | 16 ++++++++++++++++
src/test/regress/sql/create_am.sql | 12 ++++++++++++
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.cindex 0078a12f26..c815019241 100644--- a/src/backend/catalog/heap.c+++ b/src/backend/catalog/heap.c@@ -1473,7 +1473,8 @@ heap_create_with_catalog(const char *relname,
* No need to add an explicit dependency for the toast table, as the
* main table depends on it.
*/
- if (RELKIND_HAS_TABLE_AM(relkind) && relkind != RELKIND_TOASTVALUE)+ if ((RELKIND_HAS_TABLE_AM(relkind) && relkind != RELKIND_TOASTVALUE) ||+ relkind == RELKIND_PARTITIONED_TABLE)
{
ObjectAddressSet(referenced, AccessMethodRelationId, accessmtd);
add_exact_object_address(&referenced, addrs);
diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.outindex 35d4cf1d46..08a3eec142 100644--- a/src/test/regress/expected/create_am.out+++ b/src/test/regress/expected/create_am.out@@ -343,6 +343,22 @@ ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
ERROR: cannot have multiple SET ACCESS METHOD subcommands
DROP MATERIALIZED VIEW heapmv;
DROP TABLE heaptable;
+-- Partitioned table with USING+BEGIN;+CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x) USING heap2;+SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+ obj | refobj +----------------------+---------------------+ table am_partitioned | access method heap2+(1 row)++DROP TABLE am_partitioned;+COMMIT;
-- Partition hierarchies with access methods
BEGIN;
SET LOCAL default_table_access_method = 'heap';
diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sqlindex 825aed325e..c43637227e 100644--- a/src/test/regress/sql/create_am.sql+++ b/src/test/regress/sql/create_am.sql@@ -217,6 +217,18 @@ ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
DROP MATERIALIZED VIEW heapmv;
DROP TABLE heaptable;
+-- Partitioned table with USING+BEGIN;+CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x) USING heap2;+SELECT pg_describe_object(classid, objid, objsubid) AS obj,+ pg_describe_object(refclassid, refobjid, refobjsubid) as refobj+ FROM pg_depend, pg_am+ WHERE pg_depend.refclassid = 'pg_am'::regclass+ AND pg_am.oid = pg_depend.refobjid+ AND pg_depend.objid = 'am_partitioned'::regclass;+DROP TABLE am_partitioned;+COMMIT;+
-- Partition hierarchies with access methods
BEGIN;
SET LOCAL default_table_access_method = 'heap';
--
2.45.2
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, reshkekirill@gmail.com, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18674: Partitioned table doesn't depend on access method it uses
In-Reply-To: <Zx879T_oFkiHv9St@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