Re: Re: [Pg-migrator-general] Composite types break pg_migrated tables

I received the following pg_migrator bug report today and was able to
reproduce the reported failure when using composite types:

test=> SELECT * FROM breakmigrator;
ERROR: cache lookup failed for type 27604

test=> ANALYZE VERBOSE public.breakmigrator;
INFO: analyzing "public.breakmigrator"
INFO: "breakmigrator": scanned 1 of 1 pages, containing 3 live rows and
0 dead rows; 3 rows in sample, 3 estimated total rows
ERROR: cache lookup failed for type 27604

There is no pg_type row with oid 27604.

Can anyone suggest the cause? Do we embed the object oid in the
composite object? Did we change the composite object storage layout
between 8.3 and 8.4? I am surprised the regression tests didn't show
this error. (I just tried ANALYZE on the regression database and it
succeeded.)

---------------------------------------------------------------------------

Jeff wrote:
> I'm running some tests of pg_migrator and at first glance it appeared
> things were fine, but alas, that was not the truth.
>
> In a nutshell: if you have a table with a composite type as a column
> the migrated table is unusable (cache lookup errors)
> I'm testing with 8.3.7 and 8.4.0 on osx (it also happens on linux -
> where I first observed it)
>
> Here's how to reproduce:
>
> Fire up an 8.3 instance and install the following sql:
>
> create type footype as
> (
> x double precision,
> y double precision,
> z double precision
> );
>
> create table breakmigrator
> (
> id int,
> foo_a footype
> );
>
> insert into breakmigrator (id, foo_a)
> values (1, (1,2,3));
> insert into breakmigrator (id, foo_a)
> values (2, (1,2,3));
> insert into breakmigrator (id, foo_a)
> values (3, (1,2,3));
>
>
> then run pg_migrator to upgrade it to 8.4
> ... "*Upgrade complete*...
>
> fire up 8.4 and then try to vacuum the breakmigrator table:
>
> jeff=# vacuum analyze verbose breakmigrator;
> INFO: vacuuming "public.breakmigrator"
> INFO: "breakmigrator": found 0 removable, 3 nonremovable row versions
> in 1 out of 1 pages
> DETAIL: 0 dead row versions cannot be removed yet.
> There were 0 unused item pointers.
> 0 pages are entirely empty.
> CPU 0.00s/0.00u sec elapsed 0.01 sec.
> INFO: vacuuming "pg_toast.pg_toast_16406"
> INFO: index "pg_toast_16406_index" now contains 0 row versions in 1
> pages
> DETAIL: 0 index row versions were removed.
> 0 index pages have been deleted, 0 are currently reusable.
> CPU 0.00s/0.00u sec elapsed 0.00 sec.
> INFO: "pg_toast_16406": found 0 removable, 0 nonremovable row
> versions in 0 out of 0 pages
> DETAIL: 0 dead row versions cannot be removed yet.
> There were 0 unused item pointers.
> 0 pages are entirely empty.
> CPU 0.00s/0.00u sec elapsed 0.00 sec.
> INFO: analyzing "public.breakmigrator"
> INFO: "breakmigrator": scanned 1 of 1 pages, containing 3 live rows
> and 0 dead rows; 3 rows in sample, 3 estimated total rows
> ERROR: cache lookup failed for type 16387
> STATEMENT: vacuum analyze verbose breakmigrator;
> ERROR: cache lookup failed for type 16387
>
>
> thanks!
> --
> Jeff Trout <jeff(at)jefftrout(dot)com>
> http://www.stuarthamm.net/
> http://www.dellsmartexitin.com/
>
>
>
> _______________________________________________
> Pg-migrator-general mailing list
> Pg-migrator-general(at)pgfoundry(dot)org
> http://pgfoundry.org/mailman/listinfo/pg-migrator-general

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
>
> I received the following pg_migrator bug report today and was able to
> reproduce the reported failure when using composite types:
>
> test=> SELECT * FROM breakmigrator;
> ERROR: cache lookup failed for type 27604
>
> test=> ANALYZE VERBOSE public.breakmigrator;
> INFO: analyzing "public.breakmigrator"
> INFO: "breakmigrator": scanned 1 of 1 pages, containing 3 live rows and
> 0 dead rows; 3 rows in sample, 3 estimated total rows
> ERROR: cache lookup failed for type 27604
>
> There is no pg_type row with oid 27604.
>
> Can anyone suggest the cause? Do we embed the object oid in the
> composite object? Did we change the composite object storage layout
> between 8.3 and 8.4? I am surprised the regression tests didn't show
> this error. (I just tried ANALYZE on the regression database and it
> succeeded.)

More info: I found 27604 in the old 8.3 database:

test=> SELECT * FROM pg_type WHERE oid = 27604;
-[ RECORD 1 ]-+------------
typname | footype
typnamespace | 2200
typowner | 10
typlen | -1
typbyval | f
typtype | c
typisdefined | t
typdelim | ,
typrelid | 27602
typelem | 0
typarray | 27603
typinput | record_in
typoutput | record_out
typreceive | record_recv
typsend | record_send
typmodin | -
typmodout | -
typanalyze | -
typalign | d
typstorage | x
typnotnull | f
typbasetype | 0
typtypmod | -1
typndims | 0
typdefaultbin |
typdefault |

'footype' has a different oid in the new 8.4 database:

test=> SELECT oid, * FROM pg_type WHERE typname = 'footype';
-[ RECORD 1 ]--+------------
oid | 17580
typname | footype
typnamespace | 2200
typowner | 10
typlen | -1
typbyval | f
typtype | c
typcategory | C
typispreferred | f
typisdefined | t
typdelim | ,
typrelid | 17578
typelem | 0
typarray | 17579
typinput | record_in
typoutput | record_out
typreceive | record_recv
typsend | record_send
typmodin | -
typmodout | -
typanalyze | -
typalign | d
typstorage | x
typnotnull | f
typbasetype | 0
typtypmod | -1
typndims | 0
typdefaultbin |
typdefault |

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> I received the following pg_migrator bug report today and was able to
> reproduce the reported failure when using composite types:

> test=> SELECT * FROM breakmigrator;
> ERROR: cache lookup failed for type 27604

Hm ... has anyone tested pg_migrator using either composite types or
arrays of user-defined types? Both of them have got user-defined-type
OIDs in on-disk data, now that I think about it. For that matter, enums
are going to be a problem too.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > I received the following pg_migrator bug report today and was able to
> > reproduce the reported failure when using composite types:
>
> > test=> SELECT * FROM breakmigrator;
> > ERROR: cache lookup failed for type 27604
>
> Hm ... has anyone tested pg_migrator using either composite types or
> arrays of user-defined types? Both of them have got user-defined-type
> OIDs in on-disk data, now that I think about it. For that matter, enums
> are going to be a problem too.

Don't arrays have embedded element OIDs too?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tom Lane wrote:
>> Hm ... has anyone tested pg_migrator using either composite types or
>> arrays of user-defined types? Both of them have got user-defined-type
>> OIDs in on-disk data, now that I think about it. For that matter, enums
>> are going to be a problem too.

> Don't arrays have embedded element OIDs too?

Er, that's what I said. It looks nasty :-(

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > I received the following pg_migrator bug report today and was able to
> > reproduce the reported failure when using composite types:
>
> > test=> SELECT * FROM breakmigrator;
> > ERROR: cache lookup failed for type 27604
>
> Hm ... has anyone tested pg_migrator using either composite types or
> arrays of user-defined types? Both of them have got user-defined-type
> OIDs in on-disk data, now that I think about it. For that matter, enums
> are going to be a problem too.

Yep, I realized that since I posted. It seems composite types are
mini-heap tuples, except that instead of xmin/xmax, they have type
information:

typedef struct DatumTupleFields
{
int32 datum_len_; /* varlena header (do not touch directly!) */

int32 datum_typmod; /* -1, or identifier of a record type */

Oid datum_typeid; /* composite type OID, or RECORDOID */

/*
* Note: field ordering is chosen with thought that Oid might someday
* widen to 64 bits.
*/
} DatumTupleFields;

datum_typeid is where the composite type oid is stored.

Do we have no composite types in the regression tests, or do we not
store any in the database? Same the enums.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > Tom Lane wrote:
> >> Hm ... has anyone tested pg_migrator using either composite types or
> >> arrays of user-defined types? Both of them have got user-defined-type
> >> OIDs in on-disk data, now that I think about it. For that matter, enums
> >> are going to be a problem too.
>
> > Don't arrays have embedded element OIDs too?
>
> Er, that's what I said. It looks nasty :-(

Seems we have two possible directions to go in. First I can easily
cause pg_migrator to exit if it finds any of these issues in any
database.

To allow pg_migrator to work, I would need to reserve the oids in
pg_type, import the dump, and renumber the pg_type entries (and
everything pointing to them) to the proper pg_type.oid. The big problem
there is that I don't have access at the SQL level to set or change
oids. I am afraid the oid remumbering is something we would have to do
in the backend by walking through the pg_depend entries for the pg_type
row. Yuck.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> To allow pg_migrator to work, I would need to reserve the oids in
> pg_type, import the dump, and renumber the pg_type entries (and
> everything pointing to them) to the proper pg_type.oid. The big problem
> there is that I don't have access at the SQL level to set or change
> oids. I am afraid the oid remumbering is something we would have to do
> in the backend by walking through the pg_depend entries for the pg_type
> row. Yuck.

Renumbering type OIDs after-the-fact seems impossibly messy --- there's
not even any support in the backend for changing the OID of an existing
row, let alone any way to do it from the SQL level. And you'd have to
find and fix all the references elsewhere in the system catalogs. And
what about collisions?

ISTM the only reasonable way to deal with this would be to have some way
for pg_dump to emit commands to create types with specific OIDs. While
we were at it, we might as well add the ability to specify toast-table
OIDs so as to get rid of the kluge that's doing that now.

At the moment it looks to me like pg_migrator has crashed and burned
for 8.4, at least for general-purpose usage. We might be able to have
support for this stuff in 8.5. But not being able to deal with any
user-defined types is too much of a restriction to make it of general
interest.

regards, tom lane

Bruce Momjian wrote:
> Do we have no composite types in the regression tests, or do we not
> store any in the database? Same the enums.
>
>

Looks like the enum regression tests at least drop all their tables :-(

> To allow pg_migrator to work, I would need to reserve the oids in
> pg_type, import the dump, and renumber the pg_type entries (and
> everything pointing to them) to the proper pg_type.oid. The big problem
> there is that I don't have access at the SQL level to set or change
> oids. I am afraid the oid remumbering is something we would have to do
> in the backend by walking through the pg_depend entries for the pg_type
> row. Yuck.

Yeah. Maybe we need some special way of setting the oids explicitly. But
preventing a clash might be fairly difficult.

Excluding every database that has a composite/array-of
user-defined-type/enum type would be pretty nasty. After all, these are
features we boast of.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> preventing a clash might be fairly difficult.

Yeah, I was just thinking about that. The easiest way to avoid
collisions would be to make pg_dump (in --binary-upgrade mode)
responsible for being sure that *every* new pg_type and pg_class row
OID matches what it was in the old DB. We could stop doing that
once we have all the user tables in place --- I don't believe it's
necessary to preserve the OIDs of user indexes. But we need to
preserve toast table OIDs, and toast table index OIDs too if those
are created at the same time they are now (else we risk one of them
colliding with a toast table OID we want to create later).

Oh, and pg_enum rows too.

It seems doable, but we're certainly not going to back-patch
any such thing into 8.4 ...

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> preventing a clash might be fairly difficult.
>>
>
> Yeah, I was just thinking about that. The easiest way to avoid
> collisions would be to make pg_dump (in --binary-upgrade mode)
> responsible for being sure that *every* new pg_type and pg_class row
> OID matches what it was in the old DB. We could stop doing that
> once we have all the user tables in place --- I don't believe it's
> necessary to preserve the OIDs of user indexes. But we need to
> preserve toast table OIDs, and toast table index OIDs too if those
> are created at the same time they are now (else we risk one of them
> colliding with a toast table OID we want to create later).
>
> Oh, and pg_enum rows too.
>
> It seems doable, but we're certainly not going to back-patch
> any such thing into 8.4 ...
>
>
>

Is there any danger that an oid used in, say, pg_enum in the old version
will be used in the catalog bootstrap in the new version?

cheers

andrew

Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
> > Do we have no composite types in the regression tests, or do we not
> > store any in the database? Same the enums.
> >
> >
>
> Looks like the enum regression tests at least drop all their tables :-(
>
> > To allow pg_migrator to work, I would need to reserve the oids in
> > pg_type, import the dump, and renumber the pg_type entries (and
> > everything pointing to them) to the proper pg_type.oid. The big problem
> > there is that I don't have access at the SQL level to set or change
> > oids. I am afraid the oid remumbering is something we would have to do
> > in the backend by walking through the pg_depend entries for the pg_type
> > row. Yuck.
>
> Yeah. Maybe we need some special way of setting the oids explicitly. But
> preventing a clash might be fairly difficult.
>
> Excluding every database that has a composite/array-of
> user-defined-type/enum type would be pretty nasty. After all, these are
> features we boast of.

Well, pg_migrator has gotten pretty far without supporting these
features, and I think I would have heard about it if someone had these
and migrated because vacuum analyze found it right away. I am afraid
the best we can do is to throw an error when we see these cases and hope
we can improve things for 8.5.

As I understand it I have to look for the _use_ of these in user tables,
not the existance of them in pg_type --- for example, there is
certainly an array for every user type, but it might not be used by any
user tables, and that would be OK.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Is there any danger that an oid used in, say, pg_enum in the old version
> will be used in the catalog bootstrap in the new version?

No. All initdb-assigned OIDs are less than 16K, and we never assign
such an OID post-initdb (not even when wrapping around). We might get
into trouble if we ever run out of OIDs below 16K, but I don't foresee
that happening anytime soon.

Also, the design I sketched depends on the fact that it doesn't matter
if, say, a pg_proc row gets an OID that we also need to use in pg_enum.
We only need OID uniqueness within each specific catalog. So we don't
need to control the OID assignments in catalogs other than the three
we are interested in.

regards, tom lane

Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
> > Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >
> >> preventing a clash might be fairly difficult.
> >>
> >
> > Yeah, I was just thinking about that. The easiest way to avoid
> > collisions would be to make pg_dump (in --binary-upgrade mode)
> > responsible for being sure that *every* new pg_type and pg_class row
> > OID matches what it was in the old DB. We could stop doing that
> > once we have all the user tables in place --- I don't believe it's
> > necessary to preserve the OIDs of user indexes. But we need to
> > preserve toast table OIDs, and toast table index OIDs too if those
> > are created at the same time they are now (else we risk one of them
> > colliding with a toast table OID we want to create later).
> >
> > Oh, and pg_enum rows too.
> >
> > It seems doable, but we're certainly not going to back-patch
> > any such thing into 8.4 ...
> >
> >
> >
>
> Is there any danger that an oid used in, say, pg_enum in the old version
> will be used in the catalog bootstrap in the new version?

No because the catalog bootstrap oids are all lower than
FirstNormalObjectId. The _big_ problem is the creation of pg_type oids
while other things are being created, e.g. you say to create an object
of fixed oid 123 and the array is created as 124, and later you need to
use 124 as a fixed oid. We will need to assign _every_ pg_type oid from
pg_dump so we are sure there are not some assigned that we will need
later.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Andrew Dunstan wrote:
> >
> >
> > Bruce Momjian wrote:
> > > Do we have no composite types in the regression tests, or do we not
> > > store any in the database? Same the enums.
> > >
> > >
> >
> > Looks like the enum regression tests at least drop all their tables :-(
> >
> > > To allow pg_migrator to work, I would need to reserve the oids in
> > > pg_type, import the dump, and renumber the pg_type entries (and
> > > everything pointing to them) to the proper pg_type.oid. The big problem
> > > there is that I don't have access at the SQL level to set or change
> > > oids. I am afraid the oid remumbering is something we would have to do
> > > in the backend by walking through the pg_depend entries for the pg_type
> > > row. Yuck.
> >
> > Yeah. Maybe we need some special way of setting the oids explicitly. But
> > preventing a clash might be fairly difficult.
> >
> > Excluding every database that has a composite/array-of
> > user-defined-type/enum type would be pretty nasty. After all, these are
> > features we boast of.
>
> Well, pg_migrator has gotten pretty far without supporting these
> features, and I think I would have heard about it if someone had these
> and migrated because vacuum analyze found it right away. I am afraid
> the best we can do is to throw an error when we see these cases and hope
> we can improve things for 8.5.
>
> As I understand it I have to look for the _use_ of these in user tables,
> not the existance of them in pg_type --- for example, there is
> certainly an array for every user type, but it might not be used by any
> user tables, and that would be OK.

I have applied the attached patch to pg_migrator to detect enum,
composites, and arrays. I tested it and the only error I got was with
the breakmigrator table that was supplied by Jeff, and once I removed
that table the migration went fine, meaning there are no cases of these
stored in the regression test database.

I will release a new version of pg_migrator with these new detection
routines.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> preventing a clash might be fairly difficult.
>
> Yeah, I was just thinking about that. The easiest way to avoid
> collisions would be to make pg_dump (in --binary-upgrade mode)
> responsible for being sure that *every* new pg_type and pg_class row
> OID matches what it was in the old DB.

As we already have WITH OIDS for CREATE TABLE command, maybe adding
support for WITH OID ... to the necessary commands would do the trick?

Instead of messing with pg_type, pg_dump would then have to issue a OID
'decorated' command such as
CREATE TYPE footype ... WITH OID 27604;

> We could stop doing that
> once we have all the user tables in place --- I don't believe it's
> necessary to preserve the OIDs of user indexes. But we need to
> preserve toast table OIDs, and toast table index OIDs too if those
> are created at the same time they are now (else we risk one of them
> colliding with a toast table OID we want to create later).

It seems harder to come up with a general purpose syntax to support the
feature in case of toast tables, though.

Regards,
--
dim

Tom Lane írta:
> At the moment it looks to me like pg_migrator has crashed and burned
> for 8.4, at least for general-purpose usage.

It means that you don't have the restraint that
you thought you have. So you can change the
RedHat/Fedora PostgreSQL 8.4 packages to use
the upstream default for integer timestamps...

Best regards,
Zoltán Böszörményi

--
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

On Thursday 06 August 2009 06:32:06 Bruce Momjian wrote:
> I have applied the attached patch to pg_migrator to detect enum,
> composites, and arrays. I tested it and the only error I got was with
> the breakmigrator table that was supplied by Jeff, and once I removed
> that table the migration went fine, meaning there are no cases of these
> stored in the regression test database.

That might be a bit excessive. As I understand it, arrays of built-in types
(e.g., int[]) should work fine. I suspect the majority of uses of arrays will
be with built-in types, so allowing that would help a significant portion of
installations.

Dimitri Fontaine wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> > We could stop doing that
> > once we have all the user tables in place --- I don't believe it's
> > necessary to preserve the OIDs of user indexes. But we need to
> > preserve toast table OIDs, and toast table index OIDs too if those
> > are created at the same time they are now (else we risk one of them
> > colliding with a toast table OID we want to create later).
>
> It seems harder to come up with a general purpose syntax to support the
> feature in case of toast tables, though.

There's already general purpose syntax for relation options which can be
used to get options that do not ultimately end up in
pg_class.reloptions. An existing example is WITH (oids). One such
option could be used here.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

> Excluding every database that has a composite/array-of
> user-defined-type/enum type would be pretty nasty. After all, these
> are features we boast of.

Any idea whether domains are an issue? I was thinking of trying this
tool soon, and we don't seem to be using any of the problem features
-- unless type issues include domains.

-Kevin

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Dimitri Fontaine wrote:
>> It seems harder to come up with a general purpose syntax to support the
>> feature in case of toast tables, though.

> There's already general purpose syntax for relation options which can be
> used to get options that do not ultimately end up in
> pg_class.reloptions. An existing example is WITH (oids). One such
> option could be used here.

That would cover the problem for OIDs needed during CREATE TABLE, but
what about types and enum values?

The half-formed idea I had was a set of GUC variables:

set next_pg_class_oid = 12345;
set next_pg_type_oid = 12346;
set next_toast_table_oid = ...
set next_toast_index_oid = ...

and finally it could do CREATE TABLE. CREATE TYPE would only need
next_pg_type_oid (except for a composite type).

Enum values wouldn't work too well this way, unless we were willing to
have a GUC that took a list of OIDs. I thought about having binary
upgrade mode build up the enum list one entry at a time, by adding
a command like

ALTER TYPE enum_type ADD VALUE 'label' WITH OID oid

which would also have some use for modifying enums on the fly.

regards, tom lane

Kevin Grittner wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
>> Excluding every database that has a composite/array-of
>> user-defined-type/enum type would be pretty nasty. After all, these
>> are features we boast of.
>>
>
> Any idea whether domains are an issue? I was thinking of trying this
> tool soon, and we don't seem to be using any of the problem features
> -- unless type issues include domains.
>
>
>

I don't believe that they are an issue. The issue arises only when a
catalog oid is used in the on-disk representation of a type. AFAIK the
on-disk representation of a domain is the same as its base type.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Kevin Grittner wrote:
>> Any idea whether domains are an issue?

> I don't believe that they are an issue. The issue arises only when a
> catalog oid is used in the on-disk representation of a type. AFAIK the
> on-disk representation of a domain is the same as its base type.

Arrays of domains would be a problem, if we had 'em, which we don't...

Also, as Peter already noted, arrays of built-in types are not really a
problem because the OID won't have changed since 8.3. It's only arrays
of types created post-initdb that are risk factors.

regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>
>> Dimitri Fontaine wrote:
>>
>>> It seems harder to come up with a general purpose syntax to support the
>>> feature in case of toast tables, though.
>>>
>
>
>> There's already general purpose syntax for relation options which can be
>> used to get options that do not ultimately end up in
>> pg_class.reloptions. An existing example is WITH (oids). One such
>> option could be used here.
>>
>
> That would cover the problem for OIDs needed during CREATE TABLE, but
> what about types and enum values?
>
> The half-formed idea I had was a set of GUC variables:
>
> set next_pg_class_oid = 12345;
> set next_pg_type_oid = 12346;
> set next_toast_table_oid = ...
> set next_toast_index_oid = ...
>
> and finally it could do CREATE TABLE. CREATE TYPE would only need
> next_pg_type_oid (except for a composite type).
>
> Enum values wouldn't work too well this way, unless we were willing to
> have a GUC that took a list of OIDs. I thought about having binary
> upgrade mode build up the enum list one entry at a time, by adding
> a command like
>
> ALTER TYPE enum_type ADD VALUE 'label' WITH OID oid
>
> which would also have some use for modifying enums on the fly.
>
>
>

It's going to be fairly grotty whatever we do. I'm worried a bit that
we'll be providing some footguns, but I guess we'll just need to hold
our noses and do whatever it takes.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> It's going to be fairly grotty whatever we do. I'm worried a bit that
> we'll be providing some footguns, but I guess we'll just need to hold
> our noses and do whatever it takes.

Yeah. One advantage of the GUC approach is we could make 'em SUSET.
I don't actually see any particularly serious risk of abuse there
(about all you could do is make your CREATEs fail) ... but why not
be careful ...

regards, tom lane

On Thu, Aug 6, 2009 at 4:32 AM, Dimitri Fontaine<dfontaine(at)hi-media(dot)com> wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>> preventing a clash might be fairly difficult.
>>
>> Yeah, I was just thinking about that.  The easiest way to avoid
>> collisions would be to make pg_dump (in --binary-upgrade mode)
>> responsible for being sure that *every* new pg_type and pg_class row
>> OID matches what it was in the old DB.
>
> As we already have WITH OIDS for CREATE TABLE command, maybe adding
> support for WITH OID ... to the necessary commands would do the trick?
>
> Instead of messing with pg_type, pg_dump would then have to issue a OID
> 'decorated' command such as
>  CREATE TYPE footype ... WITH OID 27604;

Unfortunately it's not enough to just do this with 'create type' and
'create type as', we also have to do this with 'create table'. Some
people (like me) use tables as composite types because of the extra
flexibility it gives you. So, potentially, OIDs for enums, tables,
and types needs to be preserved.

I am very much in support for any system that allows creation of a
type with a specific OID. This is not just a problem with the
migrator, but will allow for more robust transfers of data over the
binary protocol (think binary dblink) without resorting to hacks to
that do lookups based on typename.

IOW, this setting specific OIDs should ideally be exposed at the SQL
level and should be able to be done for any type that can be part of a
container.

merlin

On Aug 6, 2009, at 7:28 AM, Tom Lane wrote:

> That would cover the problem for OIDs needed during CREATE TABLE, but
> what about types and enum values?

I haven't been following this discussion very closely, but wanted to
ask: is someone writing regression tests for these cases that
pg_migrator keeps bumping into?

Best,

David

On Thursday 06 August 2009 17:54:37 David E. Wheeler wrote:
> On Aug 6, 2009, at 7:28 AM, Tom Lane wrote:
> > That would cover the problem for OIDs needed during CREATE TABLE, but
> > what about types and enum values?
>
> I haven't been following this discussion very closely, but wanted to
> ask: is someone writing regression tests for these cases that
> pg_migrator keeps bumping into?

Well, pg_migrator has no included test suite. There you go.

On Wed, 2009-08-05 at 22:57 -0400, Bruce Momjian wrote:
> Andrew Dunstan wrote:
> >
> Well, pg_migrator has gotten pretty far without supporting these
> features, and I think I would have heard about it if someone had these
> and migrated because vacuum analyze found it right away. I am afraid
> the best we can do is to throw an error when we see these cases and hope
> we can improve things for 8.5.

*most* users will not even know there is such a thing as a composite
type. Throw an error and call it good for this release.

Joshua D. Drake

> --
> Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
> EnterpriseDB http://enterprisedb.com
>
> + If your life is a hard drive, Christ can be your backup. +
>
--
PostgreSQL - XMPP: jdrake(at)jabber(dot)postgresql(dot)org
Consulting, Development, Support, Training
503-667-4564 - http://www.commandprompt.com/
The PostgreSQL Company, serving since 1997

Peter Eisentraut wrote:
> On Thursday 06 August 2009 06:32:06 Bruce Momjian wrote:
> > I have applied the attached patch to pg_migrator to detect enum,
> > composites, and arrays. I tested it and the only error I got was with
> > the breakmigrator table that was supplied by Jeff, and once I removed
> > that table the migration went fine, meaning there are no cases of these
> > stored in the regression test database.
>
> That might be a bit excessive. As I understand it, arrays of built-in types
> (e.g., int[]) should work fine. I suspect the majority of uses of arrays will
> be with built-in types, so allowing that would help a significant portion of
> installations.

Agreed. I realized that last night, and have modified pg_migrator to
test FirstNormalObjectId.

The pg_migrator limitations are now:

pg_migrator will not work if a user column is defined as:

o data type tsquery
o data type 'name' and is not the first column
o a user-defined composite data type
o a user-defined array data type
o a user-defined enum data type

You must drop any such columns and migrate them manually.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

David E. Wheeler wrote:
> On Aug 6, 2009, at 7:28 AM, Tom Lane wrote:
>
> > That would cover the problem for OIDs needed during CREATE TABLE, but
> > what about types and enum values?
>
> I haven't been following this discussion very closely, but wanted to
> ask: is someone writing regression tests for these cases that
> pg_migrator keeps bumping into?

Yes, I have regression tests I run but they are not in CVS, partly
because they are tied to other scripts I have to manage server settings.

Here are my scripts:

http://momjian.us/tmp/pg_migrator_test.tgz

One big problem is that pg_migrator fails as soon as it hits one of
these so there isn't much to automate.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Joshua D. Drake wrote:
> On Wed, 2009-08-05 at 22:57 -0400, Bruce Momjian wrote:
> > Andrew Dunstan wrote:
> > >
> > Well, pg_migrator has gotten pretty far without supporting these
> > features, and I think I would have heard about it if someone had these
> > and migrated because vacuum analyze found it right away. I am afraid
> > the best we can do is to throw an error when we see these cases and hope
> > we can improve things for 8.5.
>
>
> *most* users will not even know there is such a thing as a composite
> type. Throw an error and call it good for this release.

Done, pg_migrator 8.4.3 released.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Aug 6, 2009, at 6:00 PM, Bruce Momjian wrote:

> Yes, I have regression tests I run but they are not in CVS, partly
> because they are tied to other scripts I have to manage server
> settings.
>
> Here are my scripts:
>
> http://momjian.us/tmp/pg_migrator_test.tgz
>
> One big problem is that pg_migrator fails as soon as it hits one of
> these so there isn't much to automate.

Perhaps when I return from vacation I'll have a look at these and see
if I can think of a way to automate them.

Best,

David

On Fri, Aug 7, 2009 at 1:56 AM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
>                o  data type 'name' and is not the first column
>

What was that about?

--
greg
http://mit.edu/~gsstark/resume.pdf

Dne 6.08.09 04:29, Tom Lane napsal(a):
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> preventing a clash might be fairly difficult.
>
> Yeah, I was just thinking about that. The easiest way to avoid
> collisions would be to make pg_dump (in --binary-upgrade mode)
> responsible for being sure that *every* new pg_type and pg_class row
> OID matches what it was in the old DB. We could stop doing that
> once we have all the user tables in place --- I don't believe it's
> necessary to preserve the OIDs of user indexes. But we need to
> preserve toast table OIDs, and toast table index OIDs too if those
> are created at the same time they are now (else we risk one of them
> colliding with a toast table OID we want to create later).
>
> Oh, and pg_enum rows too.
>
> It seems doable, but we're certainly not going to back-patch
> any such thing into 8.4 ...

Another way is to use direct catalog update which I presented on PgCon.
I think it should be easy to finish it (2-3weeks) for 8.4 - needs small
extension of bootstrap. And of course testing, testing ...

Also to remove oid in catalog and replace it with standard column (type
can be oid) should make things easier. But it is for 8.5.

I will send a code on Monday for people who wants to look on it.

Zdenek

Greg Stark wrote:
> On Fri, Aug 7, 2009 at 1:56 AM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
> > ? ? ? ? ? ? ? ?o ?data type 'name' and is not the first column
> >
>
> What was that about?

We changed the alignment of the 'name' column:

/*
* v8_3_check_for_name_data_type_usage()
*
* alignment for the 'name' data type changed to 'char' in 8.4;
* checks tables and indexes
*/

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Peter Eisentraut wrote:
>> That might be a bit excessive. As I understand it, arrays of built-in types
>> (e.g., int[]) should work fine. I suspect the majority of uses of arrays will
>> be with built-in types, so allowing that would help a significant portion of
>> installations.

> Agreed. I realized that last night, and have modified pg_migrator to
> test FirstNormalObjectId.

That's really the wrong thing. It's safe to assume OIDs below 10000
are portable across versions, because for them not to be would require
someone to have changed a hand assignment. However, OIDs between 10000
and 16K are assigned on-the-fly by initdb, and those are *not* likely
to be portable across versions. As an example, the rowtype for
pg_statistic has slightly different OIDs in 8.3 and 8.4. So if you
allow someone to port a database that is using a system catalog's
rowtype, it will fail. Admittedly that's not a real likely scenario,
but if you're going to have a check it should be accurate.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Peter Eisentraut wrote:
> >> That might be a bit excessive. As I understand it, arrays of built-in types
> >> (e.g., int[]) should work fine. I suspect the majority of uses of arrays will
> >> be with built-in types, so allowing that would help a significant portion of
> >> installations.
>
> > Agreed. I realized that last night, and have modified pg_migrator to
> > test FirstNormalObjectId.
>
> That's really the wrong thing. It's safe to assume OIDs below 10000
> are portable across versions, because for them not to be would require
> someone to have changed a hand assignment. However, OIDs between 10000
> and 16K are assigned on-the-fly by initdb, and those are *not* likely
> to be portable across versions. As an example, the rowtype for
> pg_statistic has slightly different OIDs in 8.3 and 8.4. So if you
> allow someone to port a database that is using a system catalog's
> rowtype, it will fail. Admittedly that's not a real likely scenario,
> but if you're going to have a check it should be accurate.

Thanks, I changed FirstNormalObjectId to FirstBootstrapObjectId for the
array/enum/composite oid test, and added a C comment about it.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Thu, Aug 6, 2009 at 9:28 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> The half-formed idea I had was a set of GUC variables:
>
> set next_pg_class_oid = 12345;
> set next_pg_type_oid = 12346;
> set next_toast_table_oid = ...
> set next_toast_index_oid = ...
>
> and finally it could do CREATE TABLE.  CREATE TYPE would only need
> next_pg_type_oid (except for a composite type).

Is this idea still on the table for 8.5?

merlin

Merlin Moncure <mmoncure(at)gmail(dot)com> writes:
> Is this idea still on the table for 8.5?

I've forgotten what the problem was?

regards, tom lane

Merlin Moncure wrote:
> On Thu, Aug 6, 2009 at 9:28 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > The half-formed idea I had was a set of GUC variables:
> >
> > set next_pg_class_oid = 12345;
> > set next_pg_type_oid = 12346;
> > set next_toast_table_oid = ...
> > set next_toast_index_oid = ...
> >
> > and finally it could do CREATE TABLE. ?CREATE TYPE would only need
> > next_pg_type_oid (except for a composite type).
>
> Is this idea still on the table for 8.5?

Well, pg_migrator still has these restrictions that will apply to
migrations to 8.5:

pg_migrator will not work if a user column is defined as:

o a user-defined composite data type
o a user-defined array data type
o a user-defined enum data type

You must drop any such columns and migrate them manually.

Having 'next_pg_type_oid' would fix that. The other three settings are
already handled by pg_migrator code. Having those three settings would
allow me to remove some pg_migrator code once we removed support for
migrations to 8.4.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Wed, Dec 2, 2009 at 11:28 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> >
>> > set next_pg_class_oid = 12345;
>> > set next_pg_type_oid = 12346;
>> > set next_toast_table_oid = ...
>> > set next_toast_index_oid = ...
>> >
>> > and finally it could do CREATE TABLE. ?CREATE TYPE would only need
>> > next_pg_type_oid (except for a composite type).
>>
>> Is this idea still on the table for 8.5?
>
> Well, pg_migrator still has these restrictions that will apply to
> migrations to 8.5:
>
>        pg_migrator will not work if a user column is defined as:
>
>                o  a user-defined composite data type
>                o  a user-defined array data type
>                o  a user-defined enum data type
>
>        You must drop any such columns and migrate them manually.
>
> Having 'next_pg_type_oid' would fix that.  The other three settings are
> already handled by pg_migrator code.  Having those three settings would
> allow me to remove some pg_migrator code once we removed support for
> migrations to 8.4.

I also have a personal interest for non pg_migrator reasons. The
basic problem is that there is no way to make oids consistent between
databases which causes headaches for things like migration and direct
transfer of data between databases in binary.

merlin