On Thu, Oct 09, 2025 at 11:22:47AM -0700, Masahiko Sawada wrote: > +1, sounds like a good idea to improve user experience. I think we can > use the API in pg_combinebackup.c too since it has a similar function, > read_pg_version_file().
Please find attached what I am finishing with. At the end, I have designed an API that can be reused across the board for the following tools that do the same things in the tree, removing some duplicated code: - pg_resetwal - pg_upgrade - pg_combinebackup The routine that retrieves the contents gets a uint32 number, and it is optionally possible to get the contents of PG_VERSION (pg_upgrade wants that for tablespace paths, but that's really for pg_resetwal to be able to show back buggy data): extern uint32 get_pg_version(const char *datadir, char **version_str); This support both the pre-v10 and the post-v10 version formats, for pg_upgrade. To ease comparisons with PG_MAJORVERSION_NUM, I have added a small helper macro (see GET_PG_MAJORVERSION_NUM). I have also applied the same method to pg_createsubscriber, on top of that, to take care of my original issue. I have not looked at other places where the same concept could be applied, at least it's a start. Thoughts or comments? -- Michael
From b856d200e52ccb3d841ba05df149173cb7cd8772 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Fri, 10 Oct 2025 15:01:57 +0900 Subject: [PATCH v1 1/5] Introduce API able to retrieve contents of PG_VERSION This will be used by a set of follow-up patches, to be consumed in various frontend tools. --- src/include/fe_utils/version.h | 23 ++++++++++ src/fe_utils/Makefile | 3 +- src/fe_utils/meson.build | 1 + src/fe_utils/version.c | 81 ++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 src/include/fe_utils/version.h create mode 100644 src/fe_utils/version.c diff --git a/src/include/fe_utils/version.h b/src/include/fe_utils/version.h new file mode 100644 index 000000000000..3b8c1884de5f --- /dev/null +++ b/src/include/fe_utils/version.h @@ -0,0 +1,23 @@ +/*------------------------------------------------------------------------- + * + * Routines to retrieve information of PG_VERSION + * + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/fe_utils/version.h + * + *------------------------------------------------------------------------- + */ +#ifndef VERSION_H +#define VERSION_H + +/* + * Retrieve the version major number, useful for major version checks + * based on PG_MAJORVERSION_NUM. + */ +#define GET_PG_MAJORVERSION_NUM(v) ((v) / 10000) + +extern uint32 get_pg_version(const char *datadir, char **version_str); + +#endif /* VERSION_H */ diff --git a/src/fe_utils/Makefile b/src/fe_utils/Makefile index 28196ce0f6ae..b9fd566ff873 100644 --- a/src/fe_utils/Makefile +++ b/src/fe_utils/Makefile @@ -37,7 +37,8 @@ OBJS = \ query_utils.o \ recovery_gen.o \ simple_list.o \ - string_utils.o + string_utils.o \ + version.o ifeq ($(PORTNAME), win32) override CPPFLAGS += -DFD_SETSIZE=1024 diff --git a/src/fe_utils/meson.build b/src/fe_utils/meson.build index 5a9ddb73463e..ddac3c3a6584 100644 --- a/src/fe_utils/meson.build +++ b/src/fe_utils/meson.build @@ -18,6 +18,7 @@ fe_utils_sources = files( 'recovery_gen.c', 'simple_list.c', 'string_utils.c', + 'version.c', ) psqlscan = custom_target('psqlscan', diff --git a/src/fe_utils/version.c b/src/fe_utils/version.c new file mode 100644 index 000000000000..8c06bb84b4d6 --- /dev/null +++ b/src/fe_utils/version.c @@ -0,0 +1,81 @@ +/*------------------------------------------------------------------------- + * + * version.c + * Routine to retrieve information of PG_VERSION + * + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + *------------------------------------------------------------------------- + */ + +#include "postgres_fe.h" + +#include <sys/stat.h> + +#include "common/logging.h" +#include "fe_utils/version.h" + +/* + * Assumed maximum size of PG_VERSION. This should be more than enough for + * any version numbers that need to be handled. + */ +#define PG_VERSION_MAX_SIZE 64 + +/* + * get_major_version + * + * Retrieve the major version number of the given data folder, from + * PG_VERSION. The result returned is a version number, that can be used + * for comparisons based on PG_VERSION_NUM. For example, if PG_VERSION + * contains "18\n", this function returns 180000. + * + * This supports both the pre-v10 and the post-v10 version numbering. + * Optionally. "version_str" can be specified to store the contents + * retrieved from PG_VERSION. It is allocated by this routine; the + * caller is responsible for pg_free()-ing it. + */ +uint32 +get_pg_version(const char *datadir, char **version_str) +{ + FILE *version_fd; + char ver_filename[MAXPGPATH]; + char buf[PG_VERSION_MAX_SIZE]; + int v1 = 0, + v2 = 0; + struct stat st; + + snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION", + datadir); + + if ((version_fd = fopen(ver_filename, "r")) == NULL) + pg_fatal("could not open version file \"%s\": %m", ver_filename); + + if (fstat(fileno(version_fd), &st) != 0) + pg_fatal("could not stat file \"%s\": %m", ver_filename); + if (st.st_size > PG_VERSION_MAX_SIZE) + pg_fatal("file \"%s\" is too large", ver_filename); + + if (fscanf(version_fd, "%63s", buf) == 0 || + sscanf(buf, "%d.%d", &v1, &v2) < 1) + pg_fatal("could not parse version file \"%s\"", ver_filename); + + fclose(version_fd); + + if (version_str) + { + *version_str = pg_malloc(PG_VERSION_MAX_SIZE); + memcpy(*version_str, buf, st.st_size); + } + + if (v1 < 10) + { + /* pre-v10 style, e.g. 9.6.1 */ + return v1 * 10000 + v2 * 100; + } + else + { + /* post-v10 style, e.g. 10.1 */ + return v1 * 10000; + } +} -- 2.51.0
From 80fe5aa1bcc36a12969a3936e4c18cf374ab5b90 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Fri, 10 Oct 2025 15:03:32 +0900 Subject: [PATCH v1 2/5] pg_upgrade: Use PG_VERSION generic routine --- src/bin/pg_upgrade/exec.c | 5 +++-- src/bin/pg_upgrade/pg_upgrade.h | 3 +-- src/bin/pg_upgrade/server.c | 39 --------------------------------- 3 files changed, 4 insertions(+), 43 deletions(-) diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index 63f2815a7cd1..c045633d0c2a 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -12,6 +12,7 @@ #include <fcntl.h> #include "common/string.h" +#include "fe_utils/version.h" #include "pg_upgrade.h" static void check_data_dir(ClusterInfo *cluster); @@ -343,8 +344,8 @@ check_data_dir(ClusterInfo *cluster) const char *pg_data = cluster->pgdata; /* get the cluster version */ - cluster->major_version = get_major_server_version(cluster); - + cluster->major_version = get_pg_version(cluster->pgdata, + &cluster->major_version_str); check_single_dir(pg_data, ""); check_single_dir(pg_data, "base"); check_single_dir(pg_data, "global"); diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 0ef47be0dc19..e86336f4be95 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -298,7 +298,7 @@ typedef struct char *sockdir; /* directory for Unix Domain socket, if any */ unsigned short port; /* port number where postmaster is waiting */ uint32 major_version; /* PG_VERSION of cluster */ - char major_version_str[64]; /* string PG_VERSION of cluster */ + char *major_version_str; /* string PG_VERSION of cluster */ uint32 bin_version; /* version returned from pg_ctl */ char **tablespaces; /* tablespace directories */ int num_tablespaces; @@ -473,7 +473,6 @@ char *cluster_conn_opts(ClusterInfo *cluster); bool start_postmaster(ClusterInfo *cluster, bool report_and_exit_on_error); void stop_postmaster(bool in_atexit); -uint32 get_major_server_version(ClusterInfo *cluster); void check_pghost_envvar(void); diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c index 7eb15bc7d5ac..d51bee885b82 100644 --- a/src/bin/pg_upgrade/server.c +++ b/src/bin/pg_upgrade/server.c @@ -148,45 +148,6 @@ executeQueryOrDie(PGconn *conn, const char *fmt,...) } -/* - * get_major_server_version() - * - * gets the version (in unsigned int form) for the given datadir. Assumes - * that datadir is an absolute path to a valid pgdata directory. The version - * is retrieved by reading the PG_VERSION file. - */ -uint32 -get_major_server_version(ClusterInfo *cluster) -{ - FILE *version_fd; - char ver_filename[MAXPGPATH]; - int v1 = 0, - v2 = 0; - - snprintf(ver_filename, sizeof(ver_filename), "%s/PG_VERSION", - cluster->pgdata); - if ((version_fd = fopen(ver_filename, "r")) == NULL) - pg_fatal("could not open version file \"%s\": %m", ver_filename); - - if (fscanf(version_fd, "%63s", cluster->major_version_str) == 0 || - sscanf(cluster->major_version_str, "%d.%d", &v1, &v2) < 1) - pg_fatal("could not parse version file \"%s\"", ver_filename); - - fclose(version_fd); - - if (v1 < 10) - { - /* old style, e.g. 9.6.1 */ - return v1 * 10000 + v2 * 100; - } - else - { - /* new style, e.g. 10.1 */ - return v1 * 10000; - } -} - - static void stop_postmaster_atexit(void) { -- 2.51.0
From 2cd799e94b8453f7fae4134e37de78328719d411 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Fri, 10 Oct 2025 15:04:03 +0900 Subject: [PATCH v1 3/5] pg_createsubscriber: Use PG_VERSION generic routine --- src/bin/pg_basebackup/pg_createsubscriber.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index d29407413d96..c3732adabc86 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -26,6 +26,7 @@ #include "fe_utils/recovery_gen.h" #include "fe_utils/simple_list.h" #include "fe_utils/string_utils.h" +#include "fe_utils/version.h" #include "getopt_long.h" #define DEFAULT_SUB_PORT "50432" @@ -407,7 +408,7 @@ static void check_data_directory(const char *datadir) { struct stat statbuf; - char versionfile[MAXPGPATH]; + uint32 major_version; pg_log_info("checking if directory \"%s\" is a cluster data directory", datadir); @@ -420,11 +421,18 @@ check_data_directory(const char *datadir) pg_fatal("could not access directory \"%s\": %m", datadir); } - snprintf(versionfile, MAXPGPATH, "%s/PG_VERSION", datadir); - if (stat(versionfile, &statbuf) != 0 && errno == ENOENT) + /* + * Retrieve the contents of this cluster's PG_VERSION. We require + * compatibility with the same major version as the one this tool is + * compiled with. + */ + major_version = GET_PG_MAJORVERSION_NUM(get_pg_version(datadir, NULL)); + if (major_version != PG_MAJORVERSION_NUM) { - pg_fatal("directory \"%s\" is not a database cluster directory", - datadir); + pg_log_error("data directory is of wrong version"); + pg_log_error_detail("File \"%s\" contains \"%u\", which is not compatible with this program's version \"%u\".", + "PG_VERSION", major_version, PG_MAJORVERSION_NUM); + exit(1); } } -- 2.51.0
From 241106d3f694403c75ae922446a2eb99e91a63f2 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Fri, 10 Oct 2025 15:04:29 +0900 Subject: [PATCH v1 4/5] pg_combinebackup: use PG_VERSION generic routine --- src/bin/pg_combinebackup/pg_combinebackup.c | 63 +++------------------ 1 file changed, 7 insertions(+), 56 deletions(-) diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index f5cef99f6273..1f0531154b1d 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -34,6 +34,7 @@ #include "common/relpath.h" #include "copy_file.h" #include "fe_utils/option_utils.h" +#include "fe_utils/version.h" #include "getopt_long.h" #include "lib/stringinfo.h" #include "load_manifest.h" @@ -117,7 +118,6 @@ static void process_directory_recursively(Oid tsoid, manifest_data **manifests, manifest_writer *mwriter, cb_options *opt); -static int read_pg_version_file(char *directory); static void remember_to_cleanup_directory(char *target_path, bool rmtopdir); static void reset_directory_cleanup_list(void); static cb_tablespace *scan_for_existing_tablespaces(char *pathname, @@ -153,7 +153,7 @@ main(int argc, char *argv[]) int c; int n_backups; int n_prior_backups; - int version; + uint32 version; uint64 system_identifier; char **prior_backup_dirs; cb_options opt; @@ -271,7 +271,11 @@ main(int argc, char *argv[]) } /* Read the server version from the final backup. */ - version = read_pg_version_file(argv[argc - 1]); + version = get_pg_version(argv[argc - 1], NULL); + if (GET_PG_MAJORVERSION_NUM(version) < 10) + pg_fatal("server version too old"); + pg_log_debug("read server version %u from file \"%s\"", + GET_PG_MAJORVERSION_NUM(version), "PG_VERSION"); /* Sanity-check control files. */ n_backups = argc - optind; @@ -1155,59 +1159,6 @@ process_directory_recursively(Oid tsoid, closedir(dir); } -/* - * Read the version number from PG_VERSION and convert it to the usual server - * version number format. (e.g. If PG_VERSION contains "14\n" this function - * will return 140000) - */ -static int -read_pg_version_file(char *directory) -{ - char filename[MAXPGPATH]; - StringInfoData buf; - int fd; - int version; - char *ep; - - /* Construct pathname. */ - snprintf(filename, MAXPGPATH, "%s/PG_VERSION", directory); - - /* Open file. */ - if ((fd = open(filename, O_RDONLY, 0)) < 0) - pg_fatal("could not open file \"%s\": %m", filename); - - /* Read into memory. Length limit of 128 should be more than generous. */ - initStringInfo(&buf); - slurp_file(fd, filename, &buf, 128); - - /* Close the file. */ - if (close(fd) != 0) - pg_fatal("could not close file \"%s\": %m", filename); - - /* Convert to integer. */ - errno = 0; - version = strtoul(buf.data, &ep, 10); - if (errno != 0 || *ep != '\n') - { - /* - * Incremental backup is not relevant to very old server versions that - * used multi-part version number (e.g. 9.6, or 8.4). So if we see - * what looks like the beginning of such a version number, just bail - * out. - */ - if (version < 10 && *ep == '.') - pg_fatal("%s: server version too old", filename); - pg_fatal("%s: could not parse version number", filename); - } - - /* Debugging output. */ - pg_log_debug("read server version %d from file \"%s\"", version, filename); - - /* Release memory and return result. */ - pfree(buf.data); - return version * 10000; -} - /* * Add a directory to the list of output directories to clean up. */ -- 2.51.0
From d5bd304ee6a0c236ff323cc5d43713f10c1b20e8 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Fri, 10 Oct 2025 15:04:47 +0900 Subject: [PATCH v1 5/5] pg_resetwal: use PG_VERSION generic routine --- src/bin/pg_resetwal/pg_resetwal.c | 36 +++++++++---------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index 7a4e4eb95706..42d15a2612d8 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -55,6 +55,7 @@ #include "common/restricted_token.h" #include "common/string.h" #include "fe_utils/option_utils.h" +#include "fe_utils/version.h" #include "getopt_long.h" #include "pg_getopt.h" #include "storage/large_object.h" @@ -77,7 +78,7 @@ static int WalSegSz; static int set_wal_segsize; static int set_char_signedness = -1; -static void CheckDataVersion(void); +static void CheckDataVersion(const char *datadir); static bool read_controlfile(void); static void GuessControlValues(void); static void PrintControlValues(bool guessed); @@ -377,7 +378,7 @@ main(int argc, char *argv[]) DataDir); /* Check that data directory matches our server version */ - CheckDataVersion(); + CheckDataVersion(DataDir); /* * Check for a postmaster lock file --- if there is one, refuse to @@ -537,37 +538,20 @@ main(int argc, char *argv[]) * to prevent simple user errors. */ static void -CheckDataVersion(void) +CheckDataVersion(const char *datadir) { - const char *ver_file = "PG_VERSION"; - FILE *ver_fd; - char rawline[64]; + char *version_str; + uint32 version = get_pg_version(datadir, &version_str); - if ((ver_fd = fopen(ver_file, "r")) == NULL) - pg_fatal("could not open file \"%s\" for reading: %m", - ver_file); - - /* version number has to be the first line read */ - if (!fgets(rawline, sizeof(rawline), ver_fd)) - { - if (!ferror(ver_fd)) - pg_fatal("unexpected empty file \"%s\"", ver_file); - else - pg_fatal("could not read file \"%s\": %m", ver_file); - } - - /* strip trailing newline and carriage return */ - (void) pg_strip_crlf(rawline); - - if (strcmp(rawline, PG_MAJORVERSION) != 0) + if (GET_PG_MAJORVERSION_NUM(version) != PG_MAJORVERSION_NUM) { pg_log_error("data directory is of wrong version"); pg_log_error_detail("File \"%s\" contains \"%s\", which is not compatible with this program's version \"%s\".", - ver_file, rawline, PG_MAJORVERSION); + "PG_VERSION", + version_str, + PG_MAJORVERSION); exit(1); } - - fclose(ver_fd); } -- 2.51.0
signature.asc
Description: PGP signature
