Re: pg_migrator issue with contrib - Mailing list pgsql-hackers

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Tom Lane wrote:
> >> I've spent some time thinking about possible workarounds for this, and
> >> not really come up with any.  The only feasible thing I can think of
> >> to do is teach pg_migrator to refuse to migrate if (a) the old DB
> >> contains contrib/isn, and (b) the new DB has FLOAT8PASSYVAL (which
> >> can be checked in pg_control).  One question here is how you decide
> >> if the old DB contains contrib/isn.  I don't think looking for the
> >> type name per se is a hot idea.  The best plan that has come to mind
> >> is to look through pg_proc to see if there are any C-language functions
> >> that reference "$libdir/isn".
>
> > Sure, pg_migrator is good at checking.  Please confirm you want this
> > added to pg_migrator.
>
> Yeah, I'd suggest it.  Even if we later come up with a workaround for
> contrib/isn, you're going to want to have the infrastructure in place
> for this type of check, because there will surely be cases that need it.
>
> Note that I think the FLOAT8PASSYVAL check is a must.  There is no
> reason to forbid migrating isn on 32-bit machines, for example.

Done, with patch attached, and pg_migrator beta6 released.

--
  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/controldata.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/controldata.c,v
retrieving revision 1.14
diff -c -r1.14 controldata.c
*** src/controldata.c    13 May 2009 15:19:16 -0000    1.14
--- src/controldata.c    8 Jun 2009 21:29:31 -0000
***************
*** 23,29 ****
   */
  void
  get_control_data(migratorContext *ctx, const char *bindir,
!                  const char *datadir, ControlData *ctrl)
  {
      char        cmd[MAXPGPATH];
      char        bufin[MAX_STRING];
--- 23,30 ----
   */
  void
  get_control_data(migratorContext *ctx, const char *bindir,
!                  const char *datadir, ControlData *ctrl,
!                  const uint32 pg_version)
  {
      char        cmd[MAXPGPATH];
      char        bufin[MAX_STRING];
***************
*** 43,48 ****
--- 44,50 ----
      bool        got_index = false;
      bool        got_toast = false;
      bool        got_date_is_int = false;
+     bool        got_float8_pass_by_value = false;
      char       *lang = NULL;

      /* Because we test the pg_resetxlog output strings, it has to be in English. */
***************
*** 65,71 ****
      /* Only pre-8.4 has these so if they are not set below we will check later */
      ctrl->lc_collate = NULL;
      ctrl->lc_ctype = NULL;
!
      /* we have the result of cmd in "output". so parse it line by line now */
      while (fgets(bufin, sizeof(bufin), output))
      {
--- 67,80 ----
      /* Only pre-8.4 has these so if they are not set below we will check later */
      ctrl->lc_collate = NULL;
      ctrl->lc_ctype = NULL;
!
!     /* Not in pre-8.4 */
!     if (pg_version < 80400)
!     {
!         ctrl->float8_pass_by_value = false;
!         got_float8_pass_by_value = true;
!     }
!
      /* we have the result of cmd in "output". so parse it line by line now */
      while (fgets(bufin, sizeof(bufin), output))
      {
***************
*** 249,254 ****
--- 258,275 ----
              ctrl->date_is_int = strstr(p, "64-bit integers") != NULL;
              got_date_is_int = true;
          }
+         else if ((p = strstr(bufin, "Float8 argument passing:")) != NULL)
+         {
+             p = strchr(p, ':');
+
+             if (p == NULL || strlen(p) <= 1)
+                 pg_log(ctx, PG_FATAL, "%d: pg_resetxlog  problem\n", __LINE__);
+
+             p++;                /* removing ':' char */
+             /* used later for /contrib check */
+             ctrl->float8_pass_by_value = strstr(p, "by value") != NULL;
+             got_float8_pass_by_value = true;
+         }
          /* In pre-8.4 only */
          else if ((p = strstr(bufin, "LC_COLLATE:")) != NULL)
          {
***************
*** 305,311 ****
      if (!got_xid || !got_oid || !got_log_id || !got_log_seg || !got_tli ||
          !got_align || !got_blocksz || !got_largesz || !got_walsz ||
          !got_walseg || !got_ident || !got_index || !got_toast ||
!         !got_date_is_int)
      {
          pg_log(ctx, PG_REPORT,
              "Some required control information is missing;  cannot find:\n");
--- 326,332 ----
      if (!got_xid || !got_oid || !got_log_id || !got_log_seg || !got_tli ||
          !got_align || !got_blocksz || !got_largesz || !got_walsz ||
          !got_walseg || !got_ident || !got_index || !got_toast ||
!         !got_date_is_int || !got_float8_pass_by_value)
      {
          pg_log(ctx, PG_REPORT,
              "Some required control information is missing;  cannot find:\n");
***************
*** 352,357 ****
--- 373,382 ----
          if (!got_date_is_int)
              pg_log(ctx, PG_REPORT, "  dates/times are integers?\n");

+         /* value added in Postgres 8.4 */
+         if (!got_float8_pass_by_value)
+             pg_log(ctx, PG_REPORT, "  float8 argument passing method\n");
+
          pg_log(ctx, PG_FATAL,
                 "Unable to continue without required control information, terminating\n");
      }
Index: src/pg_migrator.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/pg_migrator.c,v
retrieving revision 1.42
diff -c -r1.42 pg_migrator.c
*** src/pg_migrator.c    4 Jun 2009 22:01:55 -0000    1.42
--- src/pg_migrator.c    8 Jun 2009 21:29:31 -0000
***************
*** 71,76 ****
--- 71,77 ----
      {
          v8_3_check_for_name_data_type_usage(&ctx, CLUSTER_OLD);
          v8_3_check_for_tsquery_usage(&ctx, CLUSTER_OLD);
+         v8_3_check_for_isn_and_int8_passing_mismatch(&ctx, CLUSTER_OLD);
          if (ctx.check)
          {
              v8_3_rebuild_tsvector_tables(&ctx, true, CLUSTER_OLD);
***************
*** 560,568 ****

      /* get/check pg_control data of servers */
      get_control_data(ctx, ctx->old.bindir, ctx->old.pgdata,
!                      &ctx->old.controldata);
      get_control_data(ctx, ctx->new.bindir, ctx->new.pgdata,
!                      &ctx->new.controldata);
      check_control_data(ctx, &ctx->old.controldata, &ctx->new.controldata);
  }

--- 561,569 ----

      /* get/check pg_control data of servers */
      get_control_data(ctx, ctx->old.bindir, ctx->old.pgdata,
!                      &ctx->old.controldata, ctx->old.pg_version);
      get_control_data(ctx, ctx->new.bindir, ctx->new.pgdata,
!                      &ctx->new.controldata, ctx->new.pg_version);
      check_control_data(ctx, &ctx->old.controldata, &ctx->new.controldata);
  }

Index: src/pg_migrator.h
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/pg_migrator.h,v
retrieving revision 1.51
diff -c -r1.51 pg_migrator.h
*** src/pg_migrator.h    8 Jun 2009 18:33:32 -0000    1.51
--- src/pg_migrator.h    8 Jun 2009 21:29:31 -0000
***************
*** 145,150 ****
--- 145,151 ----
      uint32        index;
      uint32        toast;
      bool        date_is_int;
+     bool        float8_pass_by_value;
      char       *lc_collate;
      char       *lc_ctype;
      char       *encoding;
***************
*** 244,250 ****
  /* controldata.c */

  void get_control_data(migratorContext *ctx, const char *bindir,
!                  const char *datadir, ControlData *ctrl);
  void check_control_data(migratorContext *ctx, ControlData *oldctrl,
                     ControlData *newctrl);
  void set_locale_and_encoding(migratorContext *ctx, Cluster whichCluster);
--- 245,252 ----
  /* controldata.c */

  void get_control_data(migratorContext *ctx, const char *bindir,
!                  const char *datadir, ControlData *ctrl,
!                  const uint32 pg_version);
  void check_control_data(migratorContext *ctx, ControlData *oldctrl,
                     ControlData *newctrl);
  void set_locale_and_encoding(migratorContext *ctx, Cluster whichCluster);
***************
*** 375,380 ****
--- 377,384 ----
                              Cluster whichCluster);
  void        v8_3_check_for_tsquery_usage(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,
                              bool check_mode, Cluster whichCluster);
  void        v8_3_invalidate_hash_gin_indexes(migratorContext *ctx,
Index: src/version.c
===================================================================
RCS file: /cvsroot/pg-migrator/pg_migrator/src/version.c,v
retrieving revision 1.17
diff -c -r1.17 version.c
*** src/version.c    31 May 2009 20:25:48 -0000    1.17
--- src/version.c    8 Jun 2009 21:29:31 -0000
***************
*** 89,95 ****
                  "| Your installation uses the \"name\" data type in user tables.\n"
                  "| This data type changed its internal alignment between your old and\n"
                  "| new clusters so this cluster cannot currently be upgraded.\n"
!                 "| You can remove the problem tables and restart this program.\n"
                  "| Simply dropping the offending columns will not work.\n"
                  "| A list of the problem columns is in the file\n"
                  "| \"%s\".\n\n", output_path);
--- 89,95 ----
                  "| Your installation uses the \"name\" data type in user tables.\n"
                  "| This data type changed its internal alignment between your old and\n"
                  "| new clusters so this cluster cannot currently be upgraded.\n"
!                 "| You can remove the problem tables and restart the migration.\n"
                  "| Simply dropping the offending columns will not work.\n"
                  "| A list of the problem columns is in the file\n"
                  "| \"%s\".\n\n", output_path);
***************
*** 178,184 ****
                  "| Your installation uses the \"tsquery\" data type.\n"
                  "| This data type added a new internal field between your old and\n"
                  "| new clusters so this cluster cannot currently be upgraded.\n"
!                 "| You can remove the problem columns and restart this program.\n"
                  "| A list of the problem columns is in the file\n"
                  "| \"%s\".\n\n", output_path);
      }
--- 178,184 ----
                  "| Your installation uses the \"tsquery\" data type.\n"
                  "| This data type added a new internal field between your old and\n"
                  "| new clusters so this cluster cannot currently be upgraded.\n"
!                 "| You can remove the problem columns and restart the migration.\n"
                  "| A list of the problem columns is in the file\n"
                  "| \"%s\".\n\n", output_path);
      }
***************
*** 188,193 ****
--- 188,286 ----


  /*
+  * v8_3_check_for_isn_and_int8_passing_mismatch()
+  *
+  *    /contrib/isn relies on data type bigint, and the CREATE TYPE
+  *  PASSEDBYVALUE setting might not match in the old and new servers,
+  *    so the new shared object file would work unreliably.  Passing int8
+  *    by value is new in Postgres 8.4.
+  */
+ void
+ v8_3_check_for_isn_and_int8_passing_mismatch(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 /contrib/isn with bigint-passing mismatch");
+
+     if (ctx->old.controldata.float8_pass_by_value ==
+         ctx->new.controldata.float8_pass_by_value)
+     {
+         /* no mismatch */
+         check_ok(ctx);
+         return;
+     }
+
+     snprintf(output_path, sizeof(output_path), "%s/contrib_isn_and_int8_pass_by_value.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_proname;
+         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, p.proname "
+                                 "FROM    pg_catalog.pg_proc p, "
+                                 "        pg_catalog.pg_namespace n "
+                                 "WHERE    p.pronamespace = n.oid AND "
+                                 "        p.probin = '$libdir/isn' AND "
+                                 "        n.nspname != 'pg_catalog' AND "
+                                 "        n.nspname != 'information_schema'");
+
+         ntups = PQntuples(res);
+         i_nspname = PQfnumber(res, "nspname");
+         i_proname = PQfnumber(res, "proname");
+         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\n",
+                     PQgetvalue(res, rowno, i_nspname),
+                     PQgetvalue(res, rowno, i_proname));
+         }
+
+         PQclear(res);
+
+         PQfinish(conn);
+     }
+
+     if (found)
+     {
+         fclose(script);
+         pg_log(ctx, PG_REPORT, "fatal\n");
+         pg_log(ctx, PG_FATAL,
+                 "| Your installation uses \"/contrib/isn\" functions\n"
+                 "| which rely on the bigint data type.  Your old and new\n"
+                 "| clusters pass bigint values differently so this cluster\n"
+                 "| cannot currently be upgraded.  You can manually migrate\n"
+                 "| data that use \"/contrib/isn\" facilities and remove\n"
+                 "| \"/contrib/isn\" from the old cluster and restart the\n"
+                 "| migration.  A list of the problem functions is in the\n"
+                 "| file \"%s\".\n\n", output_path);
+     }
+     else
+         check_ok(ctx);
+ }
+
+
+ /*
   * v8_3_rebuild_tsvector_tables()
   *
   * 8.3 sorts lexemes by its length and if lengths are the same then it uses