pg_migrator not setting values of sequences?

Started by Tilmann Singerover 16 years ago26 messages
#1Tilmann Singer
tils@tils.net

I tried the latest pg_migrator
(http://pgfoundry.org/frs/download.php/2291/pg_migrator-8.4.tgz) to
migrate a database from an 8.3.7 to an 8.4.0 cluster. It finished with
a success message, and the schema and all tables seem to have been
migrated correctly.

However, all of the sequences were at the initial values and not
bumped up to the last used value as I would have expected. The first
nextval call on any sequence in the migrated 8.4 database always
returned 1.

Is this behaviour intended for some reason, or a bug?

Til

#2Bruce Momjian
bruce@momjian.us
In reply to: Tilmann Singer (#1)
Re: [GENERAL] pg_migrator not setting values of sequences?

Tilmann Singer wrote:

I tried the latest pg_migrator
(http://pgfoundry.org/frs/download.php/2291/pg_migrator-8.4.tgz) to
migrate a database from an 8.3.7 to an 8.4.0 cluster. It finished with
a success message, and the schema and all tables seem to have been
migrated correctly.

However, all of the sequences were at the initial values and not
bumped up to the last used value as I would have expected. The first
nextval call on any sequence in the migrated 8.4 database always
returned 1.

Is this behaviour intended for some reason, or a bug?

[ CC to hackers.]

Wow, that is also surprising. I am going to have to run some tests to
find the cause, but it certainly is not intended.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#3Bruce Momjian
bruce@momjian.us
In reply to: Tilmann Singer (#1)
Re: pg_migrator not setting values of sequences?

FYI, for general email readers, I am sending pg_migrator reports to
hackers instead of having them be dealt with on the 'general' list.

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

Tilmann Singer wrote:

I tried the latest pg_migrator
(http://pgfoundry.org/frs/download.php/2291/pg_migrator-8.4.tgz) to
migrate a database from an 8.3.7 to an 8.4.0 cluster. It finished with
a success message, and the schema and all tables seem to have been
migrated correctly.

However, all of the sequences were at the initial values and not
bumped up to the last used value as I would have expected. The first
nextval call on any sequence in the migrated 8.4 database always
returned 1.

Is this behaviour intended for some reason, or a bug?

Til

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: [GENERAL] pg_migrator not setting values of sequences?

Bruce Momjian <bruce@momjian.us> writes:

Tilmann Singer wrote:

However, all of the sequences were at the initial values and not
bumped up to the last used value as I would have expected. The first
nextval call on any sequence in the migrated 8.4 database always
returned 1.

Wow, that is also surprising. I am going to have to run some tests to
find the cause, but it certainly is not intended.

Looks like pg_migrator neglects to include relkind 'S' in the set of
tables that it needs to physically migrate.

regards, tom lane

#5Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
1 attachment(s)
Re: [GENERAL] pg_migrator not setting values of sequences?

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tilmann Singer wrote:

However, all of the sequences were at the initial values and not
bumped up to the last used value as I would have expected. The first
nextval call on any sequence in the migrated 8.4 database always
returned 1.

Wow, that is also surprising. I am going to have to run some tests to
find the cause, but it certainly is not intended.

Looks like pg_migrator neglects to include relkind 'S' in the set of
tables that it needs to physically migrate.

Thanks, I have fixed pg_migrator with the attached patch. Once we find
the cause of the lovacuum problem, I will make a new pg_migrator release.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachments:

/rtmp/difftext/x-diffDownload
Index: info.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/info.c,v
retrieving revision 1.17
retrieving revision 1.18
diff -c -c -r1.17 -r1.18
*** info.c	30 Jun 2009 22:01:12 -0000	1.17
--- info.c	14 Jul 2009 02:34:59 -0000	1.18
***************
*** 343,349 ****
  							STRINGIFY(FirstNormalObjectId) " "
  							"	AND "
  							"	(relkind = 'r' OR relkind = 't' OR "
! 							"	 relkind = 'i') "
  							"GROUP BY  c.oid, n.nspname, c.relname, c.relfilenode,"
  							"			c.reltoastrelid, t.spclocation, "
  							"			n.nspname "
--- 343,349 ----
  							STRINGIFY(FirstNormalObjectId) " "
  							"	AND "
  							"	(relkind = 'r' OR relkind = 't' OR "
! 							"	 relkind = 'i' OR relkind = 'S') "
  							"GROUP BY  c.oid, n.nspname, c.relname, c.relfilenode,"
  							"			c.reltoastrelid, t.spclocation, "
  							"			n.nspname "
#6Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#5)
Re: [GENERAL] pg_migrator not setting values of sequences?

bruce wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tilmann Singer wrote:

However, all of the sequences were at the initial values and not
bumped up to the last used value as I would have expected. The first
nextval call on any sequence in the migrated 8.4 database always
returned 1.

Wow, that is also surprising. I am going to have to run some tests to
find the cause, but it certainly is not intended.

Looks like pg_migrator neglects to include relkind 'S' in the set of
tables that it needs to physically migrate.

Thanks, I have fixed pg_migrator with the attached patch. Once we find
the cause of the lovacuum problem, I will make a new pg_migrator release.

The patch I posted definately fixes a bug. Not sure how I missed the
regression changes caused by not migrating sequences; I thought it was
an issue with cached sequence values, not an actual bug.

However, I with the patch, I am now seeing another difference; a
database with:

SELECT pg_catalog.setval('clstr_tst_a_seq', 33, true);

becomes:

SELECT pg_catalog.setval('clstr_tst_a_seq', 33, false);

and the is_called column of the migrated sequences is NULL:

regression=> \d check_seq
Sequence "public.check_seq"
Column | Type | Value
---------------+---------+---------------------
sequence_name | name | check_seq
last_value | bigint | 1
start_value | bigint | 1
increment_by | bigint | 9223372036854775807
max_value | bigint | 1
min_value | bigint | 1
cache_value | bigint | 1
log_cnt | bigint | 25387551686656
is_cycled | boolean | f
is_called | boolean |

regression=> select * from check_seq where is_called is null;
sequence_name | last_value | start_value | increment_by |
max_value | min_value | cache_value | log_cnt | is_cycled |
is_called
---------------+------------+-------------+---------------------+-----------+-----------+-------------+----------------+-----------+-----------

check_seq | 1 | 1 | 9223372036854775807 |
1 | 1 | 1 | 25387551686656 | f |
(1 row)

Something is certainly wrong. Did we change sequence table format from
8.3 to 8.4?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#6)
Re: [GENERAL] pg_migrator not setting values of sequences?

Bruce Momjian <bruce@momjian.us> writes:

Did we change sequence table format from
8.3 to 8.4?

Oh, yes we did: we added a start_value column. So this is going to take
more work than that :-(

regards, tom lane

#8Alvaro Herrera
alvherre@commandprompt.com
In reply to: Bruce Momjian (#6)
Re: [GENERAL] pg_migrator not setting values of sequences?

Bruce Momjian wrote:

Something is certainly wrong. Did we change sequence table format from
8.3 to 8.4?

8.3 does not have start_value.

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

#9Zdenek Kotala
Zdenek.Kotala@Sun.COM
In reply to: Bruce Momjian (#5)
Re: [GENERAL] pg_migrator not setting values of sequences?

Dne 14.07.09 04:37, Bruce Momjian napsal(a):

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tilmann Singer wrote:

However, all of the sequences were at the initial values and not
bumped up to the last used value as I would have expected. The first
nextval call on any sequence in the migrated 8.4 database always
returned 1.

Wow, that is also surprising. I am going to have to run some tests to
find the cause, but it certainly is not intended.

Looks like pg_migrator neglects to include relkind 'S' in the set of
tables that it needs to physically migrate.

Thanks, I have fixed pg_migrator with the attached patch. Once we find
the cause of the lovacuum problem, I will make a new pg_migrator release.

I think you don't need transfer sequence table, because pg_dump should
create DDL command (CREATE SEQUENCE) which should create new nice
sequence relation.

Zdenek

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zdenek Kotala (#9)
Re: [GENERAL] pg_migrator not setting values of sequences?

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:

I think you don't need transfer sequence table, because pg_dump should
create DDL command (CREATE SEQUENCE) which should create new nice
sequence relation.

Yeah, but pg_dump is doing a schema-only dump so it doesn't transfer the
current state of the sequence.

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

regards, tom lane

#11Alvaro Herrera
alvherre@commandprompt.com
In reply to: Tom Lane (#10)
Re: [GENERAL] pg_migrator not setting values of sequences?

Tom Lane wrote:

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:

I think you don't need transfer sequence table, because pg_dump should
create DDL command (CREATE SEQUENCE) which should create new nice
sequence relation.

Yeah, but pg_dump is doing a schema-only dump so it doesn't transfer the
current state of the sequence.

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

It seems cleaner to have a pg_dump --dump-all-sequences or some such.

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

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#11)
Re: [GENERAL] pg_migrator not setting values of sequences?

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

It seems cleaner to have a pg_dump --dump-all-sequences or some such.

Well, that's assuming that we think there's any point in having a
"clean" definition.

I have been poking at the pg_largeobject problem previously mentioned,
and have found out that there are actually two bugs:
* pg_largeobject_loid_pn_index is not transferred
* large object comments are not transferred

The first of these is clearly pg_migrator's responsibility to fix,
but I think we have to get pg_dump to handle the second one. Again,
the problem here is that the dividing line between "schema" and "data"
isn't drawn in a place that suits pg_migrator's needs --- pg_dump
thinks that both LOs and their comments are "data".

Do you really want to propose that we invent, and document, two new
switches to expose these behaviors? I think just hacking the behavior
on the basis of --binary-upgrade is the thing to do. In fact, I'm
thinking that we should remove the --schema-only switch from
pg_migrator's call of pg_dump, and just have --binary-upgrade
automatically know which things it is supposed to dump or not.

[ pokes at it some more... ] Oooh, there's another issue:
the backend rejects COMMENT ON LARGE OBJECT if the specified OID
doesn't exist in pg_largeobject. This is gonna be a problem.
pg_migrator wants to import the pg_dump output before it's moved
any tables.

I wonder if it's sane to do the physical move of pg_largeobject
before we import the dump?

regards, tom lane

#13Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#12)
Re: [GENERAL] pg_migrator not setting values of sequences?

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

It seems cleaner to have a pg_dump --dump-all-sequences or some such.

Well, that's assuming that we think there's any point in having a
"clean" definition.

I have been poking at the pg_largeobject problem previously mentioned,
and have found out that there are actually two bugs:
* pg_largeobject_loid_pn_index is not transferred
* large object comments are not transferred

Nice catch, thanks.

The first of these is clearly pg_migrator's responsibility to fix,
but I think we have to get pg_dump to handle the second one. Again,
the problem here is that the dividing line between "schema" and "data"
isn't drawn in a place that suits pg_migrator's needs --- pg_dump
thinks that both LOs and their comments are "data".

Do you really want to propose that we invent, and document, two new
switches to expose these behaviors? I think just hacking the behavior
on the basis of --binary-upgrade is the thing to do. In fact, I'm
thinking that we should remove the --schema-only switch from
pg_migrator's call of pg_dump, and just have --binary-upgrade
automatically know which things it is supposed to dump or not.

Agreed.

[ pokes at it some more... ] Oooh, there's another issue:
the backend rejects COMMENT ON LARGE OBJECT if the specified OID
doesn't exist in pg_largeobject. This is gonna be a problem.
pg_migrator wants to import the pg_dump output before it's moved
any tables.

I wonder if it's sane to do the physical move of pg_largeobject
before we import the dump?

Not sure.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#14Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#10)
Re: [GENERAL] pg_migrator not setting values of sequences?

Tom Lane wrote:

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:

I think you don't need transfer sequence table, because pg_dump should
create DDL command (CREATE SEQUENCE) which should create new nice
sequence relation.

Yeah, but pg_dump is doing a schema-only dump so it doesn't transfer the
current state of the sequence.

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

Uh, how is this going to behave in 8.5? Do we still dump sequences, and
if so, aren't we heading down the road of dumping stuff only because a
previous release needed it?

Can we run a query that just shifts columns around in the sequence heap
files we migrated?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#15Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#12)
Re: [GENERAL] pg_migrator not setting values of sequences?

A larger question is what do we do with pg_migrator now. I am
embarrassed I didn't find these errors before, but now that they are
known, and will probably need an 8.4.1 to fix, should I remove the
pg_migrator 8.4 source code from pgfoundry?

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

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Tom Lane wrote:

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

It seems cleaner to have a pg_dump --dump-all-sequences or some such.

Well, that's assuming that we think there's any point in having a
"clean" definition.

I have been poking at the pg_largeobject problem previously mentioned,
and have found out that there are actually two bugs:
* pg_largeobject_loid_pn_index is not transferred
* large object comments are not transferred

The first of these is clearly pg_migrator's responsibility to fix,
but I think we have to get pg_dump to handle the second one. Again,
the problem here is that the dividing line between "schema" and "data"
isn't drawn in a place that suits pg_migrator's needs --- pg_dump
thinks that both LOs and their comments are "data".

Do you really want to propose that we invent, and document, two new
switches to expose these behaviors? I think just hacking the behavior
on the basis of --binary-upgrade is the thing to do. In fact, I'm
thinking that we should remove the --schema-only switch from
pg_migrator's call of pg_dump, and just have --binary-upgrade
automatically know which things it is supposed to dump or not.

[ pokes at it some more... ] Oooh, there's another issue:
the backend rejects COMMENT ON LARGE OBJECT if the specified OID
doesn't exist in pg_largeobject. This is gonna be a problem.
pg_migrator wants to import the pg_dump output before it's moved
any tables.

I wonder if it's sane to do the physical move of pg_largeobject
before we import the dump?

regards, tom lane

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#16Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#15)
Re: [GENERAL] pg_migrator not setting values of sequences?

Bruce Momjian wrote:

A larger question is what do we do with pg_migrator now. I am
embarrassed I didn't find these errors before, but now that they are
known, and will probably need an 8.4.1 to fix, should I remove the
pg_migrator 8.4 source code from pgfoundry?

FYI, I am at EnterpriseDB for meetings, which is why I have not replied
as promptly as usual.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#14)
Re: [GENERAL] pg_migrator not setting values of sequences?

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

Uh, how is this going to behave in 8.5? Do we still dump sequences, and
if so, aren't we heading down the road of dumping stuff only because a
previous release needed it?

In 8.5 we'll probably have it go over to treating sequences the same as
other user tables. What, do you think that'll be the only change
required in pg_migrator's behavior between 8.4 and 8.5? I think it'll
more likely be down in the noise ...

Can we run a query that just shifts columns around in the sequence heap
files we migrated?

Nope. That's not exposed at the SQL level, even if we allowed ALTER
TABLE on sequences (which I sure hope we don't).

regards, tom lane

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#15)
Re: [GENERAL] pg_migrator not setting values of sequences?

Bruce Momjian <bruce@momjian.us> writes:

A larger question is what do we do with pg_migrator now. I am
embarrassed I didn't find these errors before, but now that they are
known, and will probably need an 8.4.1 to fix, should I remove the
pg_migrator 8.4 source code from pgfoundry?

Well, we'd still like people to test it, but at the very least it
should be marked as having known bugs.

regards, tom lane

#19Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#17)
Re: [GENERAL] pg_migrator not setting values of sequences?

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

The most effective solution might be to revert the change in pg_migrator
and instead have pg_dump interpret --binary-upgrade --schema-only to
include the data for sequences. It seems ugly as sin though :-(

Uh, how is this going to behave in 8.5? Do we still dump sequences, and
if so, aren't we heading down the road of dumping stuff only because a
previous release needed it?

In 8.5 we'll probably have it go over to treating sequences the same as
other user tables. What, do you think that'll be the only change
required in pg_migrator's behavior between 8.4 and 8.5? I think it'll
more likely be down in the noise ...

I am just worried about jerking pg_dump around as pg_migrator's needs
change.

Can we run a query that just shifts columns around in the sequence heap
files we migrated?

Nope. That's not exposed at the SQL level, even if we allowed ALTER
TABLE on sequences (which I sure hope we don't).

Ah, I see what you mean:

test=> create sequence xx;
seCREATE SEQUENCE
test=> select * from xx;
sequence_name | last_value | start_value | increment_by |
max_value | min_value | cache_value | log_cnt | is_cycled |
is_called
---------------+------------+-------------+--------------+---------------------+-----------+-------------+---------+-----------+-----------

xx | 1 | 1 | 1 |
9223372036854775807 | 1 | 1 | 1 | f | f
(1 row)

test=> update xx set last_value = 3;
ERROR: cannot change sequence "xx"

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#20Dimitri Fontaine
dfontaine@hi-media.com
In reply to: Tom Lane (#18)
Re: [GENERAL] pg_migrator not setting values of sequences?

Tom Lane <tgl@sss.pgh.pa.us> writes:

Bruce Momjian <bruce@momjian.us> writes:

A larger question is what do we do with pg_migrator now. I am
embarrassed I didn't find these errors before, but now that they are
known, and will probably need an 8.4.1 to fix, should I remove the
pg_migrator 8.4 source code from pgfoundry?

Well, we'd still like people to test it, but at the very least it
should be marked as having known bugs.

What about having pg_migrator bails out if destination cluster version
is < 80401 ?

Regards,
--
dim

#21Bruce Momjian
bruce@momjian.us
In reply to: Dimitri Fontaine (#20)
Re: [GENERAL] pg_migrator not setting values of sequences?

Dimitri Fontaine wrote:

Tom Lane <tgl@sss.pgh.pa.us> writes:

Bruce Momjian <bruce@momjian.us> writes:

A larger question is what do we do with pg_migrator now. I am
embarrassed I didn't find these errors before, but now that they are
known, and will probably need an 8.4.1 to fix, should I remove the
pg_migrator 8.4 source code from pgfoundry?

Well, we'd still like people to test it, but at the very least it
should be marked as having known bugs.

What about having pg_migrator bails out if destination cluster version
is < 80401 ?

We can do that, but because 8.4.1 is not released yet, we might as well
just remove the source code.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#22Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#14)
Re: [GENERAL] pg_migrator not setting values of sequences?

On Thursday 16 July 2009 07:09:22 Bruce Momjian wrote:

Uh, how is this going to behave in 8.5? Do we still dump sequences, and
if so, aren't we heading down the road of dumping stuff only because a
previous release needed it?

Which leads me to a related question: Do you plan to maintain one version of
pg_migrator that can upgrade any version to any other version (within reason),
or will there be separate binaries, say pg_migrator-8.4 and pg_migrator-8.5,
that each can only upgrade from $selfversion-1 to $selfversion?

#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#22)
Re: [GENERAL] pg_migrator not setting values of sequences?

Peter Eisentraut <peter_e@gmx.net> writes:

Which leads me to a related question: Do you plan to maintain one
version of pg_migrator that can upgrade any version to any other
version (within reason), or will there be separate binaries, say
pg_migrator-8.4 and pg_migrator-8.5, that each can only upgrade from
$selfversion-1 to $selfversion?

I think we should plan on the latter, at least until we have a few
versions under our belts and can see what we might be letting ourselves
in for. My guess is that n-1 to n will be plenty challenging enough.

regards, tom lane

#24Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#22)
Re: [GENERAL] pg_migrator not setting values of sequences?

Peter Eisentraut wrote:

On Thursday 16 July 2009 07:09:22 Bruce Momjian wrote:

Uh, how is this going to behave in 8.5? Do we still dump sequences, and
if so, aren't we heading down the road of dumping stuff only because a
previous release needed it?

Which leads me to a related question: Do you plan to maintain one version of
pg_migrator that can upgrade any version to any other version (within reason),
or will there be separate binaries, say pg_migrator-8.4 and pg_migrator-8.5,
that each can only upgrade from $selfversion-1 to $selfversion?

One binary/source tree.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#25Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#8)
Re: [GENERAL] pg_migrator not setting values of sequences?

Alvaro Herrera wrote:

Bruce Momjian wrote:

Something is certainly wrong. Did we change sequence table format from
8.3 to 8.4?

8.3 does not have start_value.

Looking at an invalidly-migrated sequence's columns:

regression=> \d serialtest_f2_foo
Sequence "public.serialtest_f2_foo"
Column | Type | Value
---------------+---------+---------------------
sequence_name | name | serialtest_f2_foo
last_value | bigint | 3
start_value | bigint | 1
increment_by | bigint | 9223372036854775807
max_value | bigint | 1
min_value | bigint | 1
cache_value | bigint | 0
log_cnt | bigint | 25387551686912
is_cycled | boolean | f
is_called | boolean |

Should pg_migrator just pull the misaligned values and do an ALTER
SEQUENCE/seval() to fix it, or create a script to do that?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#26Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#25)
1 attachment(s)
Re: [GENERAL] pg_migrator not setting values of sequences?

Bruce Momjian wrote:

Alvaro Herrera wrote:

Bruce Momjian wrote:

Something is certainly wrong. Did we change sequence table format from
8.3 to 8.4?

8.3 does not have start_value.

Looking at an invalidly-migrated sequence's columns:

regression=> \d serialtest_f2_foo
Sequence "public.serialtest_f2_foo"
Column | Type | Value
---------------+---------+---------------------
sequence_name | name | serialtest_f2_foo
last_value | bigint | 3
start_value | bigint | 1
increment_by | bigint | 9223372036854775807
max_value | bigint | 1
min_value | bigint | 1
cache_value | bigint | 0
log_cnt | bigint | 25387551686912
is_cycled | boolean | f
is_called | boolean |

Should pg_migrator just pull the misaligned values and do an ALTER
SEQUENCE/seval() to fix it, or create a script to do that?

I have applied the attached patch to pg_migrator that will properly
handle migrating sequences; it should apply cleanly to pg_migrator
8.4.1 alpha 1.

What I found during research is that pg_dump --schema-only already
creates the sequence:

CREATE SEQUENCE check_seq
START WITH 1
INCREMENT BY 1
NO MAXVALUE
NO MINVALUE
CACHE 1;

What it does not do is to call setval() to set the sequence value and
'is_called'. What I did was to _not_ migrate the sequence file, but
rather create a script from the old cluster that uses setval() to set
the sequence values. This can be safely run by pg_migrator
unconditionally because we are not migrating the sequence files, even in
link mode.

This solves the sequence migration problem, with no changes to pg_dump.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachments:

/rtmp/difftext/x-diffDownload
? tools
? log
? src/pg_migrator
Index: src/info.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/info.c,v
retrieving revision 1.18
diff -c -r1.18 info.c
*** src/info.c	14 Jul 2009 02:34:59 -0000	1.18
--- src/info.c	20 Jul 2009 18:55:48 -0000
***************
*** 322,353 ****
  	int			i_oid = -1;
  	int			i_relfilenode = -1;
  	int			i_reltoastrelid = -1;
  
! 	res = executeQueryOrDie(ctx, conn,
! 							"SELECT DISTINCT c.oid, n.nspname, c.relname, "
! 							"	c.relfilenode, c.reltoastrelid, "
! 							"	t.spclocation,n.nspname "
! 							"FROM (pg_catalog.pg_class c JOIN "
! 							"		pg_catalog.pg_namespace n "
! 							"	ON c.relnamespace = n.oid) "
! 							"   LEFT OUTER JOIN pg_catalog.pg_tablespace t "
! 							"	ON c.reltablespace = t.oid "
! 							"WHERE relnamespace NOT IN "
! 							"	( "
! 							"		SELECT oid "
! 							"		FROM pg_catalog.pg_namespace "
! 							"		WHERE nspname IN "
! 							"			('pg_catalog', 'information_schema') "
! 							"	) "
! 							"	AND c.oid >= "
! 							STRINGIFY(FirstNormalObjectId) " "
! 							"	AND "
! 							"	(relkind = 'r' OR relkind = 't' OR "
! 							"	 relkind = 'i' OR relkind = 'S') "
! 							"GROUP BY  c.oid, n.nspname, c.relname, c.relfilenode,"
! 							"			c.reltoastrelid, t.spclocation, "
! 							"			n.nspname "
! 							"ORDER BY n.nspname, c.relname;");
  
  	ntups = PQntuples(res);
  
--- 322,360 ----
  	int			i_oid = -1;
  	int			i_relfilenode = -1;
  	int			i_reltoastrelid = -1;
+ 	char	    query[QUERY_ALLOC];
  
! 	snprintf(query, sizeof(query),
! 				"SELECT DISTINCT c.oid, n.nspname, c.relname, "
! 				"	c.relfilenode, c.reltoastrelid, "
! 				"	t.spclocation,n.nspname "
! 				"FROM (pg_catalog.pg_class c JOIN "
! 				"		pg_catalog.pg_namespace n "
! 				"	ON c.relnamespace = n.oid) "
! 				"   LEFT OUTER JOIN pg_catalog.pg_tablespace t "
! 				"	ON c.reltablespace = t.oid "
! 				"WHERE relnamespace NOT IN "
! 				"	( "
! 				"		SELECT oid "
! 				"		FROM pg_catalog.pg_namespace "
! 				"		WHERE nspname IN "
! 				"			('pg_catalog', 'information_schema') "
! 				"	) "
! 				"	AND c.oid >= "
! 				STRINGIFY(FirstNormalObjectId) " "
! 				"	AND "
! 				"	(relkind = 'r' OR relkind = 't' OR "
! 				"	 relkind = 'i'%s)"
! 				"GROUP BY  c.oid, n.nspname, c.relname, c.relfilenode,"
! 				"			c.reltoastrelid, t.spclocation, "
! 				"			n.nspname "
! 				"ORDER BY n.nspname, c.relname;",
! 				/* see the comment at the top of v8_3_adjust_sequences() */
! 				(GET_MAJOR_VERSION(ctx->old.pg_version) == 803 &&
! 				 GET_MAJOR_VERSION(ctx->new.pg_version) > 803) ?
! 					"" : " OR relkind = 'S'");
! 
! 	res = executeQueryOrDie(ctx, conn, query);
  
  	ntups = PQntuples(res);
  
Index: src/pg_migrator.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/pg_migrator.c,v
retrieving revision 1.56
diff -c -r1.56 pg_migrator.c
*** src/pg_migrator.c	3 Jul 2009 16:46:49 -0000	1.56
--- src/pg_migrator.c	20 Jul 2009 18:55:48 -0000
***************
*** 37,43 ****
  main(int argc, char **argv)
  {
  	migratorContext ctx;
! 
  	memset(&ctx, 0, sizeof(ctx));
  
  	parseCommandLine(&ctx, argc, argv);
--- 37,44 ----
  main(int argc, char **argv)
  {
  	migratorContext ctx;
! 	char *sequence_script_file_name = NULL;
! 	
  	memset(&ctx, 0, sizeof(ctx));
  
  	parseCommandLine(&ctx, argc, argv);
***************
*** 78,83 ****
--- 79,92 ----
  			v8_3_invalidate_hash_gin_indexes(&ctx, true, CLUSTER_OLD);
  			v8_3_invalidate_bpchar_pattern_ops_indexes(&ctx, true, CLUSTER_OLD);
  		}
+ 		else
+ 			/*
+ 			 *	While we have the old server running, create the script
+ 			 *	to properly restore its sequence values but we report this
+ 			 *	at the end.
+ 			 */
+ 			sequence_script_file_name =
+ 				v8_3_create_sequence_script(&ctx, CLUSTER_OLD);
  	}
  
  	/* Looks okay so far.  Prepare the pg_dump output */
***************
*** 245,250 ****
--- 254,273 ----
  		GET_MAJOR_VERSION(ctx.new.pg_version) > 803)
  	{
  		start_postmaster(&ctx, CLUSTER_NEW, true);
+ 		/* restore proper sequence values using file created from old server */
+ 		if (strlen(sequence_script_file_name) > 0)
+ 		{
+ 			prep_status(&ctx, "Adjusting sequences");
+ 			exec_prog(&ctx, true,
+ 					  SYSTEMQUOTE "\"%s/%s\" --set ON_ERROR_STOP=on --port %d "
+ 					  "-f \"%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
+ 					  ctx.new.bindir, ctx.new_psql_exe, ctx.new.port,
+ 					  sequence_script_file_name, ctx.logfile);
+ 			unlink(sequence_script_file_name);
+ 			check_ok(&ctx);
+ 		}
+ 		pg_free(sequence_script_file_name);
+ 
  		v8_3_rebuild_tsvector_tables(&ctx, false, CLUSTER_NEW);
  		v8_3_invalidate_hash_gin_indexes(&ctx, false, CLUSTER_NEW);
  		v8_3_invalidate_bpchar_pattern_ops_indexes(&ctx, false, CLUSTER_NEW);
Index: src/pg_migrator.h
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/pg_migrator.h,v
retrieving revision 1.63
diff -c -r1.63 pg_migrator.h
*** src/pg_migrator.h	18 Jul 2009 00:14:01 -0000	1.63
--- src/pg_migrator.h	20 Jul 2009 18:55:48 -0000
***************
*** 26,31 ****
--- 26,32 ----
  
  #define MAX_STRING		1024
  #define LINE_ALLOC		4096
+ #define QUERY_ALLOC		8192
    
  #define MIGRATOR_API_VERSION	1
  
***************
*** 390,392 ****
--- 391,396 ----
  							bool check_mode, Cluster whichCluster);
  void		v8_3_invalidate_bpchar_pattern_ops_indexes(migratorContext *ctx,
  							bool check_mode, Cluster whichCluster);
+ char 		*v8_3_create_sequence_script(migratorContext *ctx,
+ 							Cluster whichCluster);
+ 
Index: src/version.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/version.c,v
retrieving revision 1.22
diff -c -r1.22 version.c
*** src/version.c	2 Jul 2009 23:22:53 -0000	1.22
--- src/version.c	20 Jul 2009 18:55:48 -0000
***************
*** 417,423 ****
  					"| when executed by psql by the database super-user, will rebuild\n"
  					"| all tables with tsvector columns.\n\n",
  					output_path);
- 					
  	}
  	else
  		check_ok(ctx);
--- 417,422 ----
***************
*** 528,534 ****
  					"| when executed by psql by the database super-user, will recreate\n"
  					"| all invalid indexes; until then, none of these indexes will be used.\n\n",
  					output_path);
- 					
  	}
  	else
  		check_ok(ctx);
--- 527,532 ----
***************
*** 657,659 ****
--- 655,756 ----
  	else
  		check_ok(ctx);
  }
+ 
+ 
+ /*
+  * v8_3_create_sequence_script()
+  *
+  *	8.4 added the column "start_value" to all sequences.  For this reason,
+  *	we don't transfer sequence files but instead use the CREATE SEQUENCE
+  *	command from the schema dump, and use setval() to restore the sequence
+  *	value and 'is_called' from the old database.  This is safe to run
+  *	by pg_migrator because sequence files are not transfered from the old
+  *	server, even in link mode.
+  */
+ char *
+ v8_3_create_sequence_script(migratorContext *ctx, Cluster whichCluster)
+ {
+ 	ClusterInfo	*active_cluster = (whichCluster == CLUSTER_OLD) ?
+ 					&ctx->old : &ctx->new;
+ 	int			dbnum;
+ 	FILE		*script = NULL;
+ 	bool		found = false;
+ 	char		*output_path = pg_malloc(ctx, MAXPGPATH);
+ 
+ 	snprintf(output_path, MAXPGPATH, "%s/adjust_sequences.txt", ctx->home_dir);
+ 
+ 	prep_status(ctx, "Creating script to adjust sequences");
+ 
+ 	for (dbnum = 0; dbnum < active_cluster->dbarr.ndbs; dbnum++)
+ 	{
+ 		PGresult   *res;
+  		bool		db_used = false;
+ 		int			ntups;
+ 		int			rowno;
+ 		int			i_nspname, i_relname;
+ 		DbInfo	   *active_db = &active_cluster->dbarr.dbs[dbnum];
+ 		PGconn	   *conn = connectToServer(ctx, active_db->db_name, whichCluster);
+ 		
+ 		/* Find any sequences */
+ 		res = executeQueryOrDie(ctx, conn,
+ 								"SELECT n.nspname, c.relname "
+ 								"FROM	pg_catalog.pg_class c, "
+ 								"		pg_catalog.pg_namespace n "
+ 								"WHERE	c.relkind = 'S' AND "
+ 								"		c.relnamespace = n.oid AND "
+ 								"		n.nspname != 'pg_catalog' AND "
+ 								"		n.nspname != 'information_schema'");
+ 
+ 		ntups = PQntuples(res);
+ 		i_nspname = PQfnumber(res, "nspname");
+ 		i_relname = PQfnumber(res, "relname");
+ 		for (rowno = 0; rowno < ntups; rowno++)
+ 		{
+ 			PGresult   *seq_res;
+ 			int			i_last_value, i_is_called;
+ 			const char *nspname = PQgetvalue(res, rowno, i_nspname);
+ 			const char *relname = PQgetvalue(res, rowno, i_relname);
+ 
+ 			found = true;
+ 
+ 			if (script == NULL && (script = fopen(output_path, "w")) == NULL)
+ 					pg_log(ctx, PG_FATAL, "Could not create necessary file:  %s\n", output_path);
+ 			if (!db_used)
+ 			{
+ 				fprintf(script, "\\connect %s\n\n",
+ 						quote_identifier(ctx, active_db->db_name));
+ 				db_used = true;
+ 			}
+ 
+ 			/* Find the desired sequence */
+ 			seq_res = executeQueryOrDie(ctx, conn,
+ 									"SELECT s.last_value, s.is_called "
+ 									"FROM	%s.%s s",
+ 									quote_identifier(ctx, nspname),
+ 									quote_identifier(ctx, relname));
+ 
+ 			assert(PQntuples(seq_res) == 1);
+ 			i_last_value = PQfnumber(seq_res, "last_value");
+ 			i_is_called = PQfnumber(seq_res, "is_called");
+ 
+ 			fprintf(script, "SELECT setval('%s.%s', %s, '%s');\n",
+ 					quote_identifier(ctx, nspname), quote_identifier(ctx, relname),
+ 					PQgetvalue(seq_res, 0, i_last_value), PQgetvalue(seq_res, 0, i_is_called));
+ 			PQclear(seq_res);
+ 		}
+ 		if (db_used)
+ 			fprintf(script, "\n");
+ 
+ 		PQclear(res);
+ 
+ 		PQfinish(conn);
+ 	}
+ 	if (found)
+ 		fclose(script);
+ 	else	/* mark script as unused */
+ 		output_path[0] = '\0';
+ 		
+ 	check_ok(ctx);
+ 
+ 	return output_path;
+ }