On Fri, Jul  4, 2014 at 12:01:37AM -0400, Bruce Momjian wrote:
> > The most robust, but not trivial, approach seems to be to prevent toast
> > table creation if there wasn't a set_next_toast_pg_class_oid(). Then,
> > after all relations are created, iterate over all pg_class entries that
> > possibly need toast tables and recheck if they now might need one.
> 
> Wow, that is going to be kind of odd in that there really isn't a good
> way to create toast tables except perhaps add a dummy TEXT column and
> remove it.  There also isn't an easy way to not create a toast table,
> but also find out that one was needed.  I suppose we would have to
> insert some dummy value in the toast pg_class column and come back later
> to clean it up.
> 
> I am wondering what the probability of having a table that didn't need a
> toast table in the old cluster, and needed one in the new cluster, and
> there being an oid collision.
> 
> I think the big question is whether we want to go down that route.

Here is an updated patch.  It was a little tricky because if the
mismatched non-toast table is after the last old relation, you need to
test differently and emit a different error message as there is no old
relation to complain about.

As far as the reusing of oids, we don't set the oid counter until after
the restore, so any new unmatched toast table would given a very low
oid.  Since we restore in oid order, for an oid to be assigned that was
used in the old cluster, it would have to be a very early relation, so I
think that reduces the odds considerably.  I am not inclined to do
anything more to avoid this until I see an actual error report ---
trying to address it might be invasive and introduce new errors.

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

  + Everyone has their own god. +
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
new file mode 100644
index 6205c74..72f805c
*** a/contrib/pg_upgrade/info.c
--- b/contrib/pg_upgrade/info.c
*************** gen_db_file_maps(DbInfo *old_db, DbInfo
*** 38,58 ****
  				 int *nmaps, const char *old_pgdata, const char *new_pgdata)
  {
  	FileNameMap *maps;
! 	int			relnum;
  	int			num_maps = 0;
  
  	maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
  									 old_db->rel_arr.nrels);
  
! 	for (relnum = 0; relnum < Min(old_db->rel_arr.nrels, new_db->rel_arr.nrels);
! 		 relnum++)
  	{
! 		RelInfo    *old_rel = &old_db->rel_arr.rels[relnum];
! 		RelInfo    *new_rel = &new_db->rel_arr.rels[relnum];
  
  		if (old_rel->reloid != new_rel->reloid)
! 			pg_fatal("Mismatch of relation OID in database \"%s\": old OID %d, new OID %d\n",
! 					 old_db->db_name, old_rel->reloid, new_rel->reloid);
  
  		/*
  		 * TOAST table names initially match the heap pg_class oid. In
--- 38,96 ----
  				 int *nmaps, const char *old_pgdata, const char *new_pgdata)
  {
  	FileNameMap *maps;
! 	int			old_relnum, new_relnum;
  	int			num_maps = 0;
  
  	maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
  									 old_db->rel_arr.nrels);
  
! 	/*
! 	 * The old database shouldn't have more relations than the new one.
! 	 * We force the new cluster to have a TOAST table if the old table
! 	 * had one.
! 	 */
! 	if (old_db->rel_arr.nrels > new_db->rel_arr.nrels)
! 		pg_fatal("old and new databases \"%s\" have a mismatched number of relations\n",
! 				 old_db->db_name);
! 
! 	/* Drive the loop using new_relnum, which might be higher. */
! 	for (old_relnum = new_relnum = 0; new_relnum < new_db->rel_arr.nrels;
! 		 new_relnum++)
  	{
! 		RelInfo    *old_rel;
! 		RelInfo    *new_rel = &new_db->rel_arr.rels[new_relnum];
! 
! 		/*
! 		 * It is possible that the new cluster has a TOAST table for a table
! 		 * that didn't need one in the old cluster, e.g. 9.0 to 9.1 changed the
! 		 * NUMERIC length computation.  Therefore, if we have a TOAST table
! 		 * in the new cluster that doesn't match, skip over it and continue
! 		 * processing.  It is possible this TOAST table used an OID that was
! 		 * reserved in the old cluster, but we have no way of testing that,
! 		 * and we would have already gotten an error at the new cluster schema
! 		 * creation stage.  Fortunately, since we only restore the OID counter
! 		 * after schema restore, and restore in OID order, a conflict would
! 		 * only happen if the new TOAST table had a very low OID.
! 		 */
! 		if (old_relnum == old_db->rel_arr.nrels)
! 		{
! 			if (strcmp(new_rel->nspname, "pg_toast") == 0)
! 				continue;
! 			else
! 				pg_fatal("Extra non-TOAST relation found in database \"%s\": new OID %d\n",
! 						 old_db->db_name, new_rel->reloid);
! 		}
! 
! 		old_rel = &old_db->rel_arr.rels[old_relnum];
  
  		if (old_rel->reloid != new_rel->reloid)
! 		{
! 			if (strcmp(new_rel->nspname, "pg_toast") == 0)
! 				continue;
! 			else
! 				pg_fatal("Mismatch of relation OID in database \"%s\": old OID %d, new OID %d\n",
! 						 old_db->db_name, old_rel->reloid, new_rel->reloid);
! 		}
  
  		/*
  		 * TOAST table names initially match the heap pg_class oid. In
*************** gen_db_file_maps(DbInfo *old_db, DbInfo
*** 76,89 ****
  		create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
  								old_rel, new_rel, maps + num_maps);
  		num_maps++;
  	}
  
! 	/*
! 	 * Do this check after the loop so hopefully we will produce a clearer
! 	 * error above
! 	 */
! 	if (old_db->rel_arr.nrels != new_db->rel_arr.nrels)
! 		pg_fatal("old and new databases \"%s\" have a different number of relations\n",
  				 old_db->db_name);
  
  	*nmaps = num_maps;
--- 114,125 ----
  		create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
  								old_rel, new_rel, maps + num_maps);
  		num_maps++;
+ 		old_relnum++;
  	}
  
! 	/* Did we fail to exhaust the old array? */
! 	if (old_relnum != old_db->rel_arr.nrels)
! 		pg_fatal("old and new databases \"%s\" have a mismatched number of relations\n",
  				 old_db->db_name);
  
  	*nmaps = num_maps;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to