pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: ALTER TABLE on system catalogs
Date: Wed, 27 Jun 2018 22:31:30 +0200
Message-ID: <e49f825b-fb25-0bc8-8afc-d5ad895c7975@2ndquadrant.com> (raw)

ALTER TABLE on system catalogs is occasionally useful, for example

    ALTER TABLE pg_attribute SET (autovacuum_vacuum_scale_factor=0);

However, this doesn't actually work.  The above command produces

    ERROR:  AccessExclusiveLock required to add toast table.

If it's a shared catalog, it will produce

    ERROR:  shared tables cannot be toasted after initdb

In other cases it will work but then silently add a TOAST table to a
catalog, which I think we don't want.

The problem is that for (almost) any ALTER TABLE command, it afterwards
checks if the just-altered table now needs a TOAST table and tries to
add it if so, which will either fail or add a TOAST table that we don't
want.

I propose that we instead silently ignore attempts to add TOAST tables
to shared and system catalogs after bootstrapping.  This fixes the above
issues.  I have attached a patch for this, and also a test that
enshrines which system catalogs are supposed to have TOAST tables.

As an alternative, I tried to modify the ALTER TABLE code to avoid the
try-to-add-TOAST-table path depending on what ALTER TABLE actions were
done, but that seemed incredibly more complicated and harder to maintain
in the long run.

(You still need allow_system_table_mods=on for all of this.  Perhaps
that's also worth revisiting, but it's a separate issue.)

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments: download all (2 files, .tar.gz)

0001-Add-test-for-system-catalog-TOAST-tables.patch (text/plain, 1.9K) download From fa75f6573a37f4065f0ba16b1ef8a55e8b933af6 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter_e@gmx.net> Date: Fri, 18 May 2018 19:07:53 -0400 Subject: [PATCH 1/5] Add test for system catalog TOAST tables This records which system catalogs are supposed to have a TOAST table. --- src/test/regress/expected/misc_sanity.out | 22 ++++++++++++++++++++++ src/test/regress/sql/misc_sanity.sql | 10 ++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out index 5aaae6c39f..a1ea485a15 100644 --- a/src/test/regress/expected/misc_sanity.out +++ b/src/test/regress/expected/misc_sanity.out @@ -76,3 +76,25 @@ NOTICE: pg_database contains unpinned initdb-created object(s) NOTICE: pg_extension contains unpinned initdb-created object(s) NOTICE: pg_rewrite contains unpinned initdb-created object(s) NOTICE: pg_tablespace contains unpinned initdb-created object(s) +-- **************** general **************** +-- check which system catalogs have toast tables +SELECT relname +FROM pg_class +WHERE relnamespace = 'pg_catalog'::regnamespace AND reltoastrelid <> 0 +ORDER BY 1; + relname +-------------------- + pg_attrdef + pg_constraint + pg_db_role_setting + pg_description + pg_proc + pg_rewrite + pg_seclabel + pg_shdescription + pg_shseclabel + pg_statistic + pg_statistic_ext + pg_trigger +(12 rows) + diff --git a/src/test/regress/sql/misc_sanity.sql b/src/test/regress/sql/misc_sanity.sql index b921117fa5..b9adcbe4e0 100644 --- a/src/test/regress/sql/misc_sanity.sql +++ b/src/test/regress/sql/misc_sanity.sql @@ -72,3 +72,13 @@ end if; end loop; end$$; + + +-- **************** general **************** + +-- check which system catalogs have toast tables + +SELECT relname +FROM pg_class +WHERE relnamespace = 'pg_catalog'::regnamespace AND reltoastrelid <> 0 +ORDER BY 1; base-commit: 2e61c50785a0dca6ed30a1a5522457b18bbb5498 -- 2.18.0
0002-Ignore-attempts-to-add-TOAST-table-to-shared-or-cata.patch (text/plain, 2.0K) download From 029fa7e97c8219f13b12ebd5026beb5ffd5ba684 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter_e@gmx.net> Date: Fri, 18 May 2018 17:05:27 -0400 Subject: [PATCH 2/5] Ignore attempts to add TOAST table to shared or catalog tables Running ALTER TABLE on any table will check if a TOAST table needs to be added. On shared tables, this would previously fail, thus effectively disabling ALTER TABLE for those tables. On (non-shared) system catalogs, on the other hand, it would add a TOAST table, even though we don't really want TOAST tables on some system catalogs. In some cases, it would also fail with an error "AccessExclusiveLock required to add toast table.", depending on what locks the ALTER TABLE actions had already taken. So instead, just ignore attempts to add TOAST tables to such tables, outside of bootstrap mode, pretending they don't need one. This allows running ALTER TABLE on such tables without messing up the TOAST situation. (All this still requires allow_system_table_mods, which is independent of this.) --- src/backend/catalog/toasting.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 3baaa08238..97a215d2c1 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -17,6 +17,7 @@ #include "access/tuptoaster.h" #include "access/xact.h" #include "catalog/binary_upgrade.h" +#include "catalog/catalog.h" #include "catalog/dependency.h" #include "catalog/heap.h" #include "catalog/index.h" @@ -153,9 +154,10 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, */ shared_relation = rel->rd_rel->relisshared; if (shared_relation && !IsBootstrapProcessingMode()) - ereport(ERROR, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("shared tables cannot be toasted after initdb"))); + return false; + + if (IsCatalogRelation(rel) && !IsBootstrapProcessingMode()) + return false; /* It's mapped if and only if its parent is, too */ mapped_relation = RelationIsMapped(rel); -- 2.18.0

view thread (25+ messages)  latest in thread

Message-ID: <e49f825b-fb25-0bc8-8afc-d5ad895c7975@2ndquadrant.com>
Permalink:  ../e49f825b-fb25-0bc8-8afc-d5ad895c7975@2ndquadrant.com/
Also on:    postgresql.org/message-id/e49f825b-fb25-0bc8-8afc-d5ad895c7975@2ndquadrant.com

 ·  · 

reply

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-hackers@postgresql.org
  Cc: peter.eisentraut@2ndquadrant.com
  Subject: Re: ALTER TABLE on system catalogs
  In-Reply-To: <e49f825b-fb25-0bc8-8afc-d5ad895c7975@2ndquadrant.com>

* 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