Skip to content

Commit 39a68e5

Browse files
robertmhaasrobertmhaas
committed
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 04e17ba broken ALTER TABLE .. SET STORAGE, which moved some actions from AT_PASS_COL_ATTRS to AT_PASS_MISC, and commit 6c57239 broke ALTER TABLE .. ADD COLUMN by changing the way that adding columns recurses into child tables. Noah Misch, with one comment change by me
1 parent eca75a1 commit 39a68e5

File tree

6 files changed

+47
-13
lines changed

6 files changed

+47
-13
lines changed

‎src/backend/catalog/toasting.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ AlterTableCreateToastTable(Oid relOid, Datum reloptions)
5959
Relation rel;
6060

6161
/*
62-
* Grab an exclusive lock on the target table, which we will NOT release
63-
* until end of transaction. (This is probably redundant in all present
64-
* uses...)
62+
* Grab a DDL-exclusive lock on the target table, since we'll update the
63+
* pg_class tuple. This is redundant for all present users. Tuple toasting
64+
* behaves safely in the face of a concurrent TOAST table add.
6565
*/
66-
rel = heap_open(relOid, AccessExclusiveLock);
66+
rel = heap_open(relOid, ShareUpdateExclusiveLock);
6767

6868
/* create_toast_table does all the work */
6969
(void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions);
@@ -103,7 +103,7 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
103103
/*
104104
* create_toast_table --- internal workhorse
105105
*
106-
* rel is already opened and exclusive-locked
106+
* rel is already opened and locked
107107
* toastOid and toastIndexOid are normally InvalidOid, but during
108108
* bootstrap they can be nonzero to specify hand-assigned OIDs
109109
*/

‎src/backend/commands/tablecmds.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3022,18 +3022,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode)
30223022
}
30233023
}
30243024

3025-
/*
3026-
* Check to see if a toast table must be added, if we executed any
3027-
* subcommands that might have added a column or changed column storage.
3028-
*/
3025+
/* Check to see if a toast table must be added. */
30293026
foreach(ltab, *wqueue)
30303027
{
30313028
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
30323029

3033-
if (tab->relkind == RELKIND_RELATION &&
3034-
(tab->subcmds[AT_PASS_ADD_COL] ||
3035-
tab->subcmds[AT_PASS_ALTER_TYPE] ||
3036-
tab->subcmds[AT_PASS_COL_ATTRS]))
3030+
if (tab->relkind == RELKIND_RELATION)
30373031
AlterTableCreateToastTable(tab->relid, (Datum) 0);
30383032
}
30393033
}

‎src/test/regress/expected/alter_table.out

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,6 +1522,19 @@ ERROR: composite type recur1 cannot be made a member of itself
15221522
alter table recur1 add column f2 int;
15231523
alter table recur1 alter column f2 type recur2; -- fails
15241524
ERROR: composite type recur1 cannot be made a member of itself
1525+
-- SET STORAGE may need to add a TOAST table
1526+
create table test_storage (a text);
1527+
alter table test_storage alter a set storage plain;
1528+
alter table test_storage add b int default 0; -- rewrite table to remove its TOAST table
1529+
alter table test_storage alter a set storage extended; -- re-add TOAST table
1530+
select reltoastrelid <> 0 as has_toast_table
1531+
from pg_class
1532+
where oid = 'test_storage'::regclass;
1533+
has_toast_table
1534+
-----------------
1535+
t
1536+
(1 row)
1537+
15251538
--
15261539
-- lock levels
15271540
--

‎src/test/regress/input/misc.source

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,12 @@ SELECT * FROM e_star*;
153153

154154
ALTER TABLE a_star* ADD COLUMN a text;
155155

156+
-- That ALTER TABLE should have added TOAST tables.
157+
SELECT relname, reltoastrelid <> 0 AS has_toast_table
158+
FROM pg_class
159+
WHERE oid::regclass IN ('a_star', 'c_star')
160+
ORDER BY 1;
161+
156162
--UPDATE b_star*
157163
-- SET a = text 'gazpacho'
158164
-- WHERE aa > 4;

‎src/test/regress/output/misc.source

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,17 @@ SELECT * FROM e_star*;
376376

377377
ALTER TABLE a_star* ADD COLUMN a text;
378378
NOTICE: merging definition of column "a" for child "d_star"
379+
-- That ALTER TABLE should have added TOAST tables.
380+
SELECT relname, reltoastrelid <> 0 AS has_toast_table
381+
FROM pg_class
382+
WHERE oid::regclass IN ('a_star', 'c_star')
383+
ORDER BY 1;
384+
relname | has_toast_table
385+
---------+-----------------
386+
a_star | t
387+
c_star | t
388+
(2 rows)
389+
379390
--UPDATE b_star*
380391
-- SET a = text 'gazpacho'
381392
-- WHERE aa > 4;

‎src/test/regress/sql/alter_table.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,16 @@ alter table recur1 add column f2 recur2; -- fails
11331133
alter table recur1 add column f2 int;
11341134
alter table recur1 alter column f2 type recur2; -- fails
11351135

1136+
-- SET STORAGE may need to add a TOAST table
1137+
create table test_storage (a text);
1138+
alter table test_storage alter a set storage plain;
1139+
alter table test_storage add b int default 0; -- rewrite table to remove its TOAST table
1140+
alter table test_storage alter a set storage extended; -- re-add TOAST table
1141+
1142+
select reltoastrelid <> 0 as has_toast_table
1143+
from pg_class
1144+
where oid = 'test_storage'::regclass;
1145+
11361146
--
11371147
-- lock levels
11381148
--

0 commit comments

Comments
 (0)