Re: [HACKERS] Pg_upgrade and toast tables bug discovered
The 9.3.5 release notes contain... - Fix pg_upgrade for cases where the new server creates a TOAST table but the old version did not (Bruce Momjian) This rare situation would manifest as relation OID mismatch errors. ...which I thought was this bug, hence my confusion. If anyone else is experiencing this bug, they may erroneously be led to believe that 9.3.5 contains the fix. I will attempt to build 9.3 stable head and retry my upgrade. On Wed, Sep 3, 2014 at 6:03 PM, Bruce Momjian br...@momjian.us wrote: On Wed, Sep 3, 2014 at 05:12:30PM -0600, Noah Yetter wrote: I'm not sure it's fixed. I am attempting a pg_upgrade from 9.2.8 to 9.3.5 and it dies like so: (...many relations restoring successfully snipped...) pg_restore: creating SEQUENCE address_address_id_seq pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 1410; 1259 17670 SEQUENCE address_address_id_seq javaprod pg_restore: [archiver (db)] could not execute query: ERROR: could not create file base/16414/17670: File exists Inspecting a copy of the source cluster, OID 17670 does indeed correspond to address_address_id_seq, but inspecting the partially-upgraded cluster that OID is taken by pg_toast_202359_index. Again conferring with a copy of the source (9.2.8) cluster, the relation corresponding to filenode 202359 does not have a toast table. (I know pg-hackers isn't the right place to discuss admin issues, but this thread is the only evidence of this bug I can find. If anyone can suggest a workaround I would be infinitely grateful.) Actually, there was a pg_upgrade fix _after_ the release of 9.3.5 which explains this failure: commit 4c6780fd17aa43ed6362aa682499cc2f9712cc8b Author: Bruce Momjian br...@momjian.us Date: Thu Aug 7 14:56:13 2014 -0400 pg_upgrade: prevent oid conflicts with new-cluster TOAST tables Previously, TOAST tables only required in the new cluster could cause oid conflicts if they were auto-numbered and a later conflicting oid had to be assigned. Backpatch through 9.3 Any chance you can download the 9.3.X source tree and try that? You need an entire install, not just a new pg_upgrade binary. I am disapointed I could not fix this before 9.3.5 was released. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Sep 4, 2014 at 11:37:27AM -0600, Noah Yetter wrote: The 9.3.5 release notes contain... • Fix pg_upgrade for cases where the new server creates a TOAST table but the old version did not (Bruce Momjian) This rare situation would manifest as relation OID mismatch errors. ...which I thought was this bug, hence my confusion. If anyone else is experiencing this bug, they may erroneously be led to believe that 9.3.5 contains the fix. I will attempt to build 9.3 stable head and retry my upgrade. Yes, please let us know. The post-9.3.5 fix is for the reverse case, where the new cluster needs a TOAST table that the old cluster didn't. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Sep 4, 2014 at 2:39 PM, Bruce Momjian [via PostgreSQL] ml-node+s1045698n5817828...@n5.nabble.com wrote: On Thu, Sep 4, 2014 at 11:37:27AM -0600, Noah Yetter wrote: The 9.3.5 release notes contain... • Fix pg_upgrade for cases where the new server creates a TOAST table but the old version did not (Bruce Momjian) This rare situation would manifest as relation OID mismatch errors. ...which I thought was this bug, hence my confusion. If anyone else is experiencing this bug, they may erroneously be led to believe that 9.3.5 contains the fix. I will attempt to build 9.3 stable head and retry my upgrade. Yes, please let us know. The post-9.3.5 fix is for the reverse case, where the new cluster needs a TOAST table that the old cluster didn't. hmmm...the 9.3.5 doc and what you just wrote (and the Aug 7 Patch Commit) are saying the same thing...both patches claim to fix oid conflicts when only the new server requires the TOAST table. I'm not sure, though, whether anything useful can be done except field questions until 9.3.6 is released. We cannot fix the 9.3.5 doc at this point and once 9.3.6 comes out the distinction will be irrelevant... David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Pg-upgrade-and-toast-tables-bug-discovered-tp5810447p5817830.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
Isn't that exactly what the release note says? where the new server creates a TOAST table but the old version did not vs. where the new cluster needs a TOAST table that the old cluster didn't At any rate, I've additionally observed that the relation which is blowing up pg_upgrade is a VIEW in the source cluster but gets created as a TABLE in the upgraded cluster, which may better explain why it had no toast table before and now it does. Is this some kind of expected behavior for views? On Thu, Sep 4, 2014 at 12:39 PM, Bruce Momjian br...@momjian.us wrote: On Thu, Sep 4, 2014 at 11:37:27AM -0600, Noah Yetter wrote: The 9.3.5 release notes contain... • Fix pg_upgrade for cases where the new server creates a TOAST table but the old version did not (Bruce Momjian) This rare situation would manifest as relation OID mismatch errors. ...which I thought was this bug, hence my confusion. If anyone else is experiencing this bug, they may erroneously be led to believe that 9.3.5 contains the fix. I will attempt to build 9.3 stable head and retry my upgrade. Yes, please let us know. The post-9.3.5 fix is for the reverse case, where the new cluster needs a TOAST table that the old cluster didn't. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Sep 4, 2014 at 01:14:01PM -0600, Noah Yetter wrote: Isn't that exactly what the release note says? where the new server creates a TOAST table but the old version did not vs. where the new cluster needs a TOAST table that the old cluster didn't Sorry, yes, I got confused. We have always handled cases where the old cluster needed a TOAST table and the new cluster didn't. The 9.3.5 fix is to prevent a certain failure for a new-only TOAST table: commit 3088cc37044a303fc50857d8d9e7e44b5c250642 Author: Bruce Momjian br...@momjian.us Date: Mon Jul 7 13:24:08 2014 -0400 pg_upgrade: allow upgrades for new-only TOAST tables Previously, when calculations on the need for toast tables changed, pg_upgrade could not handle cases where the new cluster needed a TOAST table and the old cluster did not. (It already handled the opposite case.) This fixes the OID mismatch error typically generated in this case. Backpatch through 9.2 The post-9.3.5 fix is for OID conflict that _can_ happen from a new-only TOAST tables: commit 4c6780fd17aa43ed6362aa682499cc2f9712cc8b Author: Bruce Momjian br...@momjian.us Date: Thu Aug 7 14:56:13 2014 -0400 pg_upgrade: prevent oid conflicts with new-cluster TOAST tables Previously, TOAST tables only required in the new cluster could cause oid conflicts if they were auto-numbered and a later conflicting oid had to be assigned. Backpatch through 9.3 At any rate, I've additionally observed that the relation which is blowing up pg_upgrade is a VIEW in the source cluster but gets created as a TABLE in the upgraded cluster, which may better explain why it had no toast table before and now it does. Is this some kind of expected behavior for views? Uh, it certainly should not be creating a table instead of a view, though it will get a pg_class entry. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Sep 4, 2014 at 3:35 PM, Bruce Momjian br...@momjian.us wrote: At any rate, I've additionally observed that the relation which is blowing up pg_upgrade is a VIEW in the source cluster but gets created as a TABLE in the upgraded cluster, which may better explain why it had no toast table before and now it does. Is this some kind of expected behavior for views? Uh, it certainly should not be creating a table instead of a view, though it will get a pg_class entry. Actually, there's a way this can happen. If you create two (or more) views with circular dependencies between them, then pg_dump will emit commands to create one of them as a table first, then create the others as views, then convert the first table to a view by adding a _SELECT rule to it. If pg_upgrade's logic can't cope with that, that's a bug in pg_upgrade, because there's no other way to restore views with circular dependency chains. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Sep 4, 2014 at 03:48:17PM -0400, Robert Haas wrote: On Thu, Sep 4, 2014 at 3:35 PM, Bruce Momjian br...@momjian.us wrote: At any rate, I've additionally observed that the relation which is blowing up pg_upgrade is a VIEW in the source cluster but gets created as a TABLE in the upgraded cluster, which may better explain why it had no toast table before and now it does. Is this some kind of expected behavior for views? Uh, it certainly should not be creating a table instead of a view, though it will get a pg_class entry. Actually, there's a way this can happen. If you create two (or more) views with circular dependencies between them, then pg_dump will emit commands to create one of them as a table first, then create the others as views, then convert the first table to a view by adding a _SELECT rule to it. Wow, that's super-interesting. If pg_upgrade's logic can't cope with that, that's a bug in pg_upgrade, because there's no other way to restore views with circular dependency chains. I don't see why pg_upgrade would have any problem with it as it just looks at the old schema and post-restore schema. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
Doing the upgrade with an installation built from REL9_3_STABLE at commit 52eed3d4267faf671dae0450d99982cb9ba1ac52 was successful. The view that I saw get re-created as a table doesn't have any circular references, or indeed any references to other views, nor do any other views reference it. But since it does seem that there are valid cases where a view gets temporarily re-created as a table during an upgrade, I'm going to assume it's not a bug per se. My upgraded cluster using built-from-source binaries has these views as views, so when the process is complete they end up in the correct state. Is there an expected release date for 9.3.6? On Thu, Sep 4, 2014 at 2:01 PM, Bruce Momjian br...@momjian.us wrote: On Thu, Sep 4, 2014 at 03:48:17PM -0400, Robert Haas wrote: On Thu, Sep 4, 2014 at 3:35 PM, Bruce Momjian br...@momjian.us wrote: At any rate, I've additionally observed that the relation which is blowing up pg_upgrade is a VIEW in the source cluster but gets created as a TABLE in the upgraded cluster, which may better explain why it had no toast table before and now it does. Is this some kind of expected behavior for views? Uh, it certainly should not be creating a table instead of a view, though it will get a pg_class entry. Actually, there's a way this can happen. If you create two (or more) views with circular dependencies between them, then pg_dump will emit commands to create one of them as a table first, then create the others as views, then convert the first table to a view by adding a _SELECT rule to it. Wow, that's super-interesting. If pg_upgrade's logic can't cope with that, that's a bug in pg_upgrade, because there's no other way to restore views with circular dependency chains. I don't see why pg_upgrade would have any problem with it as it just looks at the old schema and post-restore schema. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Sep 4, 2014 at 03:24:05PM -0600, Noah Yetter wrote: Doing the upgrade with an installation built from REL9_3_STABLE at commit 52eed3d4267faf671dae0450d99982cb9ba1ac52 was successful. The view that I saw get re-created as a table doesn't have any circular references, or indeed any references to other views, nor do any other views reference it. But since it does seem that there are valid cases where a view gets temporarily re-created as a table during an upgrade, I'm going to assume it's not a bug per se. My upgraded cluster using built-from-source binaries has these views as views, so when the process is complete they end up in the correct state. Is there an expected release date for 9.3.6? I don't know, but your report makes it clear we will need one sooner rather than later. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
I'm not sure it's fixed. I am attempting a pg_upgrade from 9.2.8 to 9.3.5 and it dies like so: (...many relations restoring successfully snipped...) pg_restore: creating SEQUENCE address_address_id_seq pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 1410; 1259 17670 SEQUENCE address_address_id_seq javaprod pg_restore: [archiver (db)] could not execute query: ERROR: could not create file base/16414/17670: File exists Inspecting a copy of the source cluster, OID 17670 does indeed correspond to address_address_id_seq, but inspecting the partially-upgraded cluster that OID is taken by pg_toast_202359_index. Again conferring with a copy of the source (9.2.8) cluster, the relation corresponding to filenode 202359 does not have a toast table. (I know pg-hackers isn't the right place to discuss admin issues, but this thread is the only evidence of this bug I can find. If anyone can suggest a workaround I would be infinitely grateful.) On Thu, Aug 7, 2014 at 12:57 PM, Bruce Momjian br...@momjian.us wrote: On Tue, Aug 5, 2014 at 07:31:21PM -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote: Well, we are going to need to call internal C functions, often bypassing their typical call sites and the assumption about locking, etc. Perhaps this could be done from a plpgsql function. We could add and drop a dummy column to force TOAST table creation --- we would then only need a way to detect if a function _needs_ a TOAST table, which was skipped in binary upgrade mode previously. That might be a minimalistic approach. I have thought some more on this. I thought I would need to open pg_class in C and do complex backend stuff, but I now realize I can do it from libpq, and just call ALTER TABLE and I think that always auto-checks if a TOAST table is needed. All I have to do is query pg_class from libpq, then construct ALTER TABLE commands for each item, and it will optionally create the TOAST table if needed. I just have to use a no-op ALTER TABLE command, like SET STATISTICS. Attached is a completed patch which handles oid conflicts in pg_class and pg_type for TOAST tables that were not needed in the old cluster but are needed in the new one. I was able to recreate a failure case and this fixed it. The patch need to be backpatched because I am getting more-frequent bug reports from users using pg_upgrade to leave now-end-of-life'ed 8.4. There is not a good work-around for pg_upgrade failures without this fix, but at least pg_upgrade throws an error. Patch applied through 9.3, with an additional Assert check. 9.2 code was different enough that there was too high a risk for backpatching. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
Based upon the dates the noted patch is not in 9.3.5; which was released a couple of weeks previous to it being committed. David J. nyetter wrote I'm not sure it's fixed. I am attempting a pg_upgrade from 9.2.8 to 9.3.5 and it dies like so: (...many relations restoring successfully snipped...) pg_restore: creating SEQUENCE address_address_id_seq pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 1410; 1259 17670 SEQUENCE address_address_id_seq javaprod pg_restore: [archiver (db)] could not execute query: ERROR: could not create file base/16414/17670: File exists Inspecting a copy of the source cluster, OID 17670 does indeed correspond to address_address_id_seq, but inspecting the partially-upgraded cluster that OID is taken by pg_toast_202359_index. Again conferring with a copy of the source (9.2.8) cluster, the relation corresponding to filenode 202359 does not have a toast table. (I know pg-hackers isn't the right place to discuss admin issues, but this thread is the only evidence of this bug I can find. If anyone can suggest a workaround I would be infinitely grateful.) On Thu, Aug 7, 2014 at 12:57 PM, Bruce Momjian lt; bruce@ gt; wrote: On Tue, Aug 5, 2014 at 07:31:21PM -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote: Well, we are going to need to call internal C functions, often bypassing their typical call sites and the assumption about locking, etc. Perhaps this could be done from a plpgsql function. We could add and drop a dummy column to force TOAST table creation --- we would then only need a way to detect if a function _needs_ a TOAST table, which was skipped in binary upgrade mode previously. That might be a minimalistic approach. I have thought some more on this. I thought I would need to open pg_class in C and do complex backend stuff, but I now realize I can do it from libpq, and just call ALTER TABLE and I think that always auto-checks if a TOAST table is needed. All I have to do is query pg_class from libpq, then construct ALTER TABLE commands for each item, and it will optionally create the TOAST table if needed. I just have to use a no-op ALTER TABLE command, like SET STATISTICS. Attached is a completed patch which handles oid conflicts in pg_class and pg_type for TOAST tables that were not needed in the old cluster but are needed in the new one. I was able to recreate a failure case and this fixed it. The patch need to be backpatched because I am getting more-frequent bug reports from users using pg_upgrade to leave now-end-of-life'ed 8.4. There is not a good work-around for pg_upgrade failures without this fix, but at least pg_upgrade throws an error. Patch applied through 9.3, with an additional Assert check. 9.2 code was different enough that there was too high a risk for backpatching. -- Bruce Momjian lt; bruce@ gt;http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list ( pgsql-hackers@ ) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- View this message in context: http://postgresql.1045698.n5.nabble.com/Pg-upgrade-and-toast-tables-bug-discovered-tp5810447p5817656.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Wed, Sep 3, 2014 at 05:12:30PM -0600, Noah Yetter wrote: I'm not sure it's fixed. I am attempting a pg_upgrade from 9.2.8 to 9.3.5 and it dies like so: (...many relations restoring successfully snipped...) pg_restore: creating SEQUENCE address_address_id_seq pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 1410; 1259 17670 SEQUENCE address_address_id_seq javaprod pg_restore: [archiver (db)] could not execute query: ERROR: could not create file base/16414/17670: File exists Inspecting a copy of the source cluster, OID 17670 does indeed correspond to address_address_id_seq, but inspecting the partially-upgraded cluster that OID is taken by pg_toast_202359_index. Again conferring with a copy of the source (9.2.8) cluster, the relation corresponding to filenode 202359 does not have a toast table. (I know pg-hackers isn't the right place to discuss admin issues, but this thread is the only evidence of this bug I can find. If anyone can suggest a workaround I would be infinitely grateful.) Actually, there was a pg_upgrade fix _after_ the release of 9.3.5 which explains this failure: commit 4c6780fd17aa43ed6362aa682499cc2f9712cc8b Author: Bruce Momjian br...@momjian.us Date: Thu Aug 7 14:56:13 2014 -0400 pg_upgrade: prevent oid conflicts with new-cluster TOAST tables Previously, TOAST tables only required in the new cluster could cause oid conflicts if they were auto-numbered and a later conflicting oid had to be assigned. Backpatch through 9.3 Any chance you can download the 9.3.X source tree and try that? You need an entire install, not just a new pg_upgrade binary. I am disapointed I could not fix this before 9.3.5 was released. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Tue, Aug 5, 2014 at 07:31:21PM -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote: Well, we are going to need to call internal C functions, often bypassing their typical call sites and the assumption about locking, etc. Perhaps this could be done from a plpgsql function. We could add and drop a dummy column to force TOAST table creation --- we would then only need a way to detect if a function _needs_ a TOAST table, which was skipped in binary upgrade mode previously. That might be a minimalistic approach. I have thought some more on this. I thought I would need to open pg_class in C and do complex backend stuff, but I now realize I can do it from libpq, and just call ALTER TABLE and I think that always auto-checks if a TOAST table is needed. All I have to do is query pg_class from libpq, then construct ALTER TABLE commands for each item, and it will optionally create the TOAST table if needed. I just have to use a no-op ALTER TABLE command, like SET STATISTICS. Attached is a completed patch which handles oid conflicts in pg_class and pg_type for TOAST tables that were not needed in the old cluster but are needed in the new one. I was able to recreate a failure case and this fixed it. The patch need to be backpatched because I am getting more-frequent bug reports from users using pg_upgrade to leave now-end-of-life'ed 8.4. There is not a good work-around for pg_upgrade failures without this fix, but at least pg_upgrade throws an error. Patch applied through 9.3, with an additional Assert check. 9.2 code was different enough that there was too high a risk for backpatching. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote:
On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote:
Well, we are going to need to call internal C functions, often bypassing
their typical call sites and the assumption about locking, etc. Perhaps
this could be done from a plpgsql function. We could add and drop a
dummy column to force TOAST table creation --- we would then only need a
way to detect if a function _needs_ a TOAST table, which was skipped in
binary upgrade mode previously.
That might be a minimalistic approach.
I have thought some more on this. I thought I would need to open
pg_class in C and do complex backend stuff, but I now realize I can do
it from libpq, and just call ALTER TABLE and I think that always
auto-checks if a TOAST table is needed. All I have to do is query
pg_class from libpq, then construct ALTER TABLE commands for each item,
and it will optionally create the TOAST table if needed. I just have to
use a no-op ALTER TABLE command, like SET STATISTICS.
Attached is a completed patch which handles oid conflicts in pg_class
and pg_type for TOAST tables that were not needed in the old cluster but
are needed in the new one. I was able to recreate a failure case and
this fixed it.
The patch need to be backpatched because I am getting more-frequent bug
reports from users using pg_upgrade to leave now-end-of-life'ed 8.4.
There is not a good work-around for pg_upgrade failures without this
fix, but at least pg_upgrade throws an error.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c
new file mode 100644
index b112f3a..642dd5d
*** a/contrib/pg_upgrade/dump.c
--- b/contrib/pg_upgrade/dump.c
***
*** 12,17
--- 12,19
#include pg_upgrade.h
#include sys/types.h
+ #include catalog/binary_upgrade.h
+
void
generate_old_dump(void)
*** generate_old_dump(void)
*** 67,69
--- 69,139
end_progress_output();
check_ok();
}
+
+
+ /*
+ * It is possible for there to be a mismatch in the need for TOAST tables
+ * between the old and new servers, e.g. some pre-9.1 tables didn't need
+ * TOAST tables but will need them in 9.1+. (There are also opposite cases,
+ * but these are handled by setting binary_upgrade_next_toast_pg_class_oid.)
+ *
+ * We can't allow the TOAST table to be created by pg_dump with a
+ * pg_dump-assigned oid because it might conflict with a later table that
+ * uses that oid, causing a file exists error for pg_class conflicts, and
+ * a duplicate oid error for pg_type conflicts. (TOAST tables need pg_type
+ * entries.)
+ *
+ * Therefore, a backend in binary-upgrade mode will not create a TOAST
+ * table unless an OID as passed in via pg_upgrade_support functions.
+ * This function is called after the restore and uses ALTER TABLE to
+ * auto-create any needed TOAST tables which will not conflict with
+ * restored oids.
+ */
+ void
+ optionally_create_toast_tables(void)
+ {
+ int dbnum;
+
+ prep_status(Creating newly-required TOAST tables);
+
+ for (dbnum = 0; dbnum new_cluster.dbarr.ndbs; dbnum++)
+ {
+ PGresult *res;
+ int ntups;
+ int rowno;
+ int i_nspname,
+ i_relname;
+ DbInfo *active_db = new_cluster.dbarr.dbs[dbnum];
+ PGconn *conn = connectToServer(new_cluster, active_db-db_name);
+
+ res = executeQueryOrDie(conn,
+ SELECT n.nspname, c.relname
+ FROM pg_catalog.pg_class c,
+ pg_catalog.pg_namespace n
+ WHERE c.relnamespace = n.oid AND
+ n.nspname NOT IN ('pg_catalog', 'information_schema') AND
+ c.relkind IN ('r', 'm') AND
+ c.reltoastrelid = 0);
+
+ ntups = PQntuples(res);
+ i_nspname = PQfnumber(res, nspname);
+ i_relname = PQfnumber(res, relname);
+ for (rowno = 0; rowno ntups; rowno++)
+ {
+ /* enable auto-oid-numbered TOAST creation if needed */
+ PQclear(executeQueryOrDie(conn, SELECT binary_upgrade.set_next_toast_pg_class_oid('%d'::pg_catalog.oid);,
+ OPTIONALLY_CREATE_TOAST_OID));
+
+ /* dummy command that also triggers check for required TOAST table */
+ 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;
+ }
+
+ PQclear(res);
+
+ PQfinish(conn);
+ }
+
+ check_ok();
+ }
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index 3e97b66..49ea873
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*** create_new_objects(void)
*** 363,368
--- 363,370
if (GET_MAJOR_VERSION(old_cluster.major_version) 903)
set_frozenxids(true);
+
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Mon, Jul 14, 2014 at 09:40:33PM +0200, Andres Freund wrote: On 2014-07-11 09:55:34 -0400, Bruce Momjian wrote: On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote: Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. This seems overengineered. Why not just do SELECT pg_upgrade.maintain_toast(oid) FROM pg_class WHERE relkind = 'r'; and in maintain_toast() just call AlterTableCreateToastTable()? I didn't think you could call into backend functions like that, and if I did, it might break something in the future. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On 2014-07-16 10:19:05 -0400, Bruce Momjian wrote: What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. This seems overengineered. Why not just do SELECT pg_upgrade.maintain_toast(oid) FROM pg_class WHERE relkind = 'r'; and in maintain_toast() just call AlterTableCreateToastTable()? I didn't think you could call into backend functions like that, and if I did, it might break something in the future. Why not? Pg_upgrade is already intimately linked into to the way the backend works. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Wed, Jul 16, 2014 at 10:19 AM, Bruce Momjian br...@momjian.us wrote: On Mon, Jul 14, 2014 at 09:40:33PM +0200, Andres Freund wrote: On 2014-07-11 09:55:34 -0400, Bruce Momjian wrote: On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote: Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. This seems overengineered. Why not just do SELECT pg_upgrade.maintain_toast(oid) FROM pg_class WHERE relkind = 'r'; and in maintain_toast() just call AlterTableCreateToastTable()? I didn't think you could call into backend functions like that, and if I did, it might break something in the future. It would be impossible to write meaningful code in extensions if that were true. Yeah, it could break in the future, but it's not particularly likely, the regression tests should catch it if it happens, and it's no worse a risk here than in any other extension code which does this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Fri, Jul 11, 2014 at 9:55 AM, Bruce Momjian br...@momjian.us wrote: On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote: Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. reloptions? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Mon, Jul 14, 2014 at 12:26 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Jul 11, 2014 at 9:55 AM, Bruce Momjian br...@momjian.us wrote: On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote: Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. reloptions? Is this another use case to custom reloptions idea? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Mon, Jul 14, 2014 at 11:26:19AM -0400, Robert Haas wrote: On Fri, Jul 11, 2014 at 9:55 AM, Bruce Momjian br...@momjian.us wrote: On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote: Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. reloptions? Yes, that might work. I thought about that but did not have time to see if we can easily add/remove reloptions at the C level in that area of the code. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On 2014-07-11 09:55:34 -0400, Bruce Momjian wrote: On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote: Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. This seems overengineered. Why not just do SELECT pg_upgrade.maintain_toast(oid) FROM pg_class WHERE relkind = 'r'; and in maintain_toast() just call AlterTableCreateToastTable()? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Fri, Jul 11, 2014 at 12:18:40AM -0400, Alvaro Herrera wrote: Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote: I have thought some more on this. I thought I would need to open pg_class in C and do complex backend stuff, but I now realize I can do it from libpq, and just call ALTER TABLE and I think that always auto-checks if a TOAST table is needed. All I have to do is query pg_class from libpq, then construct ALTER TABLE commands for each item, and it will optionally create the TOAST table if needed. I just have to use a no-op ALTER TABLE command, like SET STATISTICS. Attached is the backend part of the patch. I will work on the pg_upgrade/libpq/ALTER TABLE part later. Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Fri, Jul 11, 2014 at 09:48:06AM -0400, Bruce Momjian wrote: Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? Uh, I think you need to read the thread. We have to delay the toast creation part so we don't use an oid that will later be required by another table from the old cluster. This has to be done after all tables have been created. We could have pg_dump spit out those ALTER lines at the end of the dump, but it seems simpler to do it in pg_upgrade. Even if we have pg_dump create all the tables that require pre-assigned TOAST oids first, then the other tables that _might_ need a TOAST table, those later tables might create a toast oid that matches a later non-TOAST-requiring table, so I don't think that fixes the problem. What would be nice is if I could mark just the tables that will need toast tables created in that later phase (those tables that didn't have a toast table in the old cluster, but need one in the new cluster). However, I can't see where to store that or how to pass that back into pg_upgrade. I don't see a logical place in pg_class to put it. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Wed, Jul 9, 2014 at 12:09 PM, Bruce Momjian br...@momjian.us wrote: To me, that sounds vastly more complicated and error-prone than forcing the TOAST tables to be added in a second pass as Andres suggested. But I just work here. Agreed. I am now thinking we could harness the code that already exists to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN. We would just need an entry point to call it from pg_upgrade, either via an SQL command that checks (and hopefully doesn't do anything else), or a C function that does it, e.g. VACUUM would be trivial to run on every database, but I don't think it tests that; is _could_ in binary_upgrade mode. However, the idea of having a C function plug into the guts of the server and call internal functions makes me uncomforable. Well, pg_upgrade_support's charter is basically to provide access to the guts of the server in ways we wouldn't normally allow; all that next-OID stuff is basically exactly that. So I don't think this is such a big deal. It needs to be properly commented, of course. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Jul 10, 2014 at 10:46:30AM -0400, Robert Haas wrote: Agreed. I am now thinking we could harness the code that already exists to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN. We would just need an entry point to call it from pg_upgrade, either via an SQL command that checks (and hopefully doesn't do anything else), or a C function that does it, e.g. VACUUM would be trivial to run on every database, but I don't think it tests that; is _could_ in binary_upgrade mode. However, the idea of having a C function plug into the guts of the server and call internal functions makes me uncomforable. Well, pg_upgrade_support's charter is basically to provide access to the guts of the server in ways we wouldn't normally allow; all that next-OID stuff is basically exactly that. So I don't think this is such a big deal. It needs to be properly commented, of course. If you look at how oid assignment is handled, it is done in a very surgical way, i.e. pg_upgrade_support sets a global variable, and the variable triggers different behavior in a CREATE command. This change would be far more invasive than that. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On 2014-07-10 16:33:40 -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 10:46:30AM -0400, Robert Haas wrote: Agreed. I am now thinking we could harness the code that already exists to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN. We would just need an entry point to call it from pg_upgrade, either via an SQL command that checks (and hopefully doesn't do anything else), or a C function that does it, e.g. VACUUM would be trivial to run on every database, but I don't think it tests that; is _could_ in binary_upgrade mode. However, the idea of having a C function plug into the guts of the server and call internal functions makes me uncomforable. Well, pg_upgrade_support's charter is basically to provide access to the guts of the server in ways we wouldn't normally allow; all that next-OID stuff is basically exactly that. So I don't think this is such a big deal. It needs to be properly commented, of course. If you look at how oid assignment is handled, it is done in a very surgical way, i.e. pg_upgrade_support sets a global variable, and the variable triggers different behavior in a CREATE command. This change would be far more invasive than that. Meh. It's only somewhat surgical because there's pg_upgrade specific code sprinkled in the backend at strategic places. That's the contrary of a clear abstraction barrier. And arguably a function call from a SQL C function has a much clearer abstraction barrier. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Jul 10, 2014 at 11:11:19PM +0200, Andres Freund wrote: On 2014-07-10 16:33:40 -0400, Bruce Momjian wrote: On Thu, Jul 10, 2014 at 10:46:30AM -0400, Robert Haas wrote: Agreed. I am now thinking we could harness the code that already exists to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN. We would just need an entry point to call it from pg_upgrade, either via an SQL command that checks (and hopefully doesn't do anything else), or a C function that does it, e.g. VACUUM would be trivial to run on every database, but I don't think it tests that; is _could_ in binary_upgrade mode. However, the idea of having a C function plug into the guts of the server and call internal functions makes me uncomforable. Well, pg_upgrade_support's charter is basically to provide access to the guts of the server in ways we wouldn't normally allow; all that next-OID stuff is basically exactly that. So I don't think this is such a big deal. It needs to be properly commented, of course. If you look at how oid assignment is handled, it is done in a very surgical way, i.e. pg_upgrade_support sets a global variable, and the variable triggers different behavior in a CREATE command. This change would be far more invasive than that. Meh. It's only somewhat surgical because there's pg_upgrade specific code sprinkled in the backend at strategic places. That's the contrary of a clear abstraction barrier. And arguably a function call from a SQL C function has a much clearer abstraction barrier. Well, we are going to need to call internal C functions, often bypassing their typical call sites and the assumption about locking, etc. Perhaps this could be done from a plpgsql function. We could add and drop a dummy column to force TOAST table creation --- we would then only need a way to detect if a function _needs_ a TOAST table, which was skipped in binary upgrade mode previously. That might be a minimalistic approach. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote: Well, we are going to need to call internal C functions, often bypassing their typical call sites and the assumption about locking, etc. Perhaps this could be done from a plpgsql function. We could add and drop a dummy column to force TOAST table creation --- we would then only need a way to detect if a function _needs_ a TOAST table, which was skipped in binary upgrade mode previously. That might be a minimalistic approach. I have thought some more on this. I thought I would need to open pg_class in C and do complex backend stuff, but I now realize I can do it from libpq, and just call ALTER TABLE and I think that always auto-checks if a TOAST table is needed. All I have to do is query pg_class from libpq, then construct ALTER TABLE commands for each item, and it will optionally create the TOAST table if needed. I just have to use a no-op ALTER TABLE command, like SET STATISTICS. I am in Asia the next two weeks but will work on it after I return. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote:
On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote:
Well, we are going to need to call internal C functions, often bypassing
their typical call sites and the assumption about locking, etc. Perhaps
this could be done from a plpgsql function. We could add and drop a
dummy column to force TOAST table creation --- we would then only need a
way to detect if a function _needs_ a TOAST table, which was skipped in
binary upgrade mode previously.
That might be a minimalistic approach.
I have thought some more on this. I thought I would need to open
pg_class in C and do complex backend stuff, but I now realize I can do
it from libpq, and just call ALTER TABLE and I think that always
auto-checks if a TOAST table is needed. All I have to do is query
pg_class from libpq, then construct ALTER TABLE commands for each item,
and it will optionally create the TOAST table if needed. I just have to
use a no-op ALTER TABLE command, like SET STATISTICS.
I am in Asia the next two weeks but will work on it after I return.
Attached is the backend part of the patch. I will work on the
pg_upgrade/libpq/ALTER TABLE part later.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
new file mode 100644
index bdfeb90..6e8ef36
*** a/src/backend/catalog/toasting.c
--- b/src/backend/catalog/toasting.c
*** create_toast_table(Relation rel, Oid toa
*** 165,180
if (rel-rd_rel-reltoastrelid != InvalidOid)
return false;
! /*
! * Check to see whether the table actually needs a TOAST table.
! *
! * If an update-in-place toast relfilenode is specified, force toast file
! * creation even if it seems not to need one.
! */
! if (!needs_toast_table(rel)
! (!IsBinaryUpgrade ||
! !OidIsValid(binary_upgrade_next_toast_pg_class_oid)))
! return false;
/*
* If requested check lockmode is sufficient. This is a cross check in
--- 165,211
if (rel-rd_rel-reltoastrelid != InvalidOid)
return false;
! if (!IsBinaryUpgrade)
! {
! if (!needs_toast_table(rel))
! return false;
! }
! else
! {
! /*
! * Check to see whether the table needs a TOAST table.
! *
! * If an update-in-place TOAST relfilenode is specified, force TOAST file
! * creation even if it seems not to need one. This handles the case
! * where the old cluster needed a TOAST table but the new cluster
! * would not normally create one.
! */
!
! /*
! * If a TOAST oid is not specified, skip TOAST creation as we will do
! * it later so we don't create a TOAST table whose OID later conflicts
! * with a user-supplied OID. This handles cases where the old cluster
! * didn't need a TOAST table, but the new cluster does.
! */
! if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid))
! return false;
!
! /*
! * If a special TOAST value has been passed in, it means we are in
! * cleanup mode --- we are creating needed TOAST tables after all user
! * tables with specified OIDs have been created. We let the system
! * assign a TOAST oid for us. The tables are empty so the missing
! * TOAST tables were not a problem.
! */
! if (binary_upgrade_next_toast_pg_class_oid == OPTIONALLY_CREATE_TOAST_OID)
! {
! /* clear it so it is no longer used */
! binary_upgrade_next_toast_pg_class_oid = InvalidOid;
!
! if (!needs_toast_table(rel))
! return false;
! }
! }
/*
* If requested check lockmode is sufficient. This is a cross check in
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
new file mode 100644
index f39017c..7aff509
*** a/src/include/catalog/binary_upgrade.h
--- b/src/include/catalog/binary_upgrade.h
***
*** 14,19
--- 14,22
#ifndef BINARY_UPGRADE_H
#define BINARY_UPGRADE_H
+ /* pick a OID that will never be normally used for TOAST table */
+ #define OPTIONALLY_CREATE_TOAST_OID 1
+
extern PGDLLIMPORT Oid binary_upgrade_next_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_array_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_type_oid;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
Bruce Momjian wrote: On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote: I have thought some more on this. I thought I would need to open pg_class in C and do complex backend stuff, but I now realize I can do it from libpq, and just call ALTER TABLE and I think that always auto-checks if a TOAST table is needed. All I have to do is query pg_class from libpq, then construct ALTER TABLE commands for each item, and it will optionally create the TOAST table if needed. I just have to use a no-op ALTER TABLE command, like SET STATISTICS. Attached is the backend part of the patch. I will work on the pg_upgrade/libpq/ALTER TABLE part later. Uh, why does this need to be in ALTER TABLE? Can't this be part of table creation done by pg_dump? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
Uh, I guess we could write some code that iterates over all tables and finds the tables that should have TOAST tables, but don't (because binary-upgrade backend mode suppressed their creation), and adds them. However, that would be a lot of code and might be risky to backpatch. The error is so rare I am not sure it is worth it. I tried to create the failure case and it was very difficult, requiring me to create the problem table first, then some dummy stuff to get the offsets right so the oid would collide. Based on the rareness of the bug, I am not sure it is worth it, but if it is, it would be something only for 9.4 (maybe) and 9.5, not something to backpatch. Another idea would be to renumber empty toast tables that conflict during new toast table creation, when in binary upgrade mode. Since the files are always empty in binary upgrade mode, you could just create a new toast table, repoint the old table to use (because it didn't ask for a specific toast table oid because it didn't need one), and then use that one for the table that actually requested the oid. However, if one is a heap (zero size), and one is an index (8k size), it wouldn't work and you would have to recreate the file system file too. That seems a lot more localized than the other approaches. To me, that sounds vastly more complicated and error-prone than forcing the TOAST tables to be added in a second pass as Andres suggested. But I just work here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Wed, Jul 9, 2014 at 10:13:17AM -0400, Robert Haas wrote: Uh, I guess we could write some code that iterates over all tables and finds the tables that should have TOAST tables, but don't (because binary-upgrade backend mode suppressed their creation), and adds them. However, that would be a lot of code and might be risky to backpatch. The error is so rare I am not sure it is worth it. I tried to create the failure case and it was very difficult, requiring me to create the problem table first, then some dummy stuff to get the offsets right so the oid would collide. Based on the rareness of the bug, I am not sure it is worth it, but if it is, it would be something only for 9.4 (maybe) and 9.5, not something to backpatch. Another idea would be to renumber empty toast tables that conflict during new toast table creation, when in binary upgrade mode. Since the files are always empty in binary upgrade mode, you could just create a new toast table, repoint the old table to use (because it didn't ask for a specific toast table oid because it didn't need one), and then use that one for the table that actually requested the oid. However, if one is a heap (zero size), and one is an index (8k size), it wouldn't work and you would have to recreate the file system file too. That seems a lot more localized than the other approaches. To me, that sounds vastly more complicated and error-prone than forcing the TOAST tables to be added in a second pass as Andres suggested. But I just work here. Agreed. I am now thinking we could harness the code that already exists to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN. We would just need an entry point to call it from pg_upgrade, either via an SQL command that checks (and hopefully doesn't do anything else), or a C function that does it, e.g. VACUUM would be trivial to run on every database, but I don't think it tests that; is _could_ in binary_upgrade mode. However, the idea of having a C function plug into the guts of the server and call internal functions makes me uncomforable. The code already called via pg_restore would only need to suppress TOAST table creation if a heap oid is supplied but a TOAST table oid is not. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Fri, Jul 4, 2014 at 11:12 PM, Bruce Momjian br...@momjian.us wrote: On Fri, Jul 4, 2014 at 12:01:37AM -0400, Bruce Momjian wrote: The most robust, but not trivial, approach seems to be to prevent toast table creation if there wasn't a set_next_toast_pg_class_oid(). Then, after all relations are created, iterate over all pg_class entries that possibly need toast tables and recheck if they now might need one. Wow, that is going to be kind of odd in that there really isn't a good way to create toast tables except perhaps add a dummy TEXT column and remove it. There also isn't an easy way to not create a toast table, but also find out that one was needed. I suppose we would have to insert some dummy value in the toast pg_class column and come back later to clean it up. I am wondering what the probability of having a table that didn't need a toast table in the old cluster, and needed one in the new cluster, and there being an oid collision. I think the big question is whether we want to go down that route. Here is an updated patch. It was a little tricky because if the mismatched non-toast table is after the last old relation, you need to test differently and emit a different error message as there is no old relation to complain about. As far as the reusing of oids, we don't set the oid counter until after the restore, so any new unmatched toast table would given a very low oid. Since we restore in oid order, for an oid to be assigned that was used in the old cluster, it would have to be a very early relation, so I think that reduces the odds considerably. I am not inclined to do anything more to avoid this until I see an actual error report --- trying to address it might be invasive and introduce new errors. That sounds pretty shaky from where I sit. I wonder if the pg_upgrade_support module should have a function create_toast_table_if_needed(regclass) or something like that. Or maybe just create_any_missing_toast_tables(), iterating over all relations. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Fri, Jul 4, 2014 at 11:12:58PM -0400, Bruce Momjian wrote: On Fri, Jul 4, 2014 at 12:01:37AM -0400, Bruce Momjian wrote: The most robust, but not trivial, approach seems to be to prevent toast table creation if there wasn't a set_next_toast_pg_class_oid(). Then, after all relations are created, iterate over all pg_class entries that possibly need toast tables and recheck if they now might need one. Wow, that is going to be kind of odd in that there really isn't a good way to create toast tables except perhaps add a dummy TEXT column and remove it. There also isn't an easy way to not create a toast table, but also find out that one was needed. I suppose we would have to insert some dummy value in the toast pg_class column and come back later to clean it up. I am wondering what the probability of having a table that didn't need a toast table in the old cluster, and needed one in the new cluster, and there being an oid collision. I think the big question is whether we want to go down that route. Here is an updated patch. It was a little tricky because if the mismatched non-toast table is after the last old relation, you need to test differently and emit a different error message as there is no old relation to complain about. As far as the reusing of oids, we don't set the oid counter until after the restore, so any new unmatched toast table would given a very low oid. Since we restore in oid order, for an oid to be assigned that was used in the old cluster, it would have to be a very early relation, so I think that reduces the odds considerably. I am not inclined to do anything more to avoid this until I see an actual error report --- trying to address it might be invasive and introduce new errors. Patch applied back through 9.2. 9.1 and earlier had code that was different enought that I thought it would cause too many problems, and I doubt many users are doing major uprades to 9.1, and the bug is rare. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Mon, Jul 7, 2014 at 11:24:51AM -0400, Robert Haas wrote: As far as the reusing of oids, we don't set the oid counter until after the restore, so any new unmatched toast table would given a very low oid. Since we restore in oid order, for an oid to be assigned that was used in the old cluster, it would have to be a very early relation, so I think that reduces the odds considerably. I am not inclined to do anything more to avoid this until I see an actual error report --- trying to address it might be invasive and introduce new errors. That sounds pretty shaky from where I sit. I wonder if the pg_upgrade_support module should have a function create_toast_table_if_needed(regclass) or something like that. Or maybe just create_any_missing_toast_tables(), iterating over all relations. Uh, I guess we could write some code that iterates over all tables and finds the tables that should have TOAST tables, but don't (because binary-upgrade backend mode suppressed their creation), and adds them. However, that would be a lot of code and might be risky to backpatch. The error is so rare I am not sure it is worth it. I tried to create the failure case and it was very difficult, requiring me to create the problem table first, then some dummy stuff to get the offsets right so the oid would collide. Based on the rareness of the bug, I am not sure it is worth it, but if it is, it would be something only for 9.4 (maybe) and 9.5, not something to backpatch. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Mon, Jul 7, 2014 at 01:44:59PM -0400, Bruce Momjian wrote: On Mon, Jul 7, 2014 at 11:24:51AM -0400, Robert Haas wrote: As far as the reusing of oids, we don't set the oid counter until after the restore, so any new unmatched toast table would given a very low oid. Since we restore in oid order, for an oid to be assigned that was used in the old cluster, it would have to be a very early relation, so I think that reduces the odds considerably. I am not inclined to do anything more to avoid this until I see an actual error report --- trying to address it might be invasive and introduce new errors. That sounds pretty shaky from where I sit. I wonder if the pg_upgrade_support module should have a function create_toast_table_if_needed(regclass) or something like that. Or maybe just create_any_missing_toast_tables(), iterating over all relations. Uh, I guess we could write some code that iterates over all tables and finds the tables that should have TOAST tables, but don't (because binary-upgrade backend mode suppressed their creation), and adds them. However, that would be a lot of code and might be risky to backpatch. The error is so rare I am not sure it is worth it. I tried to create the failure case and it was very difficult, requiring me to create the problem table first, then some dummy stuff to get the offsets right so the oid would collide. Based on the rareness of the bug, I am not sure it is worth it, but if it is, it would be something only for 9.4 (maybe) and 9.5, not something to backpatch. Another idea would be to renumber empty toast tables that conflict during new toast table creation, when in binary upgrade mode. Since the files are always empty in binary upgrade mode, you could just create a new toast table, repoint the old table to use (because it didn't ask for a specific toast table oid because it didn't need one), and then use that one for the table that actually requested the oid. However, if one is a heap (zero size), and one is an index (8k size), it wouldn't work and you would have to recreate the file system file too. That seems a lot more localized than the other approaches. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Fri, Jul 4, 2014 at 12:01:37AM -0400, Bruce Momjian wrote:
The most robust, but not trivial, approach seems to be to prevent toast
table creation if there wasn't a set_next_toast_pg_class_oid(). Then,
after all relations are created, iterate over all pg_class entries that
possibly need toast tables and recheck if they now might need one.
Wow, that is going to be kind of odd in that there really isn't a good
way to create toast tables except perhaps add a dummy TEXT column and
remove it. There also isn't an easy way to not create a toast table,
but also find out that one was needed. I suppose we would have to
insert some dummy value in the toast pg_class column and come back later
to clean it up.
I am wondering what the probability of having a table that didn't need a
toast table in the old cluster, and needed one in the new cluster, and
there being an oid collision.
I think the big question is whether we want to go down that route.
Here is an updated patch. It was a little tricky because if the
mismatched non-toast table is after the last old relation, you need to
test differently and emit a different error message as there is no old
relation to complain about.
As far as the reusing of oids, we don't set the oid counter until after
the restore, so any new unmatched toast table would given a very low
oid. Since we restore in oid order, for an oid to be assigned that was
used in the old cluster, it would have to be a very early relation, so I
think that reduces the odds considerably. I am not inclined to do
anything more to avoid this until I see an actual error report ---
trying to address it might be invasive and introduce new errors.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
new file mode 100644
index 6205c74..72f805c
*** a/contrib/pg_upgrade/info.c
--- b/contrib/pg_upgrade/info.c
*** gen_db_file_maps(DbInfo *old_db, DbInfo
*** 38,58
int *nmaps, const char *old_pgdata, const char *new_pgdata)
{
FileNameMap *maps;
! int relnum;
int num_maps = 0;
maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
old_db-rel_arr.nrels);
! for (relnum = 0; relnum Min(old_db-rel_arr.nrels, new_db-rel_arr.nrels);
! relnum++)
{
! RelInfo*old_rel = old_db-rel_arr.rels[relnum];
! RelInfo*new_rel = new_db-rel_arr.rels[relnum];
if (old_rel-reloid != new_rel-reloid)
! pg_fatal(Mismatch of relation OID in database \%s\: old OID %d, new OID %d\n,
! old_db-db_name, old_rel-reloid, new_rel-reloid);
/*
* TOAST table names initially match the heap pg_class oid. In
--- 38,96
int *nmaps, const char *old_pgdata, const char *new_pgdata)
{
FileNameMap *maps;
! int old_relnum, new_relnum;
int num_maps = 0;
maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
old_db-rel_arr.nrels);
! /*
! * The old database shouldn't have more relations than the new one.
! * We force the new cluster to have a TOAST table if the old table
! * had one.
! */
! if (old_db-rel_arr.nrels new_db-rel_arr.nrels)
! pg_fatal(old and new databases \%s\ have a mismatched number of relations\n,
! old_db-db_name);
!
! /* Drive the loop using new_relnum, which might be higher. */
! for (old_relnum = new_relnum = 0; new_relnum new_db-rel_arr.nrels;
! new_relnum++)
{
! RelInfo*old_rel;
! RelInfo*new_rel = new_db-rel_arr.rels[new_relnum];
!
! /*
! * It is possible that the new cluster has a TOAST table for a table
! * that didn't need one in the old cluster, e.g. 9.0 to 9.1 changed the
! * NUMERIC length computation. Therefore, if we have a TOAST table
! * in the new cluster that doesn't match, skip over it and continue
! * processing. It is possible this TOAST table used an OID that was
! * reserved in the old cluster, but we have no way of testing that,
! * and we would have already gotten an error at the new cluster schema
! * creation stage. Fortunately, since we only restore the OID counter
! * after schema restore, and restore in OID order, a conflict would
! * only happen if the new TOAST table had a very low OID.
! */
! if (old_relnum == old_db-rel_arr.nrels)
! {
! if (strcmp(new_rel-nspname, pg_toast) == 0)
! continue;
! else
! pg_fatal(Extra non-TOAST relation found in database \%s\: new OID %d\n,
! old_db-db_name, new_rel-reloid);
! }
!
! old_rel = old_db-rel_arr.rels[old_relnum];
if (old_rel-reloid != new_rel-reloid)
! {
! if (strcmp(new_rel-nspname, pg_toast) == 0)
! continue;
! else
! pg_fatal(Mismatch of relation OID in database \%s\: old OID %d, new OID %d\n,
! old_db-db_name, old_rel-reloid, new_rel-reloid);
! }
/*
* TOAST table names
[HACKERS] Pg_upgrade and toast tables bug discovered
There have been periodic reports of pg_upgrade errors related to toast tables. The most recent one was from May of this year: http://www.postgresql.org/message-id/flat/20140520202223.gb3...@momjian.us#20140520202223.gb3...@momjian.us There error was: Copying user relation files /var/lib/postgresql/8.4/main/base/4275487/4278965 Mismatch of relation OID in database FNBooking: old OID 4279499, new OID 19792 Failure, exiting and the fix is to add a dummy TEXT column to the table on the old cluster to force a toast table, then drop the dummy column. I have had trouble getting a table schema that is causing problems, but received a report via EDB support recently that had a simple schema (anonymized): CREATE TABLE pg_upgrade_toast_test ( x1 numeric(15,0), x2 numeric(15,0), x3 character varying(15), x4 character varying(60), x5 numeric(15,0), x6 numeric(15,0), x7 character varying(15), x8 character varying(60), x9 numeric(15,0), x10 character varying(15), x11 character varying(60), x12 numeric(15,0), x13 numeric(15,0), x14 character varying(15), x15 character varying(60), x16 numeric(15,0), x17 character varying(15), x18 character varying(60), x19 numeric(15,0), x20 character varying(15), x21 character varying(60) ); needs_toast_table() computes the length of this table as 2024 bytes in 9.0, and 2064 bytes on 9.1, with the TOAST threshold being 2032 bytes. It turns out it is this commit that causes the difference: commit 97f38001acc61449f7ac09c539ccc29e40fecd26 Author: Robert Haas rh...@postgresql.org Date: Wed Aug 4 17:33:09 2010 + Fix numeric_maximum_size() calculation. The old computation can sometimes underestimate the necessary space by 2 bytes; however we're not back-patching this, because this result isn't used for anything critical. Per discussion with Tom Lane, make the typmod test in this function match the ones in numeric() and apply_typmod() exactly. It seems the impact of this patch on pg_upgrade wasn't considered, or even realized until now. Suggestions on a fix? My initial idea is to to allow for toast tables in the new cluster that aren't in the old cluster by skipping over the extra toast tables. This would only be for pre-9.1 old clusters. It would not involve adding toast tables to the old cluster as pg_upgrade never modifies the old cluster. We already handle cases where the old cluster had toast tables and the new cluster wouldn't ordinarily have them. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
Bruce Momjian br...@momjian.us writes: I have had trouble getting a table schema that is causing problems, but received a report via EDB support recently that had a simple schema (anonymized): ... needs_toast_table() computes the length of this table as 2024 bytes in 9.0, and 2064 bytes on 9.1, with the TOAST threshold being 2032 bytes. My initial idea is to to allow for toast tables in the new cluster that aren't in the old cluster by skipping over the extra toast tables. This would only be for pre-9.1 old clusters. TBH, it has never been more than the shakiest of assumptions that the new version could not create toast tables where the old one hadn't. I think you should just allow for that case, independently of specific PG versions. Fortunately it seems easy enough, since you certainly don't need to put any data into the new toast tables. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On 2014-07-03 17:09:41 -0400, Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
I have had trouble getting a table schema that is causing problems, but
received a report via EDB support recently that had a simple schema
(anonymized):
...
needs_toast_table() computes the length of this table as 2024 bytes in
9.0, and 2064 bytes on 9.1, with the TOAST threshold being 2032 bytes.
My initial idea is to to allow for toast tables in the new cluster that
aren't in the old cluster by skipping over the extra toast tables. This
would only be for pre-9.1 old clusters.
TBH, it has never been more than the shakiest of assumptions that the new
version could not create toast tables where the old one hadn't. I think
you should just allow for that case, independently of specific PG
versions. Fortunately it seems easy enough, since you certainly don't
need to put any data into the new toast tables.
I don't think it's just that simple unfortunately. If pg_class entries
get created that didn't exist on the old server there's a chance for oid
conflicts. Consider
SELECT binary_upgrade.set_next_heap_pg_class_oid('17094'::pg_catalog.oid);
CREATE TABLE table_without_toast_in_old_server(...);
-- heap oid 17094, toast oid 16384
SELECT binary_upgrade.set_next_heap_pg_class_oid('16384'::pg_catalog.oid);
CREATE TABLE another_table(...);
ERROR: could not create file ...: File exists
I think we even had reports of such a problem.
The most robust, but not trivial, approach seems to be to prevent toast
table creation if there wasn't a set_next_toast_pg_class_oid(). Then,
after all relations are created, iterate over all pg_class entries that
possibly need toast tables and recheck if they now might need one.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Jul 3, 2014 at 05:09:41PM -0400, Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
I have had trouble getting a table schema that is causing problems, but
received a report via EDB support recently that had a simple schema
(anonymized):
...
needs_toast_table() computes the length of this table as 2024 bytes in
9.0, and 2064 bytes on 9.1, with the TOAST threshold being 2032 bytes.
My initial idea is to to allow for toast tables in the new cluster that
aren't in the old cluster by skipping over the extra toast tables. This
would only be for pre-9.1 old clusters.
TBH, it has never been more than the shakiest of assumptions that the new
version could not create toast tables where the old one hadn't. I think
you should just allow for that case, independently of specific PG
versions. Fortunately it seems easy enough, since you certainly don't
need to put any data into the new toast tables.
OK, patch attached.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
new file mode 100644
index 6205c74..17e4d5b
*** a/contrib/pg_upgrade/info.c
--- b/contrib/pg_upgrade/info.c
*** gen_db_file_maps(DbInfo *old_db, DbInfo
*** 38,58
int *nmaps, const char *old_pgdata, const char *new_pgdata)
{
FileNameMap *maps;
! int relnum;
int num_maps = 0;
maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
old_db-rel_arr.nrels);
! for (relnum = 0; relnum Min(old_db-rel_arr.nrels, new_db-rel_arr.nrels);
! relnum++)
{
! RelInfo*old_rel = old_db-rel_arr.rels[relnum];
! RelInfo*new_rel = new_db-rel_arr.rels[relnum];
if (old_rel-reloid != new_rel-reloid)
! pg_fatal(Mismatch of relation OID in database \%s\: old OID %d, new OID %d\n,
! old_db-db_name, old_rel-reloid, new_rel-reloid);
/*
* TOAST table names initially match the heap pg_class oid. In
--- 38,85
int *nmaps, const char *old_pgdata, const char *new_pgdata)
{
FileNameMap *maps;
! int old_relnum, new_relnum;
int num_maps = 0;
maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
old_db-rel_arr.nrels);
! /*
! * The old database shouldn't have more relations than the new one.
! * We force the new cluster to have a TOAST table if the old table
! * had one.
! */
! if (old_db-rel_arr.nrels new_db-rel_arr.nrels)
! pg_fatal(old and new databases \%s\ have a mismatched number of relations\n,
! old_db-db_name);
!
! /* Drive the loop using new_relnum, which might be higher. */
! for (old_relnum = new_relnum = 0; new_relnum new_db-rel_arr.nrels;
! new_relnum++)
{
! RelInfo*old_rel = old_db-rel_arr.rels[old_relnum];
! RelInfo*new_rel = new_db-rel_arr.rels[new_relnum];
!
if (old_rel-reloid != new_rel-reloid)
! {
! if (strcmp(new_rel-nspname, pg_toast) == 0)
! /*
! * It is possible that the new cluster has a TOAST table
! * for a table that didn't need one in the old cluster,
! * e.g. 9.0 to 9.1 changed the NUMERIC length computation.
! * Therefore, if we have a TOAST table in the new cluster
! * that doesn't match, skip over it and continue processing.
! * It is possible this TOAST table used an OID that was
! * reserved in the old cluster, but we have no way of
! * testing that, and we would have already gotten an error
! * at the new cluster schema creation stage.
! */
! continue;
! else
! pg_fatal(Mismatch of relation OID in database \%s\: old OID %d, new OID %d\n,
! old_db-db_name, old_rel-reloid, new_rel-reloid);
! }
/*
* TOAST table names initially match the heap pg_class oid. In
*** gen_db_file_maps(DbInfo *old_db, DbInfo
*** 76,89
create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
old_rel, new_rel, maps + num_maps);
num_maps++;
}
! /*
! * Do this check after the loop so hopefully we will produce a clearer
! * error above
! */
! if (old_db-rel_arr.nrels != new_db-rel_arr.nrels)
! pg_fatal(old and new databases \%s\ have a different number of relations\n,
old_db-db_name);
*nmaps = num_maps;
--- 103,114
create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
old_rel, new_rel, maps + num_maps);
num_maps++;
+ old_relnum++;
}
! /* Did we fail to exhaust the old array? */
! if (old_relnum != old_db-rel_arr.nrels)
! pg_fatal(old and new databases \%s\ have a mismatched number of relations\n,
old_db-db_name);
*nmaps = num_maps;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Pg_upgrade and toast tables bug discovered
On Thu, Jul 3, 2014 at 11:55:40PM +0200, Andres Freund wrote:
I don't think it's just that simple unfortunately. If pg_class entries
get created that didn't exist on the old server there's a chance for oid
conflicts. Consider
SELECT binary_upgrade.set_next_heap_pg_class_oid('17094'::pg_catalog.oid);
CREATE TABLE table_without_toast_in_old_server(...);
-- heap oid 17094, toast oid 16384
SELECT binary_upgrade.set_next_heap_pg_class_oid('16384'::pg_catalog.oid);
CREATE TABLE another_table(...);
ERROR: could not create file ...: File exists
I think we even had reports of such a problem.
I had not considered this.
I don't remember ever seeing such a report. We have had oid mismatch
reports, but we now know the cause of those.
The most robust, but not trivial, approach seems to be to prevent toast
table creation if there wasn't a set_next_toast_pg_class_oid(). Then,
after all relations are created, iterate over all pg_class entries that
possibly need toast tables and recheck if they now might need one.
Wow, that is going to be kind of odd in that there really isn't a good
way to create toast tables except perhaps add a dummy TEXT column and
remove it. There also isn't an easy way to not create a toast table,
but also find out that one was needed. I suppose we would have to
insert some dummy value in the toast pg_class column and come back later
to clean it up.
I am wondering what the probability of having a table that didn't need a
toast table in the old cluster, and needed one in the new cluster, and
there being an oid collision.
I think the big question is whether we want to go down that route.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
