Re: Re: [Pg-migrator-general] Composite types break pg_migrated tables - Mailing list pgsql-hackers

Bruce Momjian wrote:
> Andrew Dunstan wrote:
> >
> >
> > Bruce Momjian wrote:
> > > Do we have no composite types in the regression tests, or do we not
> > > store any in the database?  Same the enums.
> > >
> > >
> >
> > Looks like the enum regression tests at least drop all their tables :-(
> >
> > > To allow pg_migrator to work, I would need to reserve the oids in
> > > pg_type, import the dump, and renumber the pg_type entries (and
> > > everything pointing to them) to the proper pg_type.oid.  The big problem
> > > there is that I don't have access at the SQL level to set or change
> > > oids. I am afraid the oid remumbering is something we would have to do
> > > in the backend by walking through the pg_depend entries for the pg_type
> > > row.  Yuck.
> >
> > Yeah. Maybe we need some special way of setting the oids explicitly. But
> > preventing a clash might be fairly difficult.
> >
> > Excluding every database that has a composite/array-of
> > user-defined-type/enum type would be pretty nasty. After all, these are
> > features we boast of.
>
> Well, pg_migrator has gotten pretty far without supporting these
> features, and I think I would have heard about it if someone had these
> and migrated because vacuum analyze found it right away.  I am afraid
> the best we can do is to throw an error when we see these cases and hope
> we can improve things for 8.5.
>
> As I understand it I have to look for the _use_ of these in user tables,
> not the existance of them in pg_type ---  for example, there is
> certainly an array for every user type, but it might not be used by any
> user tables, and that would be OK.

I have applied the attached patch to pg_migrator to detect enum,
composites, and arrays.  I tested it and the only error I got was with
the breakmigrator table that was supplied by Jeff, and once I removed
that table the migration went fine, meaning there are no cases of these
stored in the regression test database.

I will release a new version of pg_migrator with these new detection
routines.

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

  + If your life is a hard drive, Christ can be your backup. +
? tools
? log
? src/pg_migrator
Index: src/pg_migrator.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/pg_migrator.c,v
retrieving revision 1.63
diff -c -r1.63 pg_migrator.c
*** src/pg_migrator.c    3 Aug 2009 01:40:09 -0000    1.63
--- src/pg_migrator.c    6 Aug 2009 03:25:40 -0000
***************
*** 74,79 ****
--- 74,82 ----
      {
          v8_3_check_for_name_data_type_usage(&ctx, CLUSTER_OLD);
          v8_3_check_for_tsquery_usage(&ctx, CLUSTER_OLD);
+         v8_3_check_for_composite_types(&ctx, CLUSTER_OLD);
+         v8_3_check_for_array_types(&ctx, CLUSTER_OLD);
+         v8_3_check_for_enum_types(&ctx, CLUSTER_OLD);
          if (ctx.check)
          {
              v8_3_rebuild_tsvector_tables(&ctx, true, CLUSTER_OLD);
Index: src/pg_migrator.h
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/pg_migrator.h,v
retrieving revision 1.67
diff -c -r1.67 pg_migrator.h
*** src/pg_migrator.h    2 Aug 2009 03:59:06 -0000    1.67
--- src/pg_migrator.h    6 Aug 2009 03:25:40 -0000
***************
*** 389,394 ****
--- 389,400 ----
                              Cluster whichCluster);
  void        v8_3_check_for_tsquery_usage(migratorContext *ctx,
                              Cluster whichCluster);
+ void        v8_3_check_for_composite_types(migratorContext *ctx,
+                             Cluster whichCluster);
+ void        v8_3_check_for_array_types(migratorContext *ctx,
+                             Cluster whichCluster);
+ void        v8_3_check_for_enum_types(migratorContext *ctx,
+                             Cluster whichCluster);
  void        v8_3_check_for_isn_and_int8_passing_mismatch(migratorContext *ctx,
                              Cluster whichCluster);
  void        v8_3_rebuild_tsvector_tables(migratorContext *ctx,
Index: src/relfilenode.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/relfilenode.c,v
retrieving revision 1.29
diff -c -r1.29 relfilenode.c
*** src/relfilenode.c    3 Aug 2009 01:40:09 -0000    1.29
--- src/relfilenode.c    6 Aug 2009 03:25:40 -0000
***************
*** 232,238 ****
      PGresult   *res;
      int            i_relfile;

!     prep_status(&ctx, "Getting pg_database and pg_largeobject relfilenodes");

      res = executeQueryOrDie(ctx, conn,
                              "SELECT c.relname, c.relfilenode "
--- 232,238 ----
      PGresult   *res;
      int            i_relfile;

!     prep_status(ctx, "Getting pg_database and pg_largeobject relfilenodes");

      res = executeQueryOrDie(ctx, conn,
                              "SELECT c.relname, c.relfilenode "
Index: src/version.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/version.c,v
retrieving revision 1.26
diff -c -r1.26 version.c
*** src/version.c    21 Jul 2009 17:36:23 -0000    1.26
--- src/version.c    6 Aug 2009 03:25:40 -0000
***************
*** 188,193 ****
--- 188,467 ----


  /*
+  * v8_3_check_for_composite_types()
+  *
+  *    composite types have pg_type oids in their data values and
+  *  pg_type.oid is not preserved between migrations.  We don't
+  *    have to worry about arrays of composite types because we
+  *    check arrays later
+  */
+ void
+ v8_3_check_for_composite_types(migratorContext *ctx, Cluster whichCluster)
+ {
+     ClusterInfo    *active_cluster = (whichCluster == CLUSTER_OLD) ?
+                     &ctx->old : &ctx->new;
+     int            dbnum;
+     FILE        *script = NULL;
+     bool        found = false;
+     char        output_path[MAXPGPATH];
+
+     prep_status(ctx, "Checking for user columns of composite types");
+
+     snprintf(output_path, sizeof(output_path), "%s/tables_using_composite_types.txt",
+             ctx->home_dir);
+
+     for (dbnum = 0; dbnum < active_cluster->dbarr.ndbs; dbnum++)
+     {
+         PGresult   *res;
+          bool        db_used = false;
+         int            ntups;
+         int            rowno;
+         int            i_nspname, i_relname, i_attname;
+         DbInfo       *active_db = &active_cluster->dbarr.dbs[dbnum];
+         PGconn       *conn = connectToServer(ctx, active_db->db_name, whichCluster);
+
+         /* Find any user-defined tsquery columns */
+         res = executeQueryOrDie(ctx, conn,
+                                 "SELECT n.nspname, c.relname, a.attname "
+                                 "FROM    pg_catalog.pg_class c, "
+                                 "        pg_catalog.pg_namespace n, "
+                                 "        pg_catalog.pg_attribute a, "
+                                 "        pg_catalog.pg_type t "
+                                 "WHERE    c.relkind = 'r' AND "
+                                 "        c.oid = a.attrelid AND "
+                                 "        NOT a.attisdropped AND "
+                                 "        a.atttypid = t.oid AND "
+                                 "        c.relnamespace = n.oid AND "
+                                 "        t.typtype = 'c' AND "
+                                 "        n.nspname != 'pg_catalog' AND "
+                                 "        n.nspname != 'information_schema'");
+
+         ntups = PQntuples(res);
+         i_nspname = PQfnumber(res, "nspname");
+         i_relname = PQfnumber(res, "relname");
+         i_attname = PQfnumber(res, "attname");
+         for (rowno = 0; rowno < ntups; rowno++)
+         {
+             found = true;
+             if (script == NULL && (script = fopen(output_path, "w")) == NULL)
+                     pg_log(ctx, PG_FATAL, "Could not create necessary file:  %s\n", output_path);
+             if (!db_used)
+             {
+                 fprintf(script, "Database:  %s\n", active_db->db_name);
+                 db_used = true;
+             }
+             fprintf(script, "  %s.%s.%s\n",
+                     PQgetvalue(res, rowno, i_nspname),
+                     PQgetvalue(res, rowno, i_relname),
+                     PQgetvalue(res, rowno, i_attname));
+         }
+
+         PQclear(res);
+
+         PQfinish(conn);
+     }
+
+     if (found)
+     {
+         fclose(script);
+         pg_log(ctx, PG_REPORT, "fatal\n");
+         pg_log(ctx, PG_FATAL,
+                 "| Your installation uses composite types.\n"
+                 "| These types are not supported for upgrade because\n"
+                 "| they contain an internal pg_type.oid that cannot be\n"
+                 "| migrated.  You can remove the problem columns and\n"
+                 "| restart the migration.  A list of the problem columns\n"
+                 "| is in the file:\n"
+                 "| \t%s\n\n", output_path);
+     }
+     else
+         check_ok(ctx);
+ }
+
+
+ /*
+  * v8_3_check_for_array_types()
+  *
+  *    array types have pg_type oids in their data values and
+  *  pg_type.oid is not preserved between migrations.  We catch
+  *    arrays of composite types here too.
+  */
+ void
+ v8_3_check_for_array_types(migratorContext *ctx, Cluster whichCluster)
+ {
+     ClusterInfo    *active_cluster = (whichCluster == CLUSTER_OLD) ?
+                     &ctx->old : &ctx->new;
+     int            dbnum;
+     FILE        *script = NULL;
+     bool        found = false;
+     char        output_path[MAXPGPATH];
+
+     prep_status(ctx, "Checking for user columns of array types");
+
+     snprintf(output_path, sizeof(output_path), "%s/tables_using_array_types.txt",
+             ctx->home_dir);
+
+     for (dbnum = 0; dbnum < active_cluster->dbarr.ndbs; dbnum++)
+     {
+         PGresult   *res;
+          bool        db_used = false;
+         int            ntups;
+         int            rowno;
+         int            i_nspname, i_relname, i_attname;
+         DbInfo       *active_db = &active_cluster->dbarr.dbs[dbnum];
+         PGconn       *conn = connectToServer(ctx, active_db->db_name, whichCluster);
+
+         /* Find any user-defined tsquery columns */
+         res = executeQueryOrDie(ctx, conn,
+                                 "SELECT n.nspname, c.relname, a.attname "
+                                 "FROM    pg_catalog.pg_class c, "
+                                 "        pg_catalog.pg_namespace n, "
+                                 "        pg_catalog.pg_attribute a, "
+                                 "        pg_catalog.pg_type t "
+                                 "WHERE    c.relkind = 'r' AND "
+                                 "        c.oid = a.attrelid AND "
+                                 "        NOT a.attisdropped AND "
+                                 "        a.atttypid = t.oid AND "
+                                 "        c.relnamespace = n.oid AND "
+                                 "        t.typtype = 'b' AND "
+                                 "        t.typtype = 'A' AND "
+                                 "        n.nspname != 'pg_catalog' AND "
+                                 "        n.nspname != 'information_schema'");
+
+         ntups = PQntuples(res);
+         i_nspname = PQfnumber(res, "nspname");
+         i_relname = PQfnumber(res, "relname");
+         i_attname = PQfnumber(res, "attname");
+         for (rowno = 0; rowno < ntups; rowno++)
+         {
+             found = true;
+             if (script == NULL && (script = fopen(output_path, "w")) == NULL)
+                     pg_log(ctx, PG_FATAL, "Could not create necessary file:  %s\n", output_path);
+             if (!db_used)
+             {
+                 fprintf(script, "Database:  %s\n", active_db->db_name);
+                 db_used = true;
+             }
+             fprintf(script, "  %s.%s.%s\n",
+                     PQgetvalue(res, rowno, i_nspname),
+                     PQgetvalue(res, rowno, i_relname),
+                     PQgetvalue(res, rowno, i_attname));
+         }
+
+         PQclear(res);
+
+         PQfinish(conn);
+     }
+
+     if (found)
+     {
+         fclose(script);
+         pg_log(ctx, PG_REPORT, "fatal\n");
+         pg_log(ctx, PG_FATAL,
+                 "| Your installation uses array types.\n"
+                 "| These types are not supported for upgrade because\n"
+                 "| they contain an internal pg_type.oid that cannot be\n"
+                 "| migrated.  You can remove the problem columns and\n"
+                 "| restart the migration.  A list of the problem columns\n"
+                 "| is in the file:\n"
+                 "| \t%s\n\n", output_path);
+     }
+     else
+         check_ok(ctx);
+ }
+
+
+ /*
+  * v8_3_check_for_enum_types()
+  *
+  *    enum types have pg_type oids in their data values and
+  *  pg_type.oid is not preserved between migrations.
+  */
+ void
+ v8_3_check_for_enum_types(migratorContext *ctx, Cluster whichCluster)
+ {
+     ClusterInfo    *active_cluster = (whichCluster == CLUSTER_OLD) ?
+                     &ctx->old : &ctx->new;
+     int            dbnum;
+     FILE        *script = NULL;
+     bool        found = false;
+     char        output_path[MAXPGPATH];
+
+     prep_status(ctx, "Checking for user columns of enum types");
+
+     snprintf(output_path, sizeof(output_path), "%s/tables_using_enum_types.txt",
+             ctx->home_dir);
+
+     for (dbnum = 0; dbnum < active_cluster->dbarr.ndbs; dbnum++)
+     {
+         PGresult   *res;
+          bool        db_used = false;
+         int            ntups;
+         int            rowno;
+         int            i_nspname, i_relname, i_attname;
+         DbInfo       *active_db = &active_cluster->dbarr.dbs[dbnum];
+         PGconn       *conn = connectToServer(ctx, active_db->db_name, whichCluster);
+
+         /* Find any user-defined tsquery columns */
+         res = executeQueryOrDie(ctx, conn,
+                                 "SELECT n.nspname, c.relname, a.attname "
+                                 "FROM    pg_catalog.pg_class c, "
+                                 "        pg_catalog.pg_namespace n, "
+                                 "        pg_catalog.pg_attribute a, "
+                                 "        pg_catalog.pg_type t "
+                                 "WHERE    c.relkind = 'r' AND "
+                                 "        c.oid = a.attrelid AND "
+                                 "        NOT a.attisdropped AND "
+                                 "        a.atttypid = t.oid AND "
+                                 "        c.relnamespace = n.oid AND "
+                                 "        t.typtype = 'e' AND "
+                                 "        n.nspname != 'pg_catalog' AND "
+                                 "        n.nspname != 'information_schema'");
+
+         ntups = PQntuples(res);
+         i_nspname = PQfnumber(res, "nspname");
+         i_relname = PQfnumber(res, "relname");
+         i_attname = PQfnumber(res, "attname");
+         for (rowno = 0; rowno < ntups; rowno++)
+         {
+             found = true;
+             if (script == NULL && (script = fopen(output_path, "w")) == NULL)
+                     pg_log(ctx, PG_FATAL, "Could not create necessary file:  %s\n", output_path);
+             if (!db_used)
+             {
+                 fprintf(script, "Database:  %s\n", active_db->db_name);
+                 db_used = true;
+             }
+             fprintf(script, "  %s.%s.%s\n",
+                     PQgetvalue(res, rowno, i_nspname),
+                     PQgetvalue(res, rowno, i_relname),
+                     PQgetvalue(res, rowno, i_attname));
+         }
+
+         PQclear(res);
+
+         PQfinish(conn);
+     }
+
+     if (found)
+     {
+         fclose(script);
+         pg_log(ctx, PG_REPORT, "fatal\n");
+         pg_log(ctx, PG_FATAL,
+                 "| Your installation uses enum types.\n"
+                 "| These types are not supported for upgrade because\n"
+                 "| they contain an internal pg_type.oid that cannot be\n"
+                 "| migrated.  You can remove the problem columns and\n"
+                 "| restart the migration.  A list of the problem columns\n"
+                 "| is in the file:\n"
+                 "| \t%s\n\n", output_path);
+     }
+     else
+         check_ok(ctx);
+ }
+
+
+ /*
   * v8_3_check_for_isn_and_int8_passing_mismatch()
   *
   *    /contrib/isn relies on data type bigint, and the CREATE TYPE