Pg_upgrade and toast tables bug discovered

Bruce Momjian <bruce@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

On 2014-07-03 17:09:41 -0400, Tom Lane wrote:

Bruce Momjian <bruce@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

On Thu, Jul 3, 2014 at 05:09:41PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

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 <bruce@momjian.us> http://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

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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Fri, Jul 4, 2014 at 11:12 PM, Bruce Momjian <bruce@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

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 <bruce@momjian.us> http://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

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 <bruce@momjian.us> http://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

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 <bruce@momjian.us> http://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

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

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 <bruce@momjian.us> http://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

On Wed, Jul 9, 2014 at 12:09 PM, Bruce Momjian <bruce@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

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 <bruce@momjian.us> http://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

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

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 <bruce@momjian.us> http://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

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 <bruce@momjian.us> http://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

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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

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 Herrera 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

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 <bruce@momjian.us> http://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