Re: Use of PG_VERSION

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Is there a reason tablespace.c::set_short_version() uses PG_VERSION, and
> > not the simpler PG_MAJORVERSION?  initdb.c::get_short_version() does the
> > same thing.
>
> Probably that code predates the addition of the separate PG_MAJORVERSION
> #define.  +1 for simplifying.  The change I just had to make in
> backend/catalog/Makefile reinforces the thought that MAJORVERSION is
> what we should be using in everyplace related to this type of check.

Agreed.  Fixed the attached, applied patch.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/commands/tablespace.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablespace.c,v
retrieving revision 1.67
diff -c -c -r1.67 tablespace.c
*** src/backend/commands/tablespace.c    6 Jan 2010 01:48:09 -0000    1.67
--- src/backend/commands/tablespace.c    6 Jan 2010 23:22:40 -0000
***************
*** 82,88 ****


  static bool remove_tablespace_directories(Oid tablespaceoid, bool redo);
! static void set_short_version(const char *path);


  /*
--- 82,88 ----


  static bool remove_tablespace_directories(Oid tablespaceoid, bool redo);
! static void write_version_file(const char *path);


  /*
***************
*** 332,338 ****
       * (the emptiness check above will fail), and to label tablespace
       * directories by PG version.
       */
!     set_short_version(location);

      /*
       * All seems well, create the symlink
--- 332,338 ----
       * (the emptiness check above will fail), and to label tablespace
       * directories by PG version.
       */
!     write_version_file(location);

      /*
       * All seems well, create the symlink
***************
*** 673,718 ****
   * write out the PG_VERSION file in the specified directory
   */
  static void
! set_short_version(const char *path)
  {
-     char       *short_version;
-     bool        gotdot = false;
-     int            end;
      char       *fullname;
      FILE       *version_file;

-     /* Construct short version string (should match initdb.c) */
-     short_version = pstrdup(PG_VERSION);
-
-     for (end = 0; short_version[end] != '\0'; end++)
-     {
-         if (short_version[end] == '.')
-         {
-             Assert(end != 0);
-             if (gotdot)
-                 break;
-             else
-                 gotdot = true;
-         }
-         else if (short_version[end] < '0' || short_version[end] > '9')
-         {
-             /* gone past digits and dots */
-             break;
-         }
-     }
-     Assert(end > 0 && short_version[end - 1] != '.' && gotdot);
-     short_version[end] = '\0';
-
      /* Now write the file */
      fullname = palloc(strlen(path) + 11 + 1);
      sprintf(fullname, "%s/PG_VERSION", path);
!     version_file = AllocateFile(fullname, PG_BINARY_W);
!     if (version_file == NULL)
          ereport(ERROR,
                  (errcode_for_file_access(),
                   errmsg("could not write to file \"%s\": %m",
                          fullname)));
!     fprintf(version_file, "%s\n", short_version);
      if (FreeFile(version_file))
          ereport(ERROR,
                  (errcode_for_file_access(),
--- 673,693 ----
   * write out the PG_VERSION file in the specified directory
   */
  static void
! write_version_file(const char *path)
  {
      char       *fullname;
      FILE       *version_file;

      /* Now write the file */
      fullname = palloc(strlen(path) + 11 + 1);
      sprintf(fullname, "%s/PG_VERSION", path);
!
!     if ((version_file = AllocateFile(fullname, PG_BINARY_W)) == NULL)
          ereport(ERROR,
                  (errcode_for_file_access(),
                   errmsg("could not write to file \"%s\": %m",
                          fullname)));
!     fprintf(version_file, "%s\n", PG_MAJORVERSION);
      if (FreeFile(version_file))
          ereport(ERROR,
                  (errcode_for_file_access(),
***************
*** 720,726 ****
                          fullname)));

      pfree(fullname);
-     pfree(short_version);
  }

  /*
--- 695,700 ----
***************
*** 1370,1376 ****
                           location)));

          /* Create or re-create the PG_VERSION file in the target directory */
!         set_short_version(location);

          /* Create the symlink if not already present */
          linkloc = (char *) palloc(OIDCHARS + OIDCHARS + 1);
--- 1344,1350 ----
                           location)));

          /* Create or re-create the PG_VERSION file in the target directory */
!         write_version_file(location);

          /* Create the symlink if not already present */
          linkloc = (char *) palloc(OIDCHARS + OIDCHARS + 1);
Index: src/bin/initdb/initdb.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.181
diff -c -c -r1.181 initdb.c
*** src/bin/initdb/initdb.c    2 Jan 2010 16:57:58 -0000    1.181
--- src/bin/initdb/initdb.c    6 Jan 2010 23:22:41 -0000
***************
*** 156,171 ****
  static void exit_nicely(void);
  static char *get_id(void);
  static char *get_encoding_id(char *encoding_name);
- static char *get_short_version(void);
  static int    check_data_dir(char *dir);
  static bool mkdatadir(const char *subdir);
  static void set_input(char **dest, char *filename);
  static void check_input(char *path);
! static void set_short_version(char *short_version, char *extrapath);
  static void set_null_conf(void);
  static void test_config_settings(void);
  static void setup_config(void);
! static void bootstrap_template1(char *short_version);
  static void setup_auth(void);
  static void get_set_pwd(void);
  static void setup_depend(void);
--- 156,170 ----
  static void exit_nicely(void);
  static char *get_id(void);
  static char *get_encoding_id(char *encoding_name);
  static int    check_data_dir(char *dir);
  static bool mkdatadir(const char *subdir);
  static void set_input(char **dest, char *filename);
  static void check_input(char *path);
! static void write_version_file(char *extrapath);
  static void set_null_conf(void);
  static void test_config_settings(void);
  static void setup_config(void);
! static void bootstrap_template1(void);
  static void setup_auth(void);
  static void get_set_pwd(void);
  static void setup_depend(void);
***************
*** 803,844 ****


  /*
-  * get short version of VERSION
-  */
- static char *
- get_short_version(void)
- {
-     bool        gotdot = false;
-     int            end;
-     char       *vr;
-
-     vr = xstrdup(PG_VERSION);
-
-     for (end = 0; vr[end] != '\0'; end++)
-     {
-         if (vr[end] == '.')
-         {
-             if (end == 0)
-                 return NULL;
-             else if (gotdot)
-                 break;
-             else
-                 gotdot = true;
-         }
-         else if (vr[end] < '0' || vr[end] > '9')
-         {
-             /* gone past digits and dots */
-             break;
-         }
-     }
-     if (end == 0 || vr[end - 1] == '.' || !gotdot)
-         return NULL;
-
-     vr[end] = '\0';
-     return vr;
- }
-
- /*
   * make sure the directory either doesn't exist or is empty
   *
   * Returns 0 if nonexistent, 1 if exists and empty, 2 if not empty,
--- 802,807 ----
***************
*** 972,978 ****
   * if extrapath is not NULL
   */
  static void
! set_short_version(char *short_version, char *extrapath)
  {
      FILE       *version_file;
      char       *path;
--- 935,941 ----
   * if extrapath is not NULL
   */
  static void
! write_version_file(char *extrapath)
  {
      FILE       *version_file;
      char       *path;
***************
*** 987,1000 ****
          path = pg_malloc(strlen(pg_data) + strlen(extrapath) + 13);
          sprintf(path, "%s/%s/PG_VERSION", pg_data, extrapath);
      }
!     version_file = fopen(path, PG_BINARY_W);
!     if (version_file == NULL)
      {
          fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
                  progname, path, strerror(errno));
          exit_nicely();
      }
!     if (fprintf(version_file, "%s\n", short_version) < 0 ||
          fclose(version_file))
      {
          fprintf(stderr, _("%s: could not write file \"%s\": %s\n"),
--- 950,963 ----
          path = pg_malloc(strlen(pg_data) + strlen(extrapath) + 13);
          sprintf(path, "%s/%s/PG_VERSION", pg_data, extrapath);
      }
!
!     if ((version_file = fopen(path, PG_BINARY_W)) == NULL)
      {
          fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
                  progname, path, strerror(errno));
          exit_nicely();
      }
!     if (fprintf(version_file, "%s\n", PG_MAJORVERSION) < 0 ||
          fclose(version_file))
      {
          fprintf(stderr, _("%s: could not write file \"%s\": %s\n"),
***************
*** 1297,1303 ****
   * run the BKI script in bootstrap mode to create template1
   */
  static void
! bootstrap_template1(char *short_version)
  {
      PG_CMD_DECL;
      char      **line;
--- 1260,1266 ----
   * run the BKI script in bootstrap mode to create template1
   */
  static void
! bootstrap_template1(void)
  {
      PG_CMD_DECL;
      char      **line;
***************
*** 1317,1323 ****
      /* Check that bki file appears to be of the right version */

      snprintf(headerline, sizeof(headerline), "# PostgreSQL %s\n",
!              short_version);

      if (strcmp(headerline, *bki_lines) != 0)
      {
--- 1280,1286 ----
      /* Check that bki file appears to be of the right version */

      snprintf(headerline, sizeof(headerline), "# PostgreSQL %s\n",
!              PG_MAJORVERSION);

      if (strcmp(headerline, *bki_lines) != 0)
      {
***************
*** 2480,2486 ****
                  i,
                  ret;
      int            option_index;
-     char       *short_version;
      char       *effective_user;
      char       *pgdenv;            /* PGDATA value gotten from and sent to
                                   * environment */
--- 2443,2448 ----
***************
*** 2788,2799 ****

      canonicalize_path(share_path);

-     if ((short_version = get_short_version()) == NULL)
-     {
-         fprintf(stderr, _("%s: could not determine valid short version string\n"), progname);
-         exit(1);
-     }
-
      effective_user = get_id();
      if (strlen(username) == 0)
          username = effective_user;
--- 2750,2755 ----
***************
*** 3123,3129 ****
      check_ok();

      /* Top level PG_VERSION is checked by bootstrapper, so make it first */
!     set_short_version(short_version, NULL);

      /* Select suitable configuration settings */
      set_null_conf();
--- 3079,3085 ----
      check_ok();

      /* Top level PG_VERSION is checked by bootstrapper, so make it first */
!     write_version_file(NULL);

      /* Select suitable configuration settings */
      set_null_conf();
***************
*** 3133,3144 ****
      setup_config();

      /* Bootstrap template1 */
!     bootstrap_template1(short_version);

      /*
       * Make the per-database PG_VERSION for template1 only after init'ing it
       */
!     set_short_version(short_version, "base/1");

      /* Create the stuff we don't need to use bootstrap mode for */

--- 3089,3100 ----
      setup_config();

      /* Bootstrap template1 */
!     bootstrap_template1();

      /*
       * Make the per-database PG_VERSION for template1 only after init'ing it
       */
!     write_version_file("base/1");

      /* Create the stuff we don't need to use bootstrap mode for */