There is logic in pg_upgrade plus the backend, mostly added by commit
4c6780fd1, to cope with the corner cases that sometimes arise where the
old and new versions have different ideas about whether a given table
needs a TOAST table. The more common case is where there's a TOAST table
in the old DB, but (perhaps as a result of having dropped all the wide
columns) the new cluster doesn't think the table definition requires a
TOAST table. The reverse is also possible, although according to the
existing code comments it can only happen when upgrading from pre-9.1.
The way pg_upgrade handles that case is that after running all the
table creation operations it issues this command:
PQclear(executeQueryOrDie(conn, "ALTER TABLE %s.%s RESET
(binary_upgrade_dummy_option);",
quote_identifier(PQgetvalue(res, rowno, i_nspname)),
quote_identifier(PQgetvalue(res, rowno, i_relname))));
which doesn't actually do anything (no such reloption being set) but
nonetheless triggers a call of AlterTableCreateToastTable, which
will cause a toast table to be created if the new server thinks the
table definition requires one.
Or at least, it did until Simon decided that ALTER TABLE RESET
doesn't require AccessExclusiveLock. Now you get a failure.
I haven't tried to construct a pre-9.1 database that would trigger
this, but you can make it happen by applying the attached patch
to create a toast-table-less table in the regression tests,
and then doing "make check" in src/bin/pg_upgrade. You get this:
...
Restoring database schemas in the new cluster
ok
Creating newly-required TOAST tables SQL command failed
ALTER TABLE "public"."i_once_had_a_toast_table" RESET
(binary_upgrade_dummy_option);
ERROR: AccessExclusiveLock required to add toast table.
Failure, exiting
+ rm -rf /tmp/pg_upgrade_check-o0CUMm
make: *** [check] Error 1
I think possibly the easiest fix for this is to have pg_upgrade,
instead of RESETting a nonexistent option, RESET something that's
still considered to require AccessExclusiveLock. "user_catalog_table"
would work, looks like; though I'd want to annotate its entry in
reloptions.c to warn people away from downgrading its lock level.
More generally, though, I wonder how we can have some test coverage
on such cases going forward. Is the patch below too ugly to commit
permanently, and if so, what other idea can you suggest?
regards, tom lane
diff --git a/src/test/regress/expected/indirect_toast.out b/src/test/regress/expected/indirect_toast.out
index 4f4bf41..ad7127d 100644
*** a/src/test/regress/expected/indirect_toast.out
--- b/src/test/regress/expected/indirect_toast.out
*************** SELECT substring(toasttest::text, 1, 200
*** 149,151 ****
--- 149,158 ----
DROP TABLE toasttest;
DROP FUNCTION update_using_indirect();
+ -- Create a table that has a toast table, then modify it so it appears
+ -- not to have one, and leave it behind after the regression tests end.
+ -- This enables testing of this scenario for pg_upgrade.
+ create table i_once_had_a_toast_table(f1 int, f2 text);
+ insert into i_once_had_a_toast_table values(1, 'foo');
+ update pg_class set reltoastrelid = 0
+ where relname = 'i_once_had_a_toast_table';
diff --git a/src/test/regress/sql/indirect_toast.sql b/src/test/regress/sql/indirect_toast.sql
index d502480..cefbd0b 100644
*** a/src/test/regress/sql/indirect_toast.sql
--- b/src/test/regress/sql/indirect_toast.sql
*************** SELECT substring(toasttest::text, 1, 200
*** 59,61 ****
--- 59,69 ----
DROP TABLE toasttest;
DROP FUNCTION update_using_indirect();
+
+ -- Create a table that has a toast table, then modify it so it appears
+ -- not to have one, and leave it behind after the regression tests end.
+ -- This enables testing of this scenario for pg_upgrade.
+ create table i_once_had_a_toast_table(f1 int, f2 text);
+ insert into i_once_had_a_toast_table values(1, 'foo');
+ update pg_class set reltoastrelid = 0
+ where relname = 'i_once_had_a_toast_table';
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
