Re: pg_upgrade code questions

Josh Berkus wrote:
> On 5/13/10 10:14 AM, Bruce Momjian wrote:
> > I am trying to think of this as a non-EnterpriseDB employee.  If suppose
> > Greenplum had given us a utility and they wanted it to work with their
> > version of the database, what accommodation would we make for them?  I
> > agree on the documentation, but would we allow #ifdefs that were only
> > used by them if there were only a few of them?  Could we treat it as an
> > operating system that none of us use?  I don't think Greenplum would
> > require us to keep support for their database, but they would prefer it,
> > and it might encourage more contributions from them.  Maybe we would
> > just tell them to keep their own patches, but I figured I would ask
> > specifically so we have a policy for next time.
>
> My $0.021746:
>
> If something is going to be included in /contrib, it should only include
> code which relates to standard PostgreSQL.  The independant pg_migrator
> project can be a PG/EDBAS tool; the contrib module needs to be
> vanilla-postgres only.  If the donor of the code wants to keep the
> specific fork support, then it should remain an independant project.
>
> I'm not just referring to EDB here, or even just proprietary forks; even
> open source forks (like PostgresXC or pgCluster) shouldn't have specific
> code in /contrib.  Within the limits of reasonableness, of course.
>
> My argument isn't based on purity, but is rather based on:
> (a) avoiding confusing the users, and
> (b) avoiding bulking code with lots of ifdefs if we can avoid it, and
> (c) fork release cycles are often different from pgsql-core, and EDB's
> certainly is.

I was more interested in understanding our policy rather than how to
handle this specific issue.  I have removed all mentions of EnterpriseDB
Advanced Server from pg_upgrade with the attached patch.  I will keep
the patch for submission back to EnterpriseDB when they want it, or they
can just pull it from CVS.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
Index: contrib/pg_upgrade/check.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/check.c,v
retrieving revision 1.3
diff -c -c -r1.3 check.c
*** contrib/pg_upgrade/check.c    13 May 2010 15:58:15 -0000    1.3
--- contrib/pg_upgrade/check.c    13 May 2010 22:48:06 -0000
***************
*** 149,158 ****
          {
              prep_status(ctx, "Adjusting sequences");
              exec_prog(ctx, true,
!                     SYSTEMQUOTE "\"%s/%s\" --set ON_ERROR_STOP=on --port %d "
                        "-f \"%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
!                       ctx->new.bindir, ctx->new.psql_exe, ctx->new.port,
!                       sequence_script_file_name, ctx->logfile);
              unlink(sequence_script_file_name);
              pg_free(sequence_script_file_name);
              check_ok(ctx);
--- 149,158 ----
          {
              prep_status(ctx, "Adjusting sequences");
              exec_prog(ctx, true,
!                     SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on --port %d "
                        "-f \"%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
!                       ctx->new.bindir, ctx->new.port, sequence_script_file_name,
!                       ctx->logfile);
              unlink(sequence_script_file_name);
              pg_free(sequence_script_file_name);
              check_ok(ctx);
Index: contrib/pg_upgrade/controldata.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/controldata.c,v
retrieving revision 1.1
diff -c -c -r1.1 controldata.c
*** contrib/pg_upgrade/controldata.c    12 May 2010 02:19:10 -0000    1.1
--- contrib/pg_upgrade/controldata.c    13 May 2010 22:48:06 -0000
***************
*** 9,18 ****
  #include <ctype.h>
  #include <stdlib.h>

- #ifdef EDB_NATIVE_LANG
- #include "access/tuptoaster.h"
- #endif
-

  /*
   * get_control_data()
--- 9,14 ----
***************
*** 88,102 ****
          got_float8_pass_by_value = true;
      }

- #ifdef EDB_NATIVE_LANG
-     /* EDB AS 8.3 is an 8.2 code base */
-     if (cluster->is_edb_as && GET_MAJOR_VERSION(cluster->major_version) <= 803)
-     {
-         cluster->controldata.toast = TOAST_MAX_CHUNK_SIZE;
-         got_toast = true;
-     }
- #endif
-
      /* we have the result of cmd in "output". so parse it line by line now */
      while (fgets(bufin, sizeof(bufin), output))
      {
--- 84,89 ----
***************
*** 140,148 ****
              p++;                /* removing ':' char */
              cluster->controldata.cat_ver = (uint32) atol(p);
          }
!         else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL ||
!                  (cluster->is_edb_as && GET_MAJOR_VERSION(cluster->major_version) <= 803 &&
!                   (p = strstr(bufin, "Current log file ID:")) != NULL))
          {
              p = strchr(p, ':');

--- 127,133 ----
              p++;                /* removing ':' char */
              cluster->controldata.cat_ver = (uint32) atol(p);
          }
!         else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL)
          {
              p = strchr(p, ':');

***************
*** 153,161 ****
              cluster->controldata.logid = (uint32) atol(p);
              got_log_id = true;
          }
!         else if ((p = strstr(bufin, "First log file segment after reset:")) != NULL ||
!                  (cluster->is_edb_as && GET_MAJOR_VERSION(cluster->major_version) <= 803 &&
!                   (p = strstr(bufin, "Next log file segment:")) != NULL))
          {
              p = strchr(p, ':');

--- 138,144 ----
              cluster->controldata.logid = (uint32) atol(p);
              got_log_id = true;
          }
!         else if ((p = strstr(bufin, "First log file segment after reset:")) != NULL)
          {
              p = strchr(p, ':');

Index: contrib/pg_upgrade/exec.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/exec.c,v
retrieving revision 1.2
diff -c -c -r1.2 exec.c
*** contrib/pg_upgrade/exec.c    13 May 2010 15:58:15 -0000    1.2
--- contrib/pg_upgrade/exec.c    13 May 2010 22:48:06 -0000
***************
*** 11,18 ****


  static void checkBinDir(migratorContext *ctx, ClusterInfo *cluster);
! static int check_exec(migratorContext *ctx, const char *dir, const char *cmdName,
!            const char *alternative);
  static const char *validate_exec(const char *path);
  static int    check_data_dir(migratorContext *ctx, const char *pg_data);

--- 11,17 ----


  static void checkBinDir(migratorContext *ctx, ClusterInfo *cluster);
! static int check_exec(migratorContext *ctx, const char *dir, const char *cmdName);
  static const char *validate_exec(const char *path);
  static int    check_data_dir(migratorContext *ctx, const char *pg_data);

***************
*** 89,110 ****
  static void
  checkBinDir(migratorContext *ctx, ClusterInfo *cluster)
  {
!     check_exec(ctx, cluster->bindir, "postgres", "edb-postgres");
!     check_exec(ctx, cluster->bindir, "pg_ctl", NULL);
!     check_exec(ctx, cluster->bindir, "pg_dumpall", NULL);
!
! #ifdef EDB_NATIVE_LANG
!     /* check for edb-psql first because we need to detect EDB AS */
!     if (check_exec(ctx, cluster->bindir, "edb-psql", "psql") == 1)
!     {
!         cluster->psql_exe = "edb-psql";
!         cluster->is_edb_as = true;
!     }
!     else
! #else
!     if (check_exec(ctx, cluster->bindir, "psql", NULL) == 1)
! #endif
!         cluster->psql_exe = "psql";
  }


--- 88,97 ----
  static void
  checkBinDir(migratorContext *ctx, ClusterInfo *cluster)
  {
!     check_exec(ctx, cluster->bindir, "postgres");
!     check_exec(ctx, cluster->bindir, "psql");
!     check_exec(ctx, cluster->bindir, "pg_ctl");
!     check_exec(ctx, cluster->bindir, "pg_dumpall");
  }


***************
*** 146,153 ****
   *    a valid executable, this function returns 0 to indicated failure.
   */
  static int
! check_exec(migratorContext *ctx, const char *dir, const char *cmdName,
!            const char *alternative)
  {
      char        path[MAXPGPATH];
      const char *errMsg;
--- 133,139 ----
   *    a valid executable, this function returns 0 to indicated failure.
   */
  static int
! check_exec(migratorContext *ctx, const char *dir, const char *cmdName)
  {
      char        path[MAXPGPATH];
      const char *errMsg;
***************
*** 155,175 ****
      snprintf(path, sizeof(path), "%s%c%s", dir, pathSeparator, cmdName);

      if ((errMsg = validate_exec(path)) == NULL)
-     {
          return 1;                /* 1 -> first alternative OK */
-     }
      else
!     {
!         if (alternative)
!         {
!             report_status(ctx, PG_WARNING, "check for %s warning:  %s",
!                           cmdName, errMsg);
!             if (check_exec(ctx, dir, alternative, NULL) == 1)
!                 return 2;        /* 2 -> second alternative OK */
!         }
!         else
!             pg_log(ctx, PG_FATAL, "check for %s failed - %s\n", cmdName, errMsg);
!     }

      return 0;                    /* 0 -> neither alternative is acceptable */
  }
--- 141,149 ----
      snprintf(path, sizeof(path), "%s%c%s", dir, pathSeparator, cmdName);

      if ((errMsg = validate_exec(path)) == NULL)
          return 1;                /* 1 -> first alternative OK */
      else
!         pg_log(ctx, PG_FATAL, "check for %s failed - %s\n", cmdName, errMsg);

      return 0;                    /* 0 -> neither alternative is acceptable */
  }
Index: contrib/pg_upgrade/file.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/file.c,v
retrieving revision 1.3
diff -c -c -r1.3 file.c
*** contrib/pg_upgrade/file.c    13 May 2010 22:07:42 -0000    1.3
--- contrib/pg_upgrade/file.c    13 May 2010 22:48:06 -0000
***************
*** 9,18 ****
  #include <sys/types.h>
  #include <fcntl.h>

- #ifdef EDB_NATIVE_LANG
- #include <fcntl.h>
- #endif
-
  #ifdef WIN32
  #include <windows.h>
  #endif
--- 9,14 ----
Index: contrib/pg_upgrade/info.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/info.c,v
retrieving revision 1.2
diff -c -c -r1.2 info.c
*** contrib/pg_upgrade/info.c    12 May 2010 11:07:24 -0000    1.2
--- contrib/pg_upgrade/info.c    13 May 2010 22:48:06 -0000
***************
*** 291,298 ****
                RelInfoArr *relarr, Cluster whichCluster)
  {
      PGconn       *conn = connectToServer(ctx, dbinfo->db_name, whichCluster);
-     bool        is_edb_as = (whichCluster == CLUSTER_OLD) ?
-                     ctx->old.is_edb_as : ctx->new.is_edb_as;
      PGresult   *res;
      RelInfo    *relinfos;
      int            ntups;
--- 291,296 ----
***************
*** 341,378 ****
               FirstNormalObjectId,
      /* see the comment at the top of v8_3_create_sequence_script() */
               (GET_MAJOR_VERSION(ctx->old.major_version) <= 803) ?
!              "" : " OR relkind = 'S'",
!
!     /*
!      * EDB AS installs pgagent by default via initdb. We have to ignore it,
!      * and not migrate any old table contents.
!      */
!              (is_edb_as && strcmp(dbinfo->db_name, "edb") == 0) ?
!              "     AND "
!              "    n.nspname != 'pgagent' AND "
!     /* skip pgagent TOAST tables */
!              "    c.oid NOT IN "
!              "    ( "
!              "        SELECT c2.reltoastrelid "
!              "        FROM pg_catalog.pg_class c2 JOIN "
!              "                pg_catalog.pg_namespace n2 "
!              "            ON c2.relnamespace = n2.oid "
!              "        WHERE n2.nspname = 'pgagent' AND "
!              "              c2.reltoastrelid != 0 "
!              "    ) AND "
!     /* skip pgagent TOAST table indexes */
!              "    c.oid NOT IN "
!              "    ( "
!              "        SELECT c3.reltoastidxid "
!              "        FROM pg_catalog.pg_class c2 JOIN "
!              "                pg_catalog.pg_namespace n2 "
!              "            ON c2.relnamespace = n2.oid JOIN "
!              "                pg_catalog.pg_class c3 "
!              "            ON c2.reltoastrelid = c3.oid "
!              "        WHERE n2.nspname = 'pgagent' AND "
!              "              c2.reltoastrelid != 0 AND "
!              "              c3.reltoastidxid != 0 "
!              "    ) " : "");

      res = executeQueryOrDie(ctx, conn, query);

--- 339,345 ----
               FirstNormalObjectId,
      /* see the comment at the top of v8_3_create_sequence_script() */
               (GET_MAJOR_VERSION(ctx->old.major_version) <= 803) ?
!              "" : " OR relkind = 'S'");

      res = executeQueryOrDie(ctx, conn, query);

Index: contrib/pg_upgrade/pg_upgrade.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/pg_upgrade.c,v
retrieving revision 1.1
diff -c -c -r1.1 pg_upgrade.c
*** contrib/pg_upgrade/pg_upgrade.c    12 May 2010 02:19:11 -0000    1.1
--- contrib/pg_upgrade/pg_upgrade.c    13 May 2010 22:48:06 -0000
***************
*** 196,205 ****
       */
      prep_status(ctx, "Creating databases in the new cluster");
      exec_prog(ctx, true,
!               SYSTEMQUOTE "\"%s/%s\" --set ON_ERROR_STOP=on --port %d "
                "-f \"%s/%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
!               ctx->new.bindir, ctx->new.psql_exe, ctx->new.port,
!               ctx->output_dir, GLOBALS_DUMP_FILE, ctx->logfile);
      check_ok(ctx);

      get_db_and_rel_infos(ctx, &ctx->new.dbarr, CLUSTER_NEW);
--- 196,205 ----
       */
      prep_status(ctx, "Creating databases in the new cluster");
      exec_prog(ctx, true,
!               SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on --port %d "
                "-f \"%s/%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
!               ctx->new.bindir, ctx->new.port, ctx->output_dir,
!               GLOBALS_DUMP_FILE, ctx->logfile);
      check_ok(ctx);

      get_db_and_rel_infos(ctx, &ctx->new.dbarr, CLUSTER_NEW);
***************
*** 218,227 ****

      prep_status(ctx, "Restoring database schema to new cluster");
      exec_prog(ctx, true,
!               SYSTEMQUOTE "\"%s/%s\" --set ON_ERROR_STOP=on --port %d "
                "-f \"%s/%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
!               ctx->new.bindir, ctx->new.psql_exe, ctx->new.port,
!               ctx->output_dir, DB_DUMP_FILE, ctx->logfile);
      check_ok(ctx);

      /* regenerate now that we have db schemas */
--- 218,227 ----

      prep_status(ctx, "Restoring database schema to new cluster");
      exec_prog(ctx, true,
!               SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on --port %d "
                "-f \"%s/%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
!               ctx->new.bindir, ctx->new.port, ctx->output_dir,
!               DB_DUMP_FILE, ctx->logfile);
      check_ok(ctx);

      /* regenerate now that we have db schemas */
Index: contrib/pg_upgrade/pg_upgrade.h
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/pg_upgrade.h,v
retrieving revision 1.3
diff -c -c -r1.3 pg_upgrade.h
*** contrib/pg_upgrade/pg_upgrade.h    13 May 2010 15:58:15 -0000    1.3
--- contrib/pg_upgrade/pg_upgrade.h    13 May 2010 22:48:06 -0000
***************
*** 200,214 ****
      DbInfoArr    dbarr;        /* dbinfos array */
      char       *pgdata;        /* pathname for cluster's $PGDATA directory */
      char       *bindir;        /* pathname for cluster's executable directory */
-     const char *psql_exe;    /* name of the psql command to execute
-                              * in the cluster */
      unsigned short port;    /* port number where postmaster is waiting */
      uint32        major_version;        /* PG_VERSION of cluster */
      char       *major_version_str;    /* string PG_VERSION of cluster */
      Oid            pg_database_oid;        /* OID of pg_database relation */
      char       *libpath;    /* pathname for cluster's pkglibdir */
-     /* EDB AS is PG 8.2 with 8.3 enhancements backpatched. */
-     bool        is_edb_as;    /* EnterpriseDB's Postgres Plus Advanced Server? */
      char       *tablespace_suffix;    /* directory specification */
  } ClusterInfo;

--- 200,210 ----
Index: contrib/pg_upgrade/relfilenode.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/relfilenode.c,v
retrieving revision 1.2
diff -c -c -r1.2 relfilenode.c
*** contrib/pg_upgrade/relfilenode.c    12 May 2010 16:50:00 -0000    1.2
--- contrib/pg_upgrade/relfilenode.c    13 May 2010 22:48:07 -0000
***************
*** 6,15 ****

  #include "pg_upgrade.h"

- #ifdef EDB_NATIVE_LANG
- #include <fcntl.h>
- #endif
-
  #include "catalog/pg_class.h"
  #include "access/transam.h"

--- 6,11 ----
Index: doc/src/sgml/pgupgrade.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/pgupgrade.sgml,v
retrieving revision 1.3
diff -c -c -r1.3 pgupgrade.sgml
*** doc/src/sgml/pgupgrade.sgml    13 May 2010 15:03:24 -0000    1.3
--- doc/src/sgml/pgupgrade.sgml    13 May 2010 22:48:07 -0000
***************
*** 23,33 ****
     pg_upgrade supports upgrades from 8.3.X and later to the current
     major release of Postgres, including snapshot and alpha releases.

- <!--
-    pg_upgrade also supports upgrades from EnterpriseDB's Postgres Plus
-    Advanced Server.
- -->
-
    </para>

   </sect2>
--- 23,28 ----
***************
*** 120,144 ****
       start the new cluster.
      </para>

- <!--
-     <para>
-      If migrating EnterpriseDB's Postgres Plus Advanced Server, you must:
-      <itemizedlist>
-       <listitem>
-        <para>
-         <emphasis>not</> install <literal>sample tables and procedures/functions</>
-         in the new server
-        </para>
-       </listitem>
-       <listitem>
-        <para>
-         delete the empty <literal>edb</> schema in the <literal>enterprisedb</> database
-        </para>
-       </listitem>
-      </itemizedlist>
-     </para>
- -->
-
     </listitem>

     <listitem>
--- 115,120 ----