On 20/08/2018 15:34, Tom Lane wrote:
> Andres Freund <and...@anarazel.de> writes:
>> On 2018-08-20 14:38:25 +0200, Peter Eisentraut wrote:
>>> Do you have an alternative in mind?
>
>> One is to just not do anything. I'm not sure I'm on board with the goal
>> of changing things to make DDL on system tables more palatable.
>
> FWIW, I agree with doing as little as possible here. I'd be on board
> with Andres' suggestion of just swapping the two code stanzas so that
> the no-op case doesn't error out. As soon as you go beyond that, you
> are in wildly unsafe and untested territory.
That doesn't solve the original problem, which is being able to set
reloptions on pg_attribute, because pg_attribute doesn't have a toast
table but would need one according to existing rules.
Attached is a patch that instead moves those special cases into
needs_toast_table(), which was one of the options suggested by Andres.
There were already similar checks there, so I guess this makes a bit of
sense.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From c83d2f64195d93991c129812d91dfc6118bae44b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 18 May 2018 17:05:27 -0400
Subject: [PATCH v2 1/2] 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 | 42 +++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 3baaa08238..c94d7ef9a5 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"
@@ -145,21 +146,6 @@ create_toast_table(Relation rel, Oid toastOid, Oid
toastIndexOid,
ObjectAddress baseobject,
toastobject;
- /*
- * Toast table is shared if and only if its parent is.
- *
- * We cannot allow toasting a shared relation after initdb (because
- * there's no way to mark it toasted in other databases' pg_class).
- */
- 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")));
-
- /* It's mapped if and only if its parent is, too */
- mapped_relation = RelationIsMapped(rel);
-
/*
* Is it already toasted?
*/
@@ -259,6 +245,12 @@ create_toast_table(Relation rel, Oid toastOid, Oid
toastIndexOid,
binary_upgrade_next_toast_pg_type_oid = InvalidOid;
}
+ /* Toast table is shared if and only if its parent is. */
+ shared_relation = rel->rd_rel->relisshared;
+
+ /* It's mapped if and only if its parent is, too */
+ mapped_relation = RelationIsMapped(rel);
+
toast_relid = heap_create_with_catalog(toast_relname,
namespaceid,
rel->rd_rel->reltablespace,
@@ -398,7 +390,6 @@ create_toast_table(Relation rel, Oid toastOid, Oid
toastIndexOid,
* (1) there are any toastable attributes, and (2) the maximum length
* of a tuple could exceed TOAST_TUPLE_THRESHOLD. (We don't want to
* create a toast table for something like "f1 varchar(20)".)
- * No need to create a TOAST table for partitioned tables.
*/
static bool
needs_toast_table(Relation rel)
@@ -410,9 +401,28 @@ needs_toast_table(Relation rel)
int32 tuple_length;
int i;
+ /*
+ * No need to create a TOAST table for partitioned tables.
+ */
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
return false;
+ /*
+ * We cannot allow toasting a shared relation after initdb (because
+ * there's no way to mark it toasted in other databases' pg_class).
+ */
+ if (rel->rd_rel->relisshared && !IsBootstrapProcessingMode())
+ return false;
+
+ /*
+ * Ignore attempts to create toast tables on catalog tables after
initdb.
+ * Which catalogs get toast tables is explicitly chosen in
+ * catalog/toasting.h. (We could get here via some ALTER TABLE command
if
+ * the catalog doesn't have a toast table.)
+ */
+ if (IsCatalogRelation(rel) && !IsBootstrapProcessingMode())
+ return false;
+
tupdesc = rel->rd_att;
for (i = 0; i < tupdesc->natts; i++)
--
2.18.0
