Thread: Re: [GENERAL] pg_migrator not setting values of sequences?

Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Tom Lane
Date:
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


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +
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 "

Re: [GENERAL] pg_migrator not setting values of sequences?

From
Zdenek Kotala
Date:
Dne 14.07.09 04:37, Bruce Momjian napsal(a): <blockquote cite="mid:200907140237.n6E2b2x27210@momjian.us"
type="cite"><prewrap="">Tom Lane wrote: </pre><blockquote type="cite"><pre wrap="">Bruce Momjian <a
class="moz-txt-link-rfc2396E"href="mailto:bruce@momjian.us"><bruce@momjian.us></a> writes:   </pre><blockquote
type="cite"><prewrap="">Tilmann Singer wrote:     </pre><blockquote type="cite"><pre wrap="">However, all of the
sequenceswere 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.       </pre></blockquote></blockquote><blockquote type="cite"><pre wrap="">Wow, that is also surprising.  I
amgoing to have to run some tests to
 
find the cause, but it certainly is not intended.     </pre></blockquote><pre wrap="">Looks like pg_migrator neglects
toinclude relkind 'S' in the set of
 
tables that it needs to physically migrate.   </pre></blockquote><pre wrap="">
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.
 </pre></blockquote> 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.<br /><br />        Zdenek<br /> 

Re: [GENERAL] pg_migrator not setting values of sequences?

From
Tom Lane
Date:
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


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Alvaro Herrera
Date:
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.


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Tom Lane
Date:
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
commentsare 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


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Tom Lane
Date:
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


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Tom Lane
Date:
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


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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 SEQUENCEtest=> 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. +


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Dimitri Fontaine
Date:
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


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Peter Eisentraut
Date:
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?


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Tom Lane
Date:
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


Re: [GENERAL] pg_migrator not setting values of sequences?

From
Bruce Momjian
Date:
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. +