Fix toast table creation.

Instead of using slightly-too-clever heuristics to decide when we must
create a TOAST table, just check whether one is needed every time the
table is altered.  Checking whether a toast table is needed is cheap
enough that we needn't worry about doing it on every ALTER TABLE command,
and the previous coding is apparently prone to accidental breakage:
commit 04e17bae50a73af524731fa11210d5c3f7d8e1f9 broken ALTER TABLE ..
SET STORAGE, which moved some actions from AT_PASS_COL_ATTRS to
AT_PASS_MISC, and commit 6c5723998594dffa5d47c3cf8c96ccf89c033aae broke
ALTER TABLE .. ADD COLUMN by changing the way that adding columns
recurses into child tables.

Noah Misch, with one comment change by me

diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 472237c4a0f00c815dc8d534a34d6f38954c4911..85fe57fb2a5308d5079ce49bb602de1a12b23cf2 100644 (file)
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -59,11 +59,11 @@ AlterTableCreateToastTable(Oid relOid, Datum reloptions)
    Relation    rel;
 
    /*
-    * Grab an exclusive lock on the target table, which we will NOT release
-    * until end of transaction.  (This is probably redundant in all present
-    * uses...)
+    * Grab a DDL-exclusive lock on the target table, since we'll update the
+    * pg_class tuple.  This is redundant for all present users.  Tuple toasting
+    * behaves safely in the face of a concurrent TOAST table add.
     */
-   rel = heap_open(relOid, AccessExclusiveLock);
+   rel = heap_open(relOid, ShareUpdateExclusiveLock);
 
    /* create_toast_table does all the work */
    (void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions);
@@ -103,7 +103,7 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
 /*
  * create_toast_table --- internal workhorse
  *
- * rel is already opened and exclusive-locked
+ * rel is already opened and locked
  * toastOid and toastIndexOid are normally InvalidOid, but during
  * bootstrap they can be nonzero to specify hand-assigned OIDs
  */

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6afebc728f5f3ab08b7da26a8b4a475b994c1105..d3692754959770daa3b3db83a85626596ea4a417 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3022,18 +3022,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode)
        }
    }
 
-   /*
-    * Check to see if a toast table must be added, if we executed any
-    * subcommands that might have added a column or changed column storage.
-    */
+   /* Check to see if a toast table must be added. */
    foreach(ltab, *wqueue)
    {
        AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
 
-       if (tab->relkind == RELKIND_RELATION &&
-           (tab->subcmds[AT_PASS_ADD_COL] ||
-            tab->subcmds[AT_PASS_ALTER_TYPE] ||
-            tab->subcmds[AT_PASS_COL_ATTRS]))
+       if (tab->relkind == RELKIND_RELATION)
            AlterTableCreateToastTable(tab->relid, (Datum) 0);
    }
 }

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index d7d1b646cfa37f747926260f02d9d281861eb3a1..315b915c13ce774b8f7c48e5e0a5749a84be27d0 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1522,6 +1522,19 @@ ERROR:  composite type recur1 cannot be made a member of itself
 alter table recur1 add column f2 int;
 alter table recur1 alter column f2 type recur2; -- fails
 ERROR:  composite type recur1 cannot be made a member of itself
+-- SET STORAGE may need to add a TOAST table
+create table test_storage (a text);
+alter table test_storage alter a set storage plain;
+alter table test_storage add b int default 0; -- rewrite table to remove its TOAST table
+alter table test_storage alter a set storage extended; -- re-add TOAST table
+select reltoastrelid <> 0 as has_toast_table
+from pg_class
+where oid = 'test_storage'::regclass;
+ has_toast_table 
+-----------------
+ t
+(1 row)
+
 --
 -- lock levels
 --

diff --git a/src/test/regress/input/misc.source b/src/test/regress/input/misc.source
index 0930a6a4bb1fc6a49765ecb53bb691878beaf314..7cd26cb192dc60699a1f2a0c9014d1c66eb1584a 100644 (file)
--- a/src/test/regress/input/misc.source
+++ b/src/test/regress/input/misc.source
@@ -153,6 +153,12 @@ SELECT * FROM e_star*;
 
 ALTER TABLE a_star* ADD COLUMN a text;
 
+-- That ALTER TABLE should have added TOAST tables.
+SELECT relname, reltoastrelid <> 0 AS has_toast_table
+   FROM pg_class
+   WHERE oid::regclass IN ('a_star', 'c_star')
+   ORDER BY 1;
+
 --UPDATE b_star*
 --   SET a = text 'gazpacho'
 --   WHERE aa > 4;

diff --git a/src/test/regress/output/misc.source b/src/test/regress/output/misc.source
index c225d0f37f0d66698e1e3c2e91aa02b493e34194..34bde31b9f18bf0e696771ccf14879402a0bde73 100644 (file)
--- a/src/test/regress/output/misc.source
+++ b/src/test/regress/output/misc.source
@@ -376,6 +376,17 @@ SELECT * FROM e_star*;
 
 ALTER TABLE a_star* ADD COLUMN a text;
 NOTICE:  merging definition of column "a" for child "d_star"
+-- That ALTER TABLE should have added TOAST tables.
+SELECT relname, reltoastrelid <> 0 AS has_toast_table
+   FROM pg_class
+   WHERE oid::regclass IN ('a_star', 'c_star')
+   ORDER BY 1;
+ relname | has_toast_table 
+---------+-----------------
+ a_star  | t
+ c_star  | t
+(2 rows)
+
 --UPDATE b_star*
 --   SET a = text 'gazpacho'
 --   WHERE aa > 4;

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 749584d9e636c9a7bd89936f40c4f227537499ff..43a9ce971f6e31f76a1f389c2f1173ebd418a53d 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1133,6 +1133,16 @@ alter table recur1 add column f2 recur2; -- fails
 alter table recur1 add column f2 int;
 alter table recur1 alter column f2 type recur2; -- fails
 
+-- SET STORAGE may need to add a TOAST table
+create table test_storage (a text);
+alter table test_storage alter a set storage plain;
+alter table test_storage add b int default 0; -- rewrite table to remove its TOAST table
+alter table test_storage alter a set storage extended; -- re-add TOAST table
+
+select reltoastrelid <> 0 as has_toast_table
+from pg_class
+where oid = 'test_storage'::regclass;
+
 --
 -- lock levels
 --