Applied. We can mark this report closed. Groshev, let us know if you have any further problems.
--------------------------------------------------------------------------- On Thu, Dec 20, 2012 at 07:19:48AM -0500, Bruce Momjian wrote: > On Wed, Dec 19, 2012 at 10:19:30PM -0500, Bruce Momjian wrote: > > On Wed, Dec 19, 2012 at 12:56:05PM -0500, Kevin Grittner wrote: > > > Groshev Andrey wrote: > > > > > > > >>>>> Mismatch of relation names: database "database", old rel > > > > >>>>> public.lob.ВерсияВнешнегоДокумента$Документ_pkey, new rel > > > > >>>>> public.plob.ВерсияВнешнегоДокумента$Документ > > > > > > There is a limit on identifiers of 63 *bytes* (not characters) > > > after which the name is truncated. In UTF8 encoding, the underscore > > > would be in the 64th position. > > > > OK, Kevin is certainly pointing out a bug in the pg_upgrade code, though > > I am unclear how it would exhibit the mismatch error reported. > > > > pg_upgrade uses NAMEDATALEN for database, schema, and relation name > > storage lengths. While NAMEDATALEN works fine in the backend, it is > > possible that a frontend client, like pg_upgrade, could retrieve a name > > in the client encoding whose length exceeds NAMEDATALEN if the client > > encoding did not match the database encoding (or is it the cluster > > encoding for system tables). This would cause truncation of these > > values. The truncation would not cause crashes, but might cause > > failures by not being able to connect to overly-long database names, and > > it weakens the checking of relation/schema names --- the same check that > > is reported above. > > > > (I believe initdb.c also erroneously uses NAMEDATALEN.) > > I have developed the attached patch to pg_strdup() the string returned > from libpq, rather than use a fixed NAMEDATALEN buffer to store the > value. I am only going to apply this to 9.3 because I can't see this > causing problems except for weaker comparisons for very long identifiers > where the client encoding is longer than the server encoding, and > failures for very long database names, no of which we have gotten bug > reports about. > > Turns out initdb.c was fine because it expects only collation names to > be only in ASCII; I added a comment to that effect. > > -- > Bruce Momjian <br...@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > > + It's impossible for everything to be true. + > diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c > new file mode 100644 > index 2250442..5cb9b61 > *** a/contrib/pg_upgrade/info.c > --- b/contrib/pg_upgrade/info.c > *************** static void get_db_infos(ClusterInfo *cl > *** 23,29 **** > static void get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo); > static void free_rel_infos(RelInfoArr *rel_arr); > static void print_db_infos(DbInfoArr *dbinfo); > ! static void print_rel_infos(RelInfoArr *arr); > > > /* > --- 23,29 ---- > static void get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo); > static void free_rel_infos(RelInfoArr *rel_arr); > static void print_db_infos(DbInfoArr *dbinfo); > ! static void print_rel_infos(RelInfoArr *rel_arr); > > > /* > *************** create_rel_filename_map(const char *old_ > *** 127,134 **** > map->new_relfilenode = new_rel->relfilenode; > > /* used only for logging and error reporing, old/new are identical */ > ! snprintf(map->nspname, sizeof(map->nspname), "%s", old_rel->nspname); > ! snprintf(map->relname, sizeof(map->relname), "%s", old_rel->relname); > } > > > --- 127,134 ---- > map->new_relfilenode = new_rel->relfilenode; > > /* used only for logging and error reporing, old/new are identical */ > ! map->nspname = old_rel->nspname; > ! map->relname = old_rel->relname; > } > > > *************** get_db_infos(ClusterInfo *cluster) > *** 220,227 **** > for (tupnum = 0; tupnum < ntups; tupnum++) > { > dbinfos[tupnum].db_oid = atooid(PQgetvalue(res, tupnum, i_oid)); > ! snprintf(dbinfos[tupnum].db_name, > sizeof(dbinfos[tupnum].db_name), "%s", > ! PQgetvalue(res, tupnum, i_datname)); > snprintf(dbinfos[tupnum].db_tblspace, > sizeof(dbinfos[tupnum].db_tblspace), "%s", > PQgetvalue(res, tupnum, i_spclocation)); > } > --- 220,226 ---- > for (tupnum = 0; tupnum < ntups; tupnum++) > { > dbinfos[tupnum].db_oid = atooid(PQgetvalue(res, tupnum, i_oid)); > ! dbinfos[tupnum].db_name = pg_strdup(PQgetvalue(res, tupnum, > i_datname)); > snprintf(dbinfos[tupnum].db_tblspace, > sizeof(dbinfos[tupnum].db_tblspace), "%s", > PQgetvalue(res, tupnum, i_spclocation)); > } > *************** get_rel_infos(ClusterInfo *cluster, DbIn > *** 346,355 **** > curr->reloid = atooid(PQgetvalue(res, relnum, i_oid)); > > nspname = PQgetvalue(res, relnum, i_nspname); > ! strlcpy(curr->nspname, nspname, sizeof(curr->nspname)); > > relname = PQgetvalue(res, relnum, i_relname); > ! strlcpy(curr->relname, relname, sizeof(curr->relname)); > > curr->relfilenode = atooid(PQgetvalue(res, relnum, > i_relfilenode)); > > --- 345,354 ---- > curr->reloid = atooid(PQgetvalue(res, relnum, i_oid)); > > nspname = PQgetvalue(res, relnum, i_nspname); > ! curr->nspname = pg_strdup(nspname); > > relname = PQgetvalue(res, relnum, i_relname); > ! curr->relname = pg_strdup(relname); > > curr->relfilenode = atooid(PQgetvalue(res, relnum, > i_relfilenode)); > > *************** free_db_and_rel_infos(DbInfoArr *db_arr) > *** 377,383 **** > --- 376,385 ---- > int dbnum; > > for (dbnum = 0; dbnum < db_arr->ndbs; dbnum++) > + { > free_rel_infos(&db_arr->dbs[dbnum].rel_arr); > + pg_free(db_arr->dbs[dbnum].db_name); > + } > pg_free(db_arr->dbs); > db_arr->dbs = NULL; > db_arr->ndbs = 0; > *************** free_db_and_rel_infos(DbInfoArr *db_arr) > *** 387,392 **** > --- 389,401 ---- > static void > free_rel_infos(RelInfoArr *rel_arr) > { > + int relnum; > + > + for (relnum = 0; relnum < rel_arr->nrels; relnum++) > + { > + pg_free(rel_arr->rels[relnum].nspname); > + pg_free(rel_arr->rels[relnum].relname); > + } > pg_free(rel_arr->rels); > rel_arr->nrels = 0; > } > *************** print_db_infos(DbInfoArr *db_arr) > *** 407,418 **** > > > static void > ! print_rel_infos(RelInfoArr *arr) > { > int relnum; > > ! for (relnum = 0; relnum < arr->nrels; relnum++) > pg_log(PG_VERBOSE, "relname: %s.%s: reloid: %u reltblspace: > %s\n", > ! arr->rels[relnum].nspname, arr->rels[relnum].relname, > ! arr->rels[relnum].reloid, > arr->rels[relnum].tablespace); > } > --- 416,427 ---- > > > static void > ! print_rel_infos(RelInfoArr *rel_arr) > { > int relnum; > > ! for (relnum = 0; relnum < rel_arr->nrels; relnum++) > pg_log(PG_VERBOSE, "relname: %s.%s: reloid: %u reltblspace: > %s\n", > ! rel_arr->rels[relnum].nspname, > rel_arr->rels[relnum].relname, > ! rel_arr->rels[relnum].reloid, > rel_arr->rels[relnum].tablespace); > } > diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h > new file mode 100644 > index 972e8e9..cae1e46 > *** a/contrib/pg_upgrade/pg_upgrade.h > --- b/contrib/pg_upgrade/pg_upgrade.h > *************** extern char *output_files[]; > *** 114,121 **** > */ > typedef struct > { > ! char nspname[NAMEDATALEN]; /* namespace name */ > ! char relname[NAMEDATALEN]; /* relation name */ > Oid reloid; /* relation oid */ > Oid relfilenode; /* relation relfile node */ > /* relation tablespace path, or "" for the cluster default */ > --- 114,122 ---- > */ > typedef struct > { > ! /* Can't use NAMEDATALEN; not guaranteed to fit on client */ > ! char *nspname; /* namespace name */ > ! char *relname; /* relation name */ > Oid reloid; /* relation oid */ > Oid relfilenode; /* relation relfile node */ > /* relation tablespace path, or "" for the cluster default */ > *************** typedef struct > *** 143,150 **** > Oid old_relfilenode; > Oid new_relfilenode; > /* the rest are used only for logging and error reporting */ > ! char nspname[NAMEDATALEN]; /* namespaces */ > ! char relname[NAMEDATALEN]; > } FileNameMap; > > /* > --- 144,151 ---- > Oid old_relfilenode; > Oid new_relfilenode; > /* the rest are used only for logging and error reporting */ > ! char *nspname; /* namespaces */ > ! char *relname; > } FileNameMap; > > /* > *************** typedef struct > *** 153,159 **** > typedef struct > { > Oid db_oid; /* oid of the database > */ > ! char db_name[NAMEDATALEN]; /* database name */ > char db_tblspace[MAXPGPATH]; /* database default tablespace > path */ > RelInfoArr rel_arr; /* array of all user relinfos */ > } DbInfo; > --- 154,160 ---- > typedef struct > { > Oid db_oid; /* oid of the database > */ > ! char *db_name; /* database name */ > char db_tblspace[MAXPGPATH]; /* database default tablespace > path */ > RelInfoArr rel_arr; /* array of all user relinfos */ > } DbInfo; > diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c > new file mode 100644 > index 40740dc..3e05ac3 > *** a/src/bin/initdb/initdb.c > --- b/src/bin/initdb/initdb.c > *************** setup_collation(void) > *** 1836,1842 **** > #if defined(HAVE_LOCALE_T) && !defined(WIN32) > int i; > FILE *locale_a_handle; > ! char localebuf[NAMEDATALEN]; > int count = 0; > > PG_CMD_DECL; > --- 1836,1842 ---- > #if defined(HAVE_LOCALE_T) && !defined(WIN32) > int i; > FILE *locale_a_handle; > ! char localebuf[NAMEDATALEN]; /* we assume ASCII so this is > fine */ > int count = 0; > > PG_CMD_DECL; > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers