Thread: pg_upgrade

pg_upgrade

From
Brian Hirt
Date:
I'm testing pg_upgrade out and ran into a couple of problems.   First when I did pg_upgrade --check I got the tsearch2
tablespreventing the upgrade from happening: 

Database:  testdatabase
  public.pg_ts_dict.dict_init
  public.pg_ts_dict.dict_lexize
  public.pg_ts_parser.prs_start
  public.pg_ts_parser.prs_nexttoken
  public.pg_ts_parser.prs_end
  public.pg_ts_parser.prs_headline
  public.pg_ts_parser.prs_lextype

For testing, at this point I really didn't care about tsearch, so I simply dropped those tables so I could revisit them
later-- however, I'm confused about these tables in general, both pg_catalog.pg_ts_parser and public.pg_ts_parser exist
withdifferent, albeit similar, schemas.   I think that the table in public is no longer used and was a remnant from
pre-8.3when tsearch2 wasn't part of the distribution, can anyone confirm this? 

Anyway, after removing the tsearch tables, I did pg_upgrade --check again and it said the clusters were compatible. I
proceededto run the upgrade command and it bombed out in the "Restoring user relation files" section.    I've included
someoutput below, any advice on what is going on?  It seems something is messed up in either the check logic or actual
migrationcode. 

root@ubuntu:~# /usr/pg-8.4/bin/oid2name
All databases:
         Oid      Database Name  Tablespace
-------------------------------------------
    ...
       11564           postgres  pg_default
    ...
root@ubuntu:~# /usr/pg-8.4/bin/oid2name -o 2683
From database "postgres":
  Filenode                    Table Name
----------------------------------------
      2683  pg_largeobject_loid_pn_index


postgres@ubuntu:~$ /usr/pg-9.0/bin/pg_upgrade
Performing Consistency Checks
-----------------------------
Checking old data directory (/moby/pgdb-8.4)                ok
Checking old bin directory (/usr/pg-8.4/bin)                ok
Checking new data directory (/moby/pgdb-9.0)                ok
Checking new bin directory (/usr/pg-9.0/bin)                ok
Checking for reg* system oid user data types                ok
Checking for /contrib/isn with bigint-passing mismatch      ok
Checking for large objects                                  ok
Creating catalog dump                                       ok
Checking for presence of required libraries                 ok

| If pg_upgrade fails after this point, you must
| re-initdb the new cluster before continuing.
| You will also need to remove the ".old" suffix
| from /moby/pgdb-8.4/global/pg_control.old.

Performing Migration
--------------------
Adding ".old" suffix to old global/pg_control               ok
Analyzing all rows in the new cluster                       ok
Freezing all rows on the new cluster                        ok
Deleting new commit clogs                                   ok
Copying old commit clogs to new server                      ok
Setting next transaction id for new cluster                 ok
Resetting WAL archives                                      ok
Setting frozenxid counters in new cluster                   ok
Creating databases in the new cluster                       ok
Adding support functions to new cluster                     ok
Restoring database schema to new cluster                    ok
Removing support functions from new cluster                 ok
Restoring user relation files
  /moby/pgdb-8.4/base/11564/2683
Could not find pg_toast.pg_toast_2147483647 in new cluster


Re: pg_upgrade

From
Tom Lane
Date:
Brian Hirt <bhirt@me.com> writes:
> I'm testing pg_upgrade out and ran into a couple of problems.   First when I did pg_upgrade --check I got the
tsearch2tables preventing the upgrade from happening: 
> Database:  testdatabase
>   public.pg_ts_dict.dict_init
>   public.pg_ts_dict.dict_lexize
>   public.pg_ts_parser.prs_start
>   public.pg_ts_parser.prs_nexttoken
>   public.pg_ts_parser.prs_end
>   public.pg_ts_parser.prs_headline
>   public.pg_ts_parser.prs_lextype

> For testing, at this point I really didn't care about tsearch, so I simply dropped those tables so I could revisit
themlater -- however, I'm confused about these tables in general, both pg_catalog.pg_ts_parser and public.pg_ts_parser
existwith different, albeit similar, schemas.   I think that the table in public is no longer used and was a remnant
frompre-8.3 when tsearch2 wasn't part of the distribution, can anyone confirm this? 

Correct, you should just drop the ones that aren't in pg_catalog.


> Anyway, after removing the tsearch tables, I did pg_upgrade --check again and it said the clusters were compatible. I
proceededto run the upgrade command and it bombed out in the "Restoring user relation files" section. 

That sure looks like a bug, but there's not enough info here to
diagnose.  Is there actually a pg_toast.pg_toast_2147483647 table
in the 8.4 cluster?  (I'm betting not.)  Could you try extracting
a test case?  I wonder whether "pg_dump -s" from the 8.4 database,
loaded into a fresh 8.4 database, would be enough to reproduce.

            regards, tom lane

Re: pg_upgrade

From
Brian Hirt
Date:
Looks like pg_upgrade is using 32bit oids.  2147483647 is the max signed 32 bit int, but the oids for my tables are
clearlylarger than that.  

== output from pg_upgrade ==
Database: basement84_dev
relname: mit.company: reloid: 2147483647 reltblspace:
relname: mit.company_history: reloid: 2147483647 reltblspace:

== output from catalog query ==
basement84_dev=# select c.oid,c.relname from pg_catalog.pg_namespace n, pg_catalog.pg_class c where n.oid =
c.relnamespaceand n.nspname = 'mit'; 
    oid     |      relname
------------+--------------------
 3000767630 | company
 3000767633 | company_history
(22 rows)


On Sep 28, 2010, at 10:51 AM, Tom Lane wrote:

> Brian Hirt <bhirt@me.com> writes:
>> I'm testing pg_upgrade out and ran into a couple of problems.   First when I did pg_upgrade --check I got the
tsearch2tables preventing the upgrade from happening: 
>> Database:  testdatabase
>>  public.pg_ts_dict.dict_init
>>  public.pg_ts_dict.dict_lexize
>>  public.pg_ts_parser.prs_start
>>  public.pg_ts_parser.prs_nexttoken
>>  public.pg_ts_parser.prs_end
>>  public.pg_ts_parser.prs_headline
>>  public.pg_ts_parser.prs_lextype
>
>> For testing, at this point I really didn't care about tsearch, so I simply dropped those tables so I could revisit
themlater -- however, I'm confused about these tables in general, both pg_catalog.pg_ts_parser and public.pg_ts_parser
existwith different, albeit similar, schemas.   I think that the table in public is no longer used and was a remnant
frompre-8.3 when tsearch2 wasn't part of the distribution, can anyone confirm this? 
>
> Correct, you should just drop the ones that aren't in pg_catalog.
>
>
>> Anyway, after removing the tsearch tables, I did pg_upgrade --check again and it said the clusters were compatible.
Iproceeded to run the upgrade command and it bombed out in the "Restoring user relation files" section. 
>
> That sure looks like a bug, but there's not enough info here to
> diagnose.  Is there actually a pg_toast.pg_toast_2147483647 table
> in the 8.4 cluster?  (I'm betting not.)  Could you try extracting
> a test case?  I wonder whether "pg_dump -s" from the 8.4 database,
> loaded into a fresh 8.4 database, would be enough to reproduce.
>
>             regards, tom lane
>
> --
> Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-general


Re: pg_upgrade

From
Bruce Momjian
Date:
Brian Hirt wrote:
> Looks like pg_upgrade is using 32bit oids.  2147483647 is the max signed 32 bit int, but the oids for my tables are
clearlylarger than that.  
>
> == output from pg_upgrade ==
> Database: basement84_dev
> relname: mit.company: reloid: 2147483647 reltblspace:
> relname: mit.company_history: reloid: 2147483647 reltblspace:
>
> == output from catalog query ==
> basement84_dev=# select c.oid,c.relname from pg_catalog.pg_namespace n, pg_catalog.pg_class c where n.oid =
c.relnamespaceand n.nspname = 'mit'; 
>     oid     |      relname
> ------------+--------------------
>  3000767630 | company
>  3000767633 | company_history
> (22 rows)
>

Interesting.  Odd it would report the max 32-bit signed int.  I wonder
if it somehow is getting set to -1.  I looked briefly at the pg_upgrade
code and it appears to put all oids in unsigned ints.

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

  + It's impossible for everything to be true. +

Re: pg_upgrade

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Interesting.  Odd it would report the max 32-bit signed int.  I wonder
> if it somehow is getting set to -1.  I looked briefly at the pg_upgrade
> code and it appears to put all oids in unsigned ints.

On some platforms, that's what you'll get if you feed a value larger
than 2^31 to atoi() and related functions.  I will bet lunch that this
behavior reflects an attempt to use signed-integer input functions on
OID values.  You need to check the string conversion code itself, not
just the declared type of the result variables.

            regards, tom lane

Re: pg_upgrade

From
Brian Hirt
Date:
It looks like it's related to atol

$ cat test-atol.c
#include <stdlib.h>
#include <stdio.h>

int
main(int argc, char **argv)
{
  unsigned int test1;
  long test2;
  long long test3;
  unsigned int test4;

  test1 = (unsigned int)atol("3000767169");
  test2 = (long)atol("3000767169");
  test3 = atoll("3000767169");
  test4 = (unsigned int)atoll("3000767169");

  fprintf(stderr,"%u %ld %lld %u\n",test1,test2,test3,test4);
}

$ make test-atol
cc     test-atol.c   -o test-atol
$ ./test-atol
2147483647 2147483647 3000767169 3000767169


I think C90 and C99 specify different behaviors with atol

Is there some standard way postgresql parses integer strings?  Maybe that method should be used instead of duplicating
thefunctionality so at least the two behave consistently. 

--brian

On Sep 28, 2010, at 2:00 PM, Bruce Momjian wrote:

> Brian Hirt wrote:
>> Looks like pg_upgrade is using 32bit oids.  2147483647 is the max signed 32 bit int, but the oids for my tables are
clearlylarger than that.  
>>
>> == output from pg_upgrade ==
>> Database: basement84_dev
>> relname: mit.company: reloid: 2147483647 reltblspace:
>> relname: mit.company_history: reloid: 2147483647 reltblspace:
>>
>> == output from catalog query ==
>> basement84_dev=# select c.oid,c.relname from pg_catalog.pg_namespace n, pg_catalog.pg_class c where n.oid =
c.relnamespaceand n.nspname = 'mit'; 
>>    oid     |      relname
>> ------------+--------------------
>> 3000767630 | company
>> 3000767633 | company_history
>> (22 rows)
>>
>
> Interesting.  Odd it would report the max 32-bit signed int.  I wonder
> if it somehow is getting set to -1.  I looked briefly at the pg_upgrade
> code and it appears to put all oids in unsigned ints.
>
> --
>  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>  EnterpriseDB                             http://enterprisedb.com
>
>  + It's impossible for everything to be true. +
>
> --
> Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-general


Re: pg_upgrade

From
Bruce Momjian
Date:
Brian Hirt wrote:
> It looks like it's related to atol

Yep, I found the use of atol in the pg_upgrade code too.  Working on a
patch now.

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


> $ cat test-atol.c
> #include <stdlib.h>
> #include <stdio.h>
>
> int
> main(int argc, char **argv)
> {
>   unsigned int test1;
>   long test2;
>   long long test3;
>   unsigned int test4;
>
>   test1 = (unsigned int)atol("3000767169");
>   test2 = (long)atol("3000767169");
>   test3 = atoll("3000767169");
>   test4 = (unsigned int)atoll("3000767169");
>
>   fprintf(stderr,"%u %ld %lld %u\n",test1,test2,test3,test4);
> }
>
> $ make test-atol
> cc     test-atol.c   -o test-atol
> $ ./test-atol
> 2147483647 2147483647 3000767169 3000767169
>
>
> I think C90 and C99 specify different behaviors with atol
>
> Is there some standard way postgresql parses integer strings?  Maybe that method should be used instead of
duplicatingthe functionality so at least the two behave consistently. 
>
> --brian
>
> On Sep 28, 2010, at 2:00 PM, Bruce Momjian wrote:
>
> > Brian Hirt wrote:
> >> Looks like pg_upgrade is using 32bit oids.  2147483647 is the max signed 32 bit int, but the oids for my tables
areclearly larger than that.  
> >>
> >> == output from pg_upgrade ==
> >> Database: basement84_dev
> >> relname: mit.company: reloid: 2147483647 reltblspace:
> >> relname: mit.company_history: reloid: 2147483647 reltblspace:
> >>
> >> == output from catalog query ==
> >> basement84_dev=# select c.oid,c.relname from pg_catalog.pg_namespace n, pg_catalog.pg_class c where n.oid =
c.relnamespaceand n.nspname = 'mit'; 
> >>    oid     |      relname
> >> ------------+--------------------
> >> 3000767630 | company
> >> 3000767633 | company_history
> >> (22 rows)
> >>
> >
> > Interesting.  Odd it would report the max 32-bit signed int.  I wonder
> > if it somehow is getting set to -1.  I looked briefly at the pg_upgrade
> > code and it appears to put all oids in unsigned ints.
> >
> > --
> >  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
> >  EnterpriseDB                             http://enterprisedb.com
> >
> >  + It's impossible for everything to be true. +
> >
> > --
> > 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

  + It's impossible for everything to be true. +

Re: pg_upgrade

From
Bruce Momjian
Date:
Bruce Momjian wrote:
> Brian Hirt wrote:
> > It looks like it's related to atol
>
> Yep, I found the use of atol in the pg_upgrade code too.  Working on a
> patch now.

I have applied the attached patch to HEAD and 9.0.X.  Odd I had never
received a bug report about this before.  Good thing it didn't silently
fail, but it is designed to be very picky.

This patch will appear in the next 9.0.X release.  Thanks for the
report.

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

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/controldata.c b/contrib/pg_upgrade/controldata.c
index f36c2c1..c0fe821 100644
--- a/contrib/pg_upgrade/controldata.c
+++ b/contrib/pg_upgrade/controldata.c
@@ -155,7 +155,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: pg_resetxlog problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.ctrl_ver = (uint32) atol(p);
+            cluster->controldata.ctrl_ver = str2uint(p);
         }
         else if ((p = strstr(bufin, "Catalog version number:")) != NULL)
         {
@@ -165,7 +165,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.cat_ver = (uint32) atol(p);
+            cluster->controldata.cat_ver = str2uint(p);
         }
         else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL)
         {
@@ -175,7 +175,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.logid = (uint32) atol(p);
+            cluster->controldata.logid = str2uint(p);
             got_log_id = true;
         }
         else if ((p = strstr(bufin, "First log file segment after reset:")) != NULL)
@@ -186,7 +186,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.nxtlogseg = (uint32) atol(p);
+            cluster->controldata.nxtlogseg = str2uint(p);
             got_log_seg = true;
         }
         else if ((p = strstr(bufin, "Latest checkpoint's TimeLineID:")) != NULL)
@@ -197,7 +197,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.chkpnt_tli = (uint32) atol(p);
+            cluster->controldata.chkpnt_tli = str2uint(p);
             got_tli = true;
         }
         else if ((p = strstr(bufin, "Latest checkpoint's NextXID:")) != NULL)
@@ -211,7 +211,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             op++;                /* removing ':' char */
-            cluster->controldata.chkpnt_nxtxid = (uint32) atol(op);
+            cluster->controldata.chkpnt_nxtxid = str2uint(op);
             got_xid = true;
         }
         else if ((p = strstr(bufin, "Latest checkpoint's NextOID:")) != NULL)
@@ -222,7 +222,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.chkpnt_nxtoid = (uint32) atol(p);
+            cluster->controldata.chkpnt_nxtoid = str2uint(p);
             got_oid = true;
         }
         else if ((p = strstr(bufin, "Maximum data alignment:")) != NULL)
@@ -233,7 +233,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.align = (uint32) atol(p);
+            cluster->controldata.align = str2uint(p);
             got_align = true;
         }
         else if ((p = strstr(bufin, "Database block size:")) != NULL)
@@ -244,7 +244,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.blocksz = (uint32) atol(p);
+            cluster->controldata.blocksz = str2uint(p);
             got_blocksz = true;
         }
         else if ((p = strstr(bufin, "Blocks per segment of large relation:")) != NULL)
@@ -255,7 +255,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.largesz = (uint32) atol(p);
+            cluster->controldata.largesz = str2uint(p);
             got_largesz = true;
         }
         else if ((p = strstr(bufin, "WAL block size:")) != NULL)
@@ -266,7 +266,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.walsz = (uint32) atol(p);
+            cluster->controldata.walsz = str2uint(p);
             got_walsz = true;
         }
         else if ((p = strstr(bufin, "Bytes per WAL segment:")) != NULL)
@@ -277,7 +277,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.walseg = (uint32) atol(p);
+            cluster->controldata.walseg = str2uint(p);
             got_walseg = true;
         }
         else if ((p = strstr(bufin, "Maximum length of identifiers:")) != NULL)
@@ -288,7 +288,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.ident = (uint32) atol(p);
+            cluster->controldata.ident = str2uint(p);
             got_ident = true;
         }
         else if ((p = strstr(bufin, "Maximum columns in an index:")) != NULL)
@@ -299,7 +299,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.index = (uint32) atol(p);
+            cluster->controldata.index = str2uint(p);
             got_index = true;
         }
         else if ((p = strstr(bufin, "Maximum size of a TOAST chunk:")) != NULL)
@@ -310,7 +310,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.toast = (uint32) atol(p);
+            cluster->controldata.toast = str2uint(p);
             got_toast = true;
         }
         else if ((p = strstr(bufin, "Date/time type storage:")) != NULL)
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
index 1601d56..55489ac 100644
--- a/contrib/pg_upgrade/info.c
+++ b/contrib/pg_upgrade/info.c
@@ -242,7 +242,7 @@ get_db_infos(migratorContext *ctx, DbInfoArr *dbinfs_arr, Cluster whichCluster)

     for (tupnum = 0; tupnum < ntups; tupnum++)
     {
-        dbinfos[tupnum].db_oid = atol(PQgetvalue(res, tupnum, i_oid));
+        dbinfos[tupnum].db_oid = str2uint(PQgetvalue(res, tupnum, i_oid));

         snprintf(dbinfos[tupnum].db_name, sizeof(dbinfos[tupnum].db_name), "%s",
                  PQgetvalue(res, tupnum, i_datname));
@@ -360,7 +360,7 @@ get_rel_infos(migratorContext *ctx, const DbInfo *dbinfo,
         RelInfo    *curr = &relinfos[num_rels++];
         const char *tblspace;

-        curr->reloid = atol(PQgetvalue(res, relnum, i_oid));
+        curr->reloid = str2uint(PQgetvalue(res, relnum, i_oid));

         nspname = PQgetvalue(res, relnum, i_nspname);
         strlcpy(curr->nspname, nspname, sizeof(curr->nspname));
@@ -368,8 +368,8 @@ get_rel_infos(migratorContext *ctx, const DbInfo *dbinfo,
         relname = PQgetvalue(res, relnum, i_relname);
         strlcpy(curr->relname, relname, sizeof(curr->relname));

-        curr->relfilenode = atol(PQgetvalue(res, relnum, i_relfilenode));
-        curr->toastrelid = atol(PQgetvalue(res, relnum, i_reltoastrelid));
+        curr->relfilenode = str2uint(PQgetvalue(res, relnum, i_relfilenode));
+        curr->toastrelid = str2uint(PQgetvalue(res, relnum, i_reltoastrelid));

         tblspace = PQgetvalue(res, relnum, i_spclocation);
         /* if no table tablespace, use the database tablespace */
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index a7d46cf..296b80f 100644
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -376,6 +376,8 @@ char       *pg_strdup(migratorContext *ctx, const char *s);
 void       *pg_malloc(migratorContext *ctx, int size);
 void        pg_free(void *ptr);
 const char *getErrorText(int errNum);
+unsigned int str2uint(const char *str);
+

 /* version.c */

diff --git a/contrib/pg_upgrade/relfilenode.c b/contrib/pg_upgrade/relfilenode.c
index a69548b..dd605bb 100644
--- a/contrib/pg_upgrade/relfilenode.c
+++ b/contrib/pg_upgrade/relfilenode.c
@@ -94,9 +94,9 @@ get_pg_database_relfilenode(migratorContext *ctx, Cluster whichCluster)

     i_relfile = PQfnumber(res, "relfilenode");
     if (whichCluster == CLUSTER_OLD)
-        ctx->old.pg_database_oid = atol(PQgetvalue(res, 0, i_relfile));
+        ctx->old.pg_database_oid = str2uint(PQgetvalue(res, 0, i_relfile));
     else
-        ctx->new.pg_database_oid = atol(PQgetvalue(res, 0, i_relfile));
+        ctx->new.pg_database_oid = str2uint(PQgetvalue(res, 0, i_relfile));

     PQclear(res);
     PQfinish(conn);
diff --git a/contrib/pg_upgrade/util.c b/contrib/pg_upgrade/util.c
index b9968e9..3f3a4c7 100644
--- a/contrib/pg_upgrade/util.c
+++ b/contrib/pg_upgrade/util.c
@@ -259,3 +259,15 @@ getErrorText(int errNum)
 #endif
     return strdup(strerror(errNum));
 }
+
+
+/*
+ *    str2uint()
+ *
+ *    convert string to oid
+ */
+unsigned int
+str2uint(const char *str)
+{
+    return strtol(str, NULL, 10);
+}

Re: pg_upgrade

From
Brian Hirt
Date:
Bruce,

The applied patch has the same behavior on i686 Ubuntu 10.04.   It looks like atol() is just a macro for strtol() in
stdio.h.  I think you want strtoul() instead of strtol() 

when i change str2uint() to use strtoul() pg_upgrade completes without a problem (I still haven't tested the upgrade
database,but I expect that will be just fine). 

I think it's pretty uncommon for the OID to be that big which is why nobody stumbled onto this.   This particular
installationhas pretty much been reloading development databases non stop for the last year.  Also, people tend to
initdba lot when testing and doing development which will keep resetting the oid low. 

Thanks for getting this one fixed

--brian

On Sep 28, 2010, at 3:49 PM, Bruce Momjian wrote:
> I have applied the attached patch to HEAD and 9.0.X.  Odd I had never
> received a bug report about this before.  Good thing it didn't silently
> fail, but it is designed to be very picky.
>

Re: pg_upgrade

From
Bruce Momjian
Date:
Brian Hirt wrote:
> Bruce,
>
> The applied patch has the same behavior on i686 Ubuntu 10.04.   It
> looks like atol() is just a macro for strtol() in stdio.h.   I think
> you want strtoul() instead of strtol()

Yes, thanks.  I have now applied that fix in HEAD and 9.0.X.

> when i change str2uint() to use strtoul() pg_upgrade completes without
> a problem (I still haven't tested the upgrade database, but I expect
> that will be just fine).

Yep.

> I think it's pretty uncommon for the OID to be that big which is why
> nobody stumbled onto this.   This particular installation has pretty
> much been reloading development databases non stop for the last year.
> Also, people tend to initdb a lot when testing and doing development
> which will keep resetting the oid low.

Yes, seems > 2^31 oids are rarer than I thought.

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

  + It's impossible for everything to be true. +