Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit e1d88f9

Browse files
committedMay 7, 2016
Fix pg_upgrade to not fail when new-cluster TOAST rules differ from old.
This patch essentially reverts commit 4c6780f, in favor of a much simpler solution for the case where the new cluster would choose to create a TOAST table but the old cluster doesn't have one: just don't create a TOAST table. The existing code failed in at least two different ways if the situation arose: (1) ALTER TABLE RESET didn't grab an exclusive lock, so that the lock sanity check in create_toast_table failed; (2) pg_upgrade did not provide a pg_type OID for the new toast table, so that the crosscheck in TypeCreate failed. While both these problems were introduced by later patches, they show that the hack being used to cause TOAST table creation is overwhelmingly fragile (and untested). I also note that before the TypeCreate crosscheck was added, the code would have resulted in assigning an indeterminate pg_type OID to the toast table, possibly causing a later OID conflict in that catalog; so that it didn't really work even when committed. If we simply don't create a TOAST table, there will only be a problem if the code tries to store a tuple that's wider than a page, and field compression isn't sufficient to get it under a page. Given that the TOAST creation threshold is intended to be about a quarter of a page, it's very hard to believe that cross-version differences in the do-we-need-a-toast- table heuristic could result in an observable problem. So let's just follow the old version's conclusion about whether a TOAST table is needed. (If we ever do change needs_toast_table() so much that this conclusion doesn't apply, we can devise a solution at that time, and hopefully do it in a less klugy way than 4c6780f did.) Back-patch to 9.3, like the previous patch. Discussion: <8110.1462291671@sss.pgh.pa.us>
1 parent 462456d commit e1d88f9

File tree

4 files changed

+22
-113
lines changed

4 files changed

+22
-113
lines changed
 

‎contrib/pg_upgrade/dump.c

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@
1212
#include "pg_upgrade.h"
1313

1414
#include <sys/types.h>
15-
#include "catalog/pg_authid.h"
16-
17-
/* pick a OID that will never be used for TOAST tables */
18-
#define OPTIONALLY_CREATE_TOAST_OID BOOTSTRAP_SUPERUSERID
1915

2016

2117
void
@@ -72,71 +68,3 @@ generate_old_dump(void)
7268
end_progress_output();
7369
check_ok();
7470
}
75-
76-
77-
/*
78-
* It is possible for there to be a mismatch in the need for TOAST tables
79-
* between the old and new servers, e.g. some pre-9.1 tables didn't need
80-
* TOAST tables but will need them in 9.1+. (There are also opposite cases,
81-
* but these are handled by setting binary_upgrade_next_toast_pg_class_oid.)
82-
*
83-
* We can't allow the TOAST table to be created by pg_dump with a
84-
* pg_dump-assigned oid because it might conflict with a later table that
85-
* uses that oid, causing a "file exists" error for pg_class conflicts, and
86-
* a "duplicate oid" error for pg_type conflicts. (TOAST tables need pg_type
87-
* entries.)
88-
*
89-
* Therefore, a backend in binary-upgrade mode will not create a TOAST
90-
* table unless an OID as passed in via pg_upgrade_support functions.
91-
* This function is called after the restore and uses ALTER TABLE to
92-
* auto-create any needed TOAST tables which will not conflict with
93-
* restored oids.
94-
*/
95-
void
96-
optionally_create_toast_tables(void)
97-
{
98-
int dbnum;
99-
100-
prep_status("Creating newly-required TOAST tables");
101-
102-
for (dbnum = 0; dbnum < new_cluster.dbarr.ndbs; dbnum++)
103-
{
104-
PGresult *res;
105-
int ntups;
106-
int rowno;
107-
int i_nspname,
108-
i_relname;
109-
DbInfo *active_db = &new_cluster.dbarr.dbs[dbnum];
110-
PGconn *conn = connectToServer(&new_cluster, active_db->db_name);
111-
112-
res = executeQueryOrDie(conn,
113-
"SELECT n.nspname, c.relname "
114-
"FROM pg_catalog.pg_class c, "
115-
" pg_catalog.pg_namespace n "
116-
"WHERE c.relnamespace = n.oid AND "
117-
" n.nspname NOT IN ('pg_catalog', 'information_schema') AND "
118-
"c.relkind IN ('r', 'm') AND "
119-
"c.reltoastrelid = 0");
120-
121-
ntups = PQntuples(res);
122-
i_nspname = PQfnumber(res, "nspname");
123-
i_relname = PQfnumber(res, "relname");
124-
for (rowno = 0; rowno < ntups; rowno++)
125-
{
126-
/* enable auto-oid-numbered TOAST creation if needed */
127-
PQclear(executeQueryOrDie(conn, "SELECT binary_upgrade.set_next_toast_pg_class_oid('%d'::pg_catalog.oid);",
128-
OPTIONALLY_CREATE_TOAST_OID));
129-
130-
/* dummy command that also triggers check for required TOAST table */
131-
PQclear(executeQueryOrDie(conn, "ALTER TABLE %s.%s RESET (binary_upgrade_dummy_option);",
132-
quote_identifier(PQgetvalue(res, rowno, i_nspname)),
133-
quote_identifier(PQgetvalue(res, rowno, i_relname))));
134-
}
135-
136-
PQclear(res);
137-
138-
PQfinish(conn);
139-
}
140-
141-
check_ok();
142-
}

‎contrib/pg_upgrade/pg_upgrade.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -525,13 +525,11 @@ create_new_objects(void)
525525

526526
/*
527527
* We don't have minmxids for databases or relations in pre-9.3
528-
* clusters, so set those after we have restores the schemas.
528+
* clusters, so set those after we have restored the schema.
529529
*/
530530
if (GET_MAJOR_VERSION(old_cluster.major_version) < 903)
531531
set_frozenxids(true);
532532

533-
optionally_create_toast_tables();
534-
535533
/* regenerate now that we have objects in the databases */
536534
get_db_and_rel_infos(&new_cluster);
537535

‎contrib/pg_upgrade/pg_upgrade.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,6 @@ void disable_old_cluster(void);
333333
/* dump.c */
334334

335335
void generate_old_dump(void);
336-
void optionally_create_toast_tables(void);
337336

338337

339338
/* exec.c */

‎src/backend/catalog/toasting.c

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include "catalog/heap.h"
2121
#include "catalog/index.h"
2222
#include "catalog/namespace.h"
23-
#include "catalog/pg_authid.h"
2423
#include "catalog/pg_namespace.h"
2524
#include "catalog/pg_opclass.h"
2625
#include "catalog/pg_type.h"
@@ -40,9 +39,6 @@ static bool create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
4039
Datum reloptions);
4140
static bool needs_toast_table(Relation rel);
4241

43-
/* pick a OID that will never be used for TOAST tables */
44-
#define OPTIONALLY_CREATE_TOAST_OID BOOTSTRAP_SUPERUSERID
45-
4642

4743
/*
4844
* AlterTableCreateToastTable
@@ -154,50 +150,38 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptio
154150
if (rel->rd_rel->reltoastrelid != InvalidOid)
155151
return false;
156152

153+
/*
154+
* Check to see whether the table actually needs a TOAST table.
155+
*/
157156
if (!IsBinaryUpgrade)
158157
{
158+
/* Normal mode, normal check */
159159
if (!needs_toast_table(rel))
160160
return false;
161161
}
162162
else
163163
{
164164
/*
165-
* Check to see whether the table needs a TOAST table.
165+
* In binary-upgrade mode, create a TOAST table if and only if
166+
* pg_upgrade told us to (ie, a TOAST table OID has been provided).
166167
*
167-
* If an update-in-place TOAST relfilenode is specified, force TOAST file
168-
* creation even if it seems not to need one. This handles the case
169-
* where the old cluster needed a TOAST table but the new cluster
170-
* would not normally create one.
171-
*/
172-
173-
/*
174-
* If a TOAST oid is not specified, skip TOAST creation as we will do
175-
* it later so we don't create a TOAST table whose OID later conflicts
176-
* with a user-supplied OID. This handles cases where the old cluster
177-
* didn't need a TOAST table, but the new cluster does.
168+
* This indicates that the old cluster had a TOAST table for the
169+
* current table. We must create a TOAST table to receive the old
170+
* TOAST file, even if the table seems not to need one.
171+
*
172+
* Contrariwise, if the old cluster did not have a TOAST table, we
173+
* should be able to get along without one even if the new version's
174+
* needs_toast_table rules suggest we should have one. There is a lot
175+
* of daylight between where we will create a TOAST table and where
176+
* one is really necessary to avoid failures, so small cross-version
177+
* differences in the when-to-create heuristic shouldn't be a problem.
178+
* If we tried to create a TOAST table anyway, we would have the
179+
* problem that it might take up an OID that will conflict with some
180+
* old-cluster table we haven't seen yet.
178181
*/
179-
if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid))
182+
if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid) ||
183+
!OidIsValid(binary_upgrade_next_toast_pg_type_oid))
180184
return false;
181-
182-
/*
183-
* If a special TOAST value has been passed in, it means we are in
184-
* cleanup mode --- we are creating needed TOAST tables after all user
185-
* tables with specified OIDs have been created. We let the system
186-
* assign a TOAST oid for us. The tables are empty so the missing
187-
* TOAST tables were not a problem.
188-
*/
189-
if (binary_upgrade_next_toast_pg_class_oid == OPTIONALLY_CREATE_TOAST_OID)
190-
{
191-
/* clear as it is not to be used; it is just a flag */
192-
binary_upgrade_next_toast_pg_class_oid = InvalidOid;
193-
194-
if (!needs_toast_table(rel))
195-
return false;
196-
}
197-
198-
/* both should be set, or not set */
199-
Assert(OidIsValid(binary_upgrade_next_toast_pg_class_oid) ==
200-
OidIsValid(binary_upgrade_next_toast_pg_type_oid));
201185
}
202186

203187
/*

0 commit comments

Comments
 (0)
Failed to load comments.