Re: Removing pg_migrator limitations

Tom Lane wrote:
> The more I think about it the less I want such warts placed in the
> regular SQL syntax for creation commands.  As soon as we add a wart like
> that we'll be stuck with supporting it forever.  Whatever we do here
> should be off in a little corner that only pg_migrator can get at.

Yea, and we might need more some day so a system that can be easily
enhanced would help.  Adding to SQL syntax and maintaining it seems like
overkill.

> And we already have a way to manage that: there's already something
> in pg_migrator to let it install special functions that are present
> only while migrating.  So I suggest that we make whatever hacks are
> needed available only at the C-code level, and let pg_migrator get
> at them via its special functions.

Right.

> In practice, this would mean teaching pg_dump to call these functions
> when it is making a --binary_upgrade dump.  The reason I think this
> is less of a support hazard than changing SQL statements is that there
> is no promise or intention that a --binary_upgrade dump will load into
> anything but the specific PG version that it's intended for.  (We
> could, and probably should, add some version labeling to the dump to
> help enforce that.)

Yea, that is easy.

> At the moment it appears that we need the following hacks:
> 
> * ability to control the OIDs assigned to user tables and types.
> Because a table also has a rowtype, this means at least two separate
> state variables.  And we already knew we had to control the OIDs
> assigned to toast tables.  I'm imagining dump output like
> 
> 	select pg_migrator_set_next_table_oid(123456);
> 	select pg_migrator_set_next_type_oid(12347);
> 	select pg_migrator_set_next_toast_table_oid(123458);

I was thinking of something even more general:

	select pg_migrator_set_oid('pg_type', 100);
	select pg_migrator_set_oid('pg_type_array', 101);

and you just check for the strings in pg_migrator_set_oid and set the
proper variable.  The idea I had was to create a global structure:

	struct pg_migrator_oids {
		Oid	pg_type;
		Oid	pg_type_array;
		...
	}

This would initialize to zero as a global structure, and only
pg_migrator server-side functions set it.

> 	CREATE TABLE ...
> 
> where the functions cause static variables to become set, and the
> core code gets changed to look like
> 
> 	if (next_table_oid)
> 	{
> 		newoid = next_table_oid;
> 		next_table_oid = 0;
> 	}
> 	else
> 		newoid = GetNewOid(...);

Yes, that is what I was thinking too:

 	if (pg_migrator_oid.pg_type)
 	{
 		newoid = pg_migrator_oid.pg_type;
 		pg_migrator_oid.pg_type = 0;
 	}
	else
 		newoid = GetNewOid(...);

> in selected places where currently there's just a GetNewOid(...) call.
> 
> * ability to control the OIDs assigned to enum values.  To keep this
> sane I think the easiest way is to have pg_migrator have a function
> that adds one value with a predetermined OID to an existing enum.
> So instead of CREATE TYPE foo AS ENUM ('bar', 'baz', ...)
> I envision the --binary_upgrade dump output looking like
> 
> 	-- force the OID of the enum type itself
> 	select pg_migrator_set_next_type_oid(12347);
> 
> 	CREATE TYPE foo AS ENUM ();
> 
> 	select pg_migrator_add_enum_value(12347, 'bar', 12348);
> 	select pg_migrator_add_enum_value(12347, 'baz', 12349);
> 	...


Good idea --- I was trying to figure out how to assign an array of oids
and couldn't think of a simple way.

> I don't see any value in the placeholder-row approach Bruce suggests;
> AFAICS it would require significantly uglier backend hacks than the
> above because dealing with an already-present row would be a bigger
> code change.

True.

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

  + If your life is a hard drive, Christ can be your backup. +