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
