From 3117260f6ccdb23bab2dd836e56b0bc3f748795e Mon Sep 17 00:00:00 2001 From: Bohyun Lee Date: Mon, 17 Aug 2026 13:13:59 +0200 Subject: [PATCH] doc: pg_upgrade: state which initdb settings must match the old cluster The manual "Initialize the new cluster" step said only to use "compatible initdb flags that match the old cluster", which is vague. Spell out the two settings pg_upgrade actually enforces through its control-data check: the WAL segment size (--wal-segsize) and the data checksum setting (--data-checksums/--no-data-checksums), and point to pg_controldata for reading the old cluster's values. Note that the encoding and locale need not match, since pg_upgrade copies them from the old cluster's template0. --- doc/src/sgml/ref/pgupgrade.sgml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index e4e8c02e6d67f..d5c223968358e 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -457,10 +457,16 @@ make prefix=/usr/local/pgsql.new install Initialize the new cluster using initdb. - Again, use compatible initdb - flags that match the old cluster. Many - prebuilt installers do this step automatically. There is no need to - start the new cluster. + Two initdb settings must match the old cluster, or + pg_upgrade will fail with a control-data + mismatch: the WAL segment size () and the + data checksum setting ( or + ). You can read the old cluster's + values with + pg_controldata. + (The encoding and locale need not match: pg_upgrade + copies them from the old cluster.) Many prebuilt installers do this step + automatically. There is no need to start the new cluster. From ef20745f125c16034de12265f83cf39bf6622c6b Mon Sep 17 00:00:00 2001 From: Bohyun Lee Date: Mon, 17 Aug 2026 13:15:47 +0200 Subject: [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --initdb has pg_upgrade run initdb itself before upgrading, deriving the new cluster's WAL segment size, data checksum setting, encoding, and locale from the old cluster so it passes the control-data check. The initdb command is assembled with appendShellString() so untrusted values cannot break out of their shell arguments. The target directory must be empty or not yet exist, and the old server must be shut down. Both are checked up front. On a later failure the newly created directory is removed so the upgrade can be retried. With --check, --initdb reports the command it would run without creating the cluster. Includes documentation and a TAP test. Builds on the v5 patch by Hüseyin Demir. --- doc/src/sgml/ref/pgupgrade.sgml | 53 ++++ src/bin/pg_upgrade/exec.c | 4 +- src/bin/pg_upgrade/info.c | 3 +- src/bin/pg_upgrade/option.c | 22 +- src/bin/pg_upgrade/pg_upgrade.c | 309 +++++++++++++++++++++- src/bin/pg_upgrade/pg_upgrade.h | 5 + src/bin/pg_upgrade/t/009_initdb_option.pl | 234 ++++++++++++++++ 7 files changed, 614 insertions(+), 16 deletions(-) create mode 100644 src/bin/pg_upgrade/t/009_initdb_option.pl diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index d5c223968358e..996c04206ba2b 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -262,6 +262,53 @@ PostgreSQL documentation + + + + + Create the new cluster automatically by running + initdb before upgrading, instead of requiring the + user to have created it manually. The WAL segment size and data + checksum setting are derived from the old cluster so the new cluster + passes pg_upgrade's control-data check; the + encoding and locale are matched to the old cluster's + template0. + + + The new cluster data directory specified with + / must be empty or + not yet exist; otherwise pg_upgrade exits + with an error. + + + When combined with /, + performs a dry run: it reports the + initdb command it would run and verifies that the new + cluster could be created (the new binaries' major version matches, the + target directory is empty, and the old cluster is reachable), but + does not create the new cluster. This is a lighter check than a plain + , which instead queries an already-created new + cluster. + + + cannot be combined with + /: those options are + passed to the new cluster's server process, which accepts a different + set of options than initdb. If you need to supply + such options, create the new cluster manually and omit + . + + + To discover the old cluster's encoding and locale, + briefly starts the old server in + binary-upgrade mode (which disables autovacuum) and stops it again + before creating the new cluster. If a compatibility check fails after + the new cluster has been created, its data directory is removed + automatically, so the upgrade can be retried without manual cleanup. + + + + @@ -468,6 +515,12 @@ make prefix=/usr/local/pgsql.new install copies them from the old cluster.) Many prebuilt installers do this step automatically. There is no need to start the new cluster. + + Alternatively, pass to + pg_upgrade to have it run + initdb automatically, deriving the required settings + from the old cluster. In that case this manual step can be skipped. + diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index a1bdbf373e385..e9d7f8a5789b0 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -17,7 +17,7 @@ static void check_data_dir(ClusterInfo *cluster); static void check_bin_dir(ClusterInfo *cluster, bool check_versions); -static void get_bin_version(ClusterInfo *cluster); +void get_bin_version(ClusterInfo *cluster); static void check_exec(const char *dir, const char *program, bool check_version); #ifdef WIN32 @@ -30,7 +30,7 @@ static int win32_check_directory_write_permissions(void); * * Fetch major version of binaries for cluster. */ -static void +void get_bin_version(ClusterInfo *cluster) { char cmd[MAXPGPATH], diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index 5c59cfb32e8b7..d66fc170fa018 100644 --- a/src/bin/pg_upgrade/info.c +++ b/src/bin/pg_upgrade/info.c @@ -21,7 +21,6 @@ static void create_rel_filename_map(const char *old_data, const char *new_data, static void report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db); static void free_db_and_rel_infos(DbInfoArr *db_arr); -static void get_template0_info(ClusterInfo *cluster); static void get_db_infos(ClusterInfo *cluster); static char *get_rel_infos_query(void); static void process_rel_infos(DbInfo *dbinfo, PGresult *res, void *arg); @@ -328,7 +327,7 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster) * Get information about template0, which will be copied from the old cluster * to the new cluster. */ -static void +void get_template0_info(ClusterInfo *cluster) { PGconn *conn = connectToServer(cluster, "template1"); diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index f01d2f92d9548..d05b2d913f410 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -63,6 +63,7 @@ parseCommandLine(int argc, char *argv[]) {"no-statistics", no_argument, NULL, 5}, {"set-char-signedness", required_argument, NULL, 6}, {"swap", no_argument, NULL, 7}, + {"initdb", no_argument, NULL, 8}, {NULL, 0, NULL, 0} }; @@ -234,6 +235,10 @@ parseCommandLine(int argc, char *argv[]) user_opts.transfer_mode = TRANSFER_MODE_SWAP; break; + case 8: + user_opts.initdb_new_cluster = true; + break; + default: fprintf(stderr, _("Try \"%s --help\" for more information.\n"), os_info.progname); @@ -244,6 +249,17 @@ parseCommandLine(int argc, char *argv[]) if (optind < argc) pg_fatal("too many command-line arguments (first is \"%s\")", argv[optind]); + /* + * -O passes options to the new cluster's postmaster, but with --initdb + * the new cluster is created by initdb, which accepts a different option + * set. Rather than guess which -O options initdb also understands, reject + * the combination and let the user create the cluster manually (without + * --initdb) if they need postmaster-only options. + */ + if (new_cluster.pgopts && user_opts.initdb_new_cluster) + pg_fatal("options %s and %s cannot be used together", + "-O/--new-options", "--initdb"); + if (!user_opts.sync_method) user_opts.sync_method = pg_strdup("fsync"); @@ -328,6 +344,8 @@ usage(void) printf(_(" --clone clone instead of copying files to new cluster\n")); printf(_(" --copy copy files to new cluster (default)\n")); printf(_(" --copy-file-range copy files to new cluster with copy_file_range\n")); + printf(_(" --initdb create the new cluster with initdb before\n" + " upgrading (settings derived from old cluster)\n")); printf(_(" --no-statistics do not import statistics from old cluster\n")); printf(_(" --set-char-signedness=OPTION set new cluster char signedness to \"signed\" or\n" " \"unsigned\"\n")); @@ -336,7 +354,9 @@ usage(void) printf(_(" -?, --help show this help, then exit\n")); printf(_("\n" "Before running pg_upgrade you must:\n" - " create a new database cluster (using the new version of initdb)\n" + " create a new database cluster (using the new version of initdb),\n" + " unless the --initdb option is given, in which case pg_upgrade\n" + " creates the new cluster for you\n" " shutdown the postmaster servicing the old cluster\n" " shutdown the postmaster servicing the new cluster\n")); printf(_("\n" diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index c0fadb3f31773..6588c6b5a3ee6 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -41,14 +41,18 @@ #include "postgres_fe.h" +#include #include #include "access/multixact.h" #include "catalog/pg_class_d.h" +#include "catalog/pg_collation_d.h" #include "common/file_perm.h" #include "common/logging.h" #include "common/restricted_token.h" #include "fe_utils/string_utils.h" +#include "fe_utils/version.h" +#include "mb/pg_wchar.h" #include "pg_upgrade.h" /* @@ -67,6 +71,10 @@ static void copy_xact_xlog_xid(void); static void set_frozenxids(void); static void make_outputdirs(char *pgdata); static void setup(char *argv0); +static void resolve_new_bindir(const char *argv0); +static void build_new_cluster_initdb_cmd(PQExpBuffer cmd); +static void create_new_cluster_via_initdb(void); +static void check_new_cluster_via_initdb(void); static void create_logical_replication_slots(void); static void create_conflict_detection_slot(void); @@ -74,6 +82,9 @@ ClusterInfo old_cluster, new_cluster; OSInfo os_info; +static bool new_cluster_created_by_initdb = false; +static bool initdb_cleanup_registered = false; + char *output_files[] = { SERVER_LOG_FILE, #ifdef WIN32 @@ -109,6 +120,11 @@ main(int argc, char **argv) adjust_data_dir(&old_cluster); adjust_data_dir(&new_cluster); + if (user_opts.check && user_opts.initdb_new_cluster) + check_new_cluster_via_initdb(); /* exits(0), never returns */ + else if (user_opts.initdb_new_cluster) + create_new_cluster_via_initdb(); + /* * Set mask based on PGDATA permissions, needed for the creation of the * output directories with correct permissions. @@ -145,6 +161,9 @@ main(int argc, char **argv) check_new_cluster(); report_clusters_compatible(); + /* Disarm orphan cleanup once we reach the point of no easy return. */ + new_cluster_created_by_initdb = false; + pg_log(PG_REPORT, "\n" "Performing Upgrade\n" @@ -358,6 +377,284 @@ make_outputdirs(char *pgdata) } +/* + * resolve_new_bindir() + * + * If new_cluster.bindir was not set by the user via -B, derive it from the + * path of the currently executing pg_upgrade binary. Safe to call more than + * once. + */ +static void +resolve_new_bindir(const char *argv0) +{ + if (!new_cluster.bindir) + { + char exec_path[MAXPGPATH]; + + if (find_my_exec(argv0, exec_path) < 0) + pg_fatal("%s: could not find own program executable", argv0); + /* Trim off program name and keep just the directory */ + *last_dir_separator(exec_path) = '\0'; + canonicalize_path(exec_path); + new_cluster.bindir = pg_strdup(exec_path); + } +} + + +/* + * new_cluster_cleanup_atexit() + * + * atexit() handler: remove the new cluster's data directory if --initdb + * created it but the run failed before reaching the point of no return. + * Does nothing unless new_cluster_created_by_initdb is set. + */ +static void +new_cluster_cleanup_atexit(void) +{ + if (!new_cluster_created_by_initdb) + return; + (void) rmtree(new_cluster.pgdata, true); +} + + +/* + * build_new_cluster_initdb_cmd() + * + * Shared helper for both the real --initdb path and the --check --initdb + * dry-run path. Starts the old cluster (in binary-upgrade mode, which + * disables autovacuum), queries template0 for encoding/locale settings, reads + * old cluster pg_control via get_control_data(), stops the old cluster, and + * populates 'cmd' with the initdb command-string needed to create the new + * cluster with matching settings. + * + * This helper does not execute the command; callers decide whether to + * exec_prog() it (real upgrade) or just report it (dry-run). + * + * Both callers must have already called adjust_data_dir(&new_cluster) and + * resolve_new_bindir() before calling this, to ensure new_cluster.pgdata + * and new_cluster.bindir are set. + * + * On return, log_opts.logdir points at a temporary directory used for the + * old-cluster start/stop and the later initdb run. Callers save and restore + * it around this helper. + */ +static void +build_new_cluster_initdb_cmd(PQExpBuffer cmd) +{ + DbLocaleInfo *locale; + const char *encoding_name; + char initdb_path[MAXPGPATH]; + char verfile[MAXPGPATH]; + char tmp_logdir[MAXPGPATH]; + struct stat st; + DIR *dir; + struct dirent *de; + + /* Verify initdb is present in the new cluster's bin directory. */ + snprintf(initdb_path, sizeof(initdb_path), "%s/initdb", new_cluster.bindir); + if (validate_exec(initdb_path) != 0) + pg_fatal("could not find \"initdb\" in \"%s\": %m\n" + "The --initdb option requires initdb to be present in the new cluster's bin directory.", + new_cluster.bindir); + + /* + * Refuse to run initdb into a directory that already exists and is not + * empty. If a later step fails, the cleanup handler removes the entire + * new data directory. Requiring it to be empty first ensures the cleanup + * never destroys files the user already had there. + */ + snprintf(verfile, sizeof(verfile), "%s/PG_VERSION", new_cluster.pgdata); + if (stat(verfile, &st) == 0) + pg_fatal("new cluster data directory \"%s\" already contains a database system; " + "--initdb requires an empty or nonexistent directory", + new_cluster.pgdata); + dir = opendir(new_cluster.pgdata); + if (dir) + { + while (errno = 0, (de = readdir(dir)) != NULL) + { + if (strcmp(de->d_name, ".") != 0 && + strcmp(de->d_name, "..") != 0) + pg_fatal("new cluster data directory \"%s\" is not empty; " + "--initdb requires an empty or nonexistent directory", + new_cluster.pgdata); + } + if (errno) + pg_fatal("could not read directory \"%s\": %m", new_cluster.pgdata); + closedir(dir); + } + else if (errno != ENOENT) + pg_fatal("could not open directory \"%s\": %m", new_cluster.pgdata); + + /* + * Validate the new binaries' version before touching disk, so a wrong + * --new-bindir fails before the new cluster is created and there is + * nothing to clean up. + */ + if (new_cluster.bin_version == 0) + get_bin_version(&new_cluster); + if (GET_PG_MAJORVERSION_NUM(new_cluster.bin_version) != + GET_PG_MAJORVERSION_NUM(PG_VERSION_NUM)) + pg_fatal("new cluster binaries are version %d, but pg_upgrade is version %d", + GET_PG_MAJORVERSION_NUM(new_cluster.bin_version), + GET_PG_MAJORVERSION_NUM(PG_VERSION_NUM)); + + old_cluster.major_version = get_pg_version(old_cluster.pgdata, + &old_cluster.major_version_str); + + /* + * The normal output directory does not exist yet, so use a temporary log + * directory next to the new data directory (writable, unlike the new bin + * directory) for initdb and the brief old-server start. + */ + snprintf(tmp_logdir, sizeof(tmp_logdir), "%s.initdb_log", new_cluster.pgdata); + if (mkdir(tmp_logdir, pg_dir_create_mode) < 0 && errno != EEXIST) + pg_fatal("could not create log directory \"%s\": %m", tmp_logdir); + log_opts.logdir = pg_strdup(tmp_logdir); + + if (!old_cluster.sockdir) + old_cluster.sockdir = user_opts.socketdir ? user_opts.socketdir : "."; + + /* + * The old server must be shut down. The template0 read below starts a + * postmaster on the old cluster, and get_control_data() runs pg_resetwal, + * both of which need exclusive access to the old data directory. A stale + * lock file is tolerated as setup() does. + */ + if (pid_lock_file_exists(old_cluster.pgdata)) + { + if (start_postmaster(&old_cluster, false)) + stop_postmaster(false); + else + pg_fatal("There seems to be a postmaster servicing the old cluster.\n" + "Please shutdown that postmaster and try again."); + } + + get_control_data(&old_cluster); + + prep_status("Examining old cluster settings"); + start_postmaster(&old_cluster, true); + get_template0_info(&old_cluster); + stop_postmaster(false); + check_ok(); + + locale = old_cluster.template0; + encoding_name = pg_encoding_to_char(locale->db_encoding); + + prep_status("Constructing new cluster initdb command"); + + initPQExpBuffer(cmd); + + /* + * Build the command with appendShellString() for every value that comes + * from outside our control: the username is from the command line, and + * the encoding and locale strings are read from the old cluster's + * template0. This prevents shell metacharacters in any of them from + * breaking out of their argument when the command is run through the + * shell. + */ + appendShellString(cmd, initdb_path); + appendPQExpBufferStr(cmd, " -N -D "); + appendShellString(cmd, new_cluster.pgdata); + appendPQExpBufferStr(cmd, " -U "); + appendShellString(cmd, os_info.user); + appendPQExpBuffer(cmd, " --wal-segsize=%u", + old_cluster.controldata.walseg / (1024 * 1024)); + + /* + * Pass --data-checksums or --no-data-checksums explicitly. Starting from + * PG18, initdb enables checksums by default, so we must mirror the old + * cluster's setting to avoid a mismatch that check_control_data() would + * reject. + */ + if (old_cluster.controldata.data_checksum_version != 0) + appendPQExpBufferStr(cmd, " --data-checksums"); + else + appendPQExpBufferStr(cmd, " --no-data-checksums"); + + appendPQExpBufferStr(cmd, " --encoding="); + appendShellString(cmd, encoding_name); + appendPQExpBufferStr(cmd, " --locale-provider="); + appendShellString(cmd, collprovider_name(locale->db_collprovider)); + appendPQExpBufferStr(cmd, " --lc-collate="); + appendShellString(cmd, locale->db_collate); + appendPQExpBufferStr(cmd, " --lc-ctype="); + appendShellString(cmd, locale->db_ctype); + + if (locale->db_locale) + { + if (locale->db_collprovider == COLLPROVIDER_ICU) + { + appendPQExpBufferStr(cmd, " --icu-locale="); + appendShellString(cmd, locale->db_locale); + } + else if (locale->db_collprovider == COLLPROVIDER_BUILTIN) + { + appendPQExpBufferStr(cmd, " --builtin-locale="); + appendShellString(cmd, locale->db_locale); + } + } + + check_ok(); +} + + +/* + * create_new_cluster_via_initdb() + * + * Create the new cluster for --initdb: build the initdb command with + * build_new_cluster_initdb_cmd() and execute it. The atexit cleanup is + * enabled just before execution, so a failure after initdb runs (but before + * the point of no return) removes the new directory. + */ +static void +create_new_cluster_via_initdb(void) +{ + PQExpBufferData cmd; + char *saved_logdir = log_opts.logdir; + + resolve_new_bindir(os_info.progname); + build_new_cluster_initdb_cmd(&cmd); + + prep_status("Creating new cluster with initdb"); + + if (!initdb_cleanup_registered) + { + atexit(new_cluster_cleanup_atexit); + initdb_cleanup_registered = true; + } + new_cluster_created_by_initdb = true; + exec_prog(UTILITY_LOG_FILE, NULL, true, true, "%s", cmd.data); + + termPQExpBuffer(&cmd); + log_opts.logdir = saved_logdir; + check_ok(); +} + + +/* + * check_new_cluster_via_initdb() + * + * Dry-run --check --initdb path: build the initdb command with + * build_new_cluster_initdb_cmd() but do not execute it. Report the command + * and the checks it performed, then exit. Lighter than plain --check, which + * queries an already-created new cluster. + */ +static void +check_new_cluster_via_initdb(void) +{ + PQExpBufferData cmd; + + resolve_new_bindir(os_info.progname); + build_new_cluster_initdb_cmd(&cmd); + + pg_log(PG_REPORT, _("The following initdb command would be run to create the new cluster:\n %s"), cmd.data); + pg_log(PG_REPORT, _("The new cluster would be created with settings matching the old cluster. Run pg_upgrade --check afterward for the full compatibility check.")); + termPQExpBuffer(&cmd); + exit(0); +} + + static void setup(char *argv0) { @@ -372,17 +669,7 @@ setup(char *argv0) * with -B, default to using the path of the currently executed pg_upgrade * binary. */ - if (!new_cluster.bindir) - { - char exec_path[MAXPGPATH]; - - if (find_my_exec(argv0, exec_path) < 0) - pg_fatal("%s: could not find own program executable", argv0); - /* Trim off program name and keep just path */ - *last_dir_separator(exec_path) = '\0'; - canonicalize_path(exec_path); - new_cluster.bindir = pg_strdup(exec_path); - } + resolve_new_bindir(argv0); verify_directories(); diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 4cb79bb780e61..ebd3aac080576 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -325,6 +325,9 @@ typedef struct int char_signedness; /* default char signedness: -1 for initial * value, 1 for "signed" and 0 for * "unsigned" */ + bool initdb_new_cluster; /* run initdb to create the new cluster + * before upgrading, instead of requiring + * the user to have created it manually */ } UserOpts; typedef struct @@ -391,6 +394,7 @@ void generate_old_dump(void); bool exec_prog(const char *log_filename, const char *opt_log_file, bool report_error, bool exit_on_error, const char *fmt, ...) pg_attribute_printf(5, 6); +void get_bin_version(ClusterInfo *cluster); void verify_directories(void); bool pid_lock_file_exists(const char *datadir); @@ -423,6 +427,7 @@ FileNameMap *gen_db_file_maps(DbInfo *old_db, DbInfo *new_db, int *nmaps, const char *old_pgdata, const char *new_pgdata); void get_db_rel_and_slot_infos(ClusterInfo *cluster); +void get_template0_info(ClusterInfo *cluster); int count_old_cluster_logical_slots(void); void get_subscription_info(ClusterInfo *cluster); diff --git a/src/bin/pg_upgrade/t/009_initdb_option.pl b/src/bin/pg_upgrade/t/009_initdb_option.pl new file mode 100644 index 0000000000000..3f4243946643d --- /dev/null +++ b/src/bin/pg_upgrade/t/009_initdb_option.pl @@ -0,0 +1,234 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test the --initdb option of pg_upgrade: pg_upgrade creates the new cluster +# itself via initdb, instead of requiring the user to have run initdb first. + +use strict; +use warnings FATAL => 'all'; + +use File::Path qw(rmtree); +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Initialize and populate the old cluster. +# +# Use non-default settings that --initdb must carry over to the new cluster +# (derived from the old cluster's pg_control): disabled data checksums (initdb +# enables them by default since PG18), a non-default WAL segment size, and the +# C locale. We check below that the new cluster inherits them. +my $oldnode = PostgreSQL::Test::Cluster->new('old_node'); +$oldnode->init( + extra => [ + '--no-data-checksums', + '--wal-segsize' => '2', + '--locale' => 'C', + ]); +$oldnode->start; +$oldnode->safe_psql('postgres', + "CREATE TABLE t (id int primary key, note text); " + . "INSERT INTO t SELECT g, 'row ' || g FROM generate_series(1, 100) g; " + . "CREATE DATABASE extra_db;"); +my $rows_before = + $oldnode->safe_psql('postgres', 'SELECT count(*) FROM t'); +is($rows_before, '100', 'old cluster has expected rows before upgrade'); + +# Record the old cluster's settings so we can compare them after the upgrade. +my $old_checksums = $oldnode->safe_psql('postgres', 'SHOW data_checksums'); +my $old_wal_segsize = $oldnode->safe_psql('postgres', 'SHOW wal_segment_size'); +my $old_encoding = $oldnode->safe_psql('postgres', + "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'"); +my $old_collate = $oldnode->safe_psql('postgres', + "SELECT datcollate FROM pg_database WHERE datname = 'template0'"); +my $old_ctype = $oldnode->safe_psql('postgres', + "SELECT datctype FROM pg_database WHERE datname = 'template0'"); +my $old_provider = $oldnode->safe_psql('postgres', + "SELECT datlocprovider FROM pg_database WHERE datname = 'template0'"); +$oldnode->stop; + +# Create the new node object but do NOT init() it: pg_upgrade --initdb is +# responsible for creating the data directory. Only new() runs, which +# allocates the port/host/basedir the framework needs. +my $newnode = PostgreSQL::Test::Cluster->new('new_node'); + +my $oldbindir = $oldnode->config_data('--bindir'); +my $newbindir = $newnode->config_data('--bindir'); + +# Sanity: the new data directory must not exist yet. +ok(!-d $newnode->data_dir, + 'new cluster data directory does not exist before --initdb'); + +# Run pg_upgrade with --initdb. We must run in a writable directory because +# pg_upgrade writes output files relative to the current directory. +chdir ${PostgreSQL::Test::Utils::tmp_check}; + +command_ok( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir, + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + ], + 'run of pg_upgrade --initdb creates and upgrades the new cluster'); + +# The new data directory should now exist and be a v18+ cluster. +ok(-f $newnode->data_dir . '/PG_VERSION', + 'new cluster data directory created by --initdb'); + +# The framework's init() would normally write port/socket settings into +# postgresql.conf; since we skipped it, append them now so we can start the +# upgraded cluster through the test harness. Mirror init()'s own TCP vs Unix +# socket handling so this works on Windows (where TCP is used) as well. +my $host = $newnode->host; +$newnode->append_conf('postgresql.conf', "port = " . $newnode->port); +if ($PostgreSQL::Test::Cluster::use_tcp) +{ + $newnode->append_conf('postgresql.conf', "unix_socket_directories = ''"); + $newnode->append_conf('postgresql.conf', "listen_addresses = '$host'"); +} +else +{ + $newnode->append_conf('postgresql.conf', + "unix_socket_directories = '$host'"); + $newnode->append_conf('postgresql.conf', "listen_addresses = ''"); +} + +$newnode->start; + +# Verify the user data survived the upgrade. +my $rows_after = $newnode->safe_psql('postgres', 'SELECT count(*) FROM t'); +is($rows_after, '100', 'user data survived --initdb upgrade'); + +# Verify the extra database carried over too. +my $has_extra = $newnode->safe_psql('postgres', + "SELECT count(*) FROM pg_database WHERE datname = 'extra_db'"); +is($has_extra, '1', 'user database carried over by --initdb upgrade'); + +# Verify the new cluster is a newer major version than the old one. +my $newver = $newnode->safe_psql('postgres', + "SELECT current_setting('server_version_num')::int / 10000"); +ok($newver >= 18, "new cluster reports target major version ($newver)"); + +# --initdb must reproduce these settings from the old cluster; otherwise +# check_control_data() would reject the new cluster. Verify each carried over. +my $new_checksums = $newnode->safe_psql('postgres', 'SHOW data_checksums'); +is($new_checksums, $old_checksums, + "data_checksums propagated by --initdb ($new_checksums)"); + +my $new_wal_segsize = $newnode->safe_psql('postgres', 'SHOW wal_segment_size'); +is($new_wal_segsize, $old_wal_segsize, + "wal_segment_size propagated by --initdb ($new_wal_segsize)"); + +my $new_encoding = $newnode->safe_psql('postgres', + "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'"); +is($new_encoding, $old_encoding, + "template0 encoding propagated by --initdb ($new_encoding)"); + +my $new_collate = $newnode->safe_psql('postgres', + "SELECT datcollate FROM pg_database WHERE datname = 'template0'"); +is($new_collate, $old_collate, + "template0 collation propagated by --initdb ($new_collate)"); + +my $new_ctype = $newnode->safe_psql('postgres', + "SELECT datctype FROM pg_database WHERE datname = 'template0'"); +is($new_ctype, $old_ctype, + "template0 ctype propagated by --initdb ($new_ctype)"); + +my $new_provider = $newnode->safe_psql('postgres', + "SELECT datlocprovider FROM pg_database WHERE datname = 'template0'"); +is($new_provider, $old_provider, + "template0 locale provider propagated by --initdb ($new_provider)"); + +$newnode->stop; + +# --initdb must refuse to clobber an already-populated data directory, and the +# failure must come from pg_upgrade's own PG_VERSION check (not initdb's +# "directory not empty" error), so confirm the specific message. pg_upgrade +# prints its fatal message to stdout, so match there. +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir, + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + ], + 1, + [qr/already contains a database system/], + [qr/^$/], + '--initdb refuses to overwrite an existing cluster (PG_VERSION check)'); + +# --initdb must fail early with a clear message if initdb is not present in the +# new cluster's bin directory. Point --new-bindir at an empty directory and use +# a fresh (nonexistent) new data directory so we reach the initdb-present check. +my $empty_bindir = PostgreSQL::Test::Utils::tempdir; +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir . '_nonexistent', + '--old-bindir' => $oldbindir, + '--new-bindir' => $empty_bindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + ], + 1, + [qr/could not find "initdb"/], + [qr/^$/], + '--initdb fails early when initdb is missing from the new bindir'); + +# --check --initdb performs validation without creating anything. +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir . '_dry_run', + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + '--check', + ], + 0, + [qr/created with settings matching the old cluster/], + [qr/^$/], + '--check --initdb validates without creating the cluster'); + +# Verify that --check --initdb didn't create anything. +ok(!-d $newnode->data_dir . '_dry_run', + '--check --initdb does not create the new cluster directory'); + +# -O passes postmaster-only options, which initdb does not accept, so the +# combination is rejected during option parsing rather than forwarded. +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir . '_nonexistent', + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + '--new-options' => '-c work_mem=1MB', + ], + 1, + [qr/options -O\/--new-options and --initdb cannot be used together/], + [qr/^$/], + '--initdb and -O cannot be used together'); + +done_testing(); From e9b94386fecb96767c3a198e8d463df0402e4b80 Mon Sep 17 00:00:00 2001 From: Bohyun Lee Date: Tue, 6 Oct 2026 16:18:13 +0200 Subject: [PATCH] pg_upgrade: address --initdb review feedback Reuse source compatibility checks for --check --initdb, including running sources. Resolve the target bindir from argv[0], preserve source group access, and keep -O handling limited to the target server. Reuse output-directory cleanup, reject overlapping data and output directories, and preserve pre-existing empty targets after early failures. Register cleanup before server exit handlers so postmasters stop before their data directories are removed. Register and extend TAP coverage, clarify documentation and comments, and remove the redundant target-version assertion. Additional initdb arguments remain a separate upstream discussion. --- doc/src/sgml/ref/pgupgrade.sgml | 62 +++-- src/bin/pg_upgrade/check.c | 7 +- src/bin/pg_upgrade/controldata.c | 30 ++- src/bin/pg_upgrade/exec.c | 27 +- src/bin/pg_upgrade/meson.build | 1 + src/bin/pg_upgrade/option.c | 11 - src/bin/pg_upgrade/pg_upgrade.c | 291 ++++++++++++--------- src/bin/pg_upgrade/pg_upgrade.h | 8 +- src/bin/pg_upgrade/t/009_initdb_option.pl | 295 +++++++++++++++++++--- 9 files changed, 516 insertions(+), 216 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 996c04206ba2b..21f883886dd30 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -268,43 +268,47 @@ PostgreSQL documentation Create the new cluster automatically by running initdb before upgrading, instead of requiring the - user to have created it manually. The WAL segment size and data - checksum setting are derived from the old cluster so the new cluster - passes pg_upgrade's control-data check; the - encoding and locale are matched to the old cluster's - template0. + user to have created it manually. + The new cluster uses the old cluster's WAL segment size and + data-checksum setting, and the encoding and locale of its + template0 database. + On Unix systems, if the old cluster allows group read/execute access + to its data directory, is passed + to initdb for the new cluster. The new cluster data directory specified with / must be empty or not yet exist; otherwise pg_upgrade exits with an error. + It must not overlap the pg_upgrade_output.d + directory in the current working directory: neither directory may + be equal to or contain the other. When combined with /, - performs a dry run: it reports the - initdb command it would run and verifies that the new - cluster could be created (the new binaries' major version matches, the - target directory is empty, and the old cluster is reachable), but - does not create the new cluster. This is a lighter check than a plain - , which instead queries an already-created new - cluster. + runs source-cluster compatibility checks, + validates the new binaries and the empty or nonexistent target + directory, and reports the initdb command it would + run without creating the new cluster. + Checks requiring an initialized target cluster or a writable target + data directory, including loading required libraries and testing the + selected file transfer mode, are deferred until the real upgrade. - cannot be combined with - /: those options are - passed to the new cluster's server process, which accepts a different - set of options than initdb. If you need to supply - such options, create the new cluster manually and omit - . + / continues to pass + options to the new cluster's server process; those options are not + passed to initdb. To use additional + initdb options, create the new cluster manually and + omit . - To discover the old cluster's encoding and locale, - briefly starts the old server in - binary-upgrade mode (which disables autovacuum) and stops it again - before creating the new cluster. If a compatibility check fails after - the new cluster has been created, its data directory is removed - automatically, so the upgrade can be retried without manual cleanup. + If initdb or a compatibility check fails, + pg_upgrade removes the newly created files. + A directory created by pg_upgrade is also + removed. + A pre-existing empty directory is preserved, and its original + permissions are restored on supported Unix systems. @@ -670,7 +674,9 @@ pg_upgrade.exe You can use the same port number for both clusters when doing an upgrade because the old and new clusters will not be running at the same time. However, when checking an old running server, the old - and new port numbers must be different. + and new port numbers must be different unless + is combined with , which does not start a new + server. @@ -1125,8 +1131,10 @@ psql --username=postgres --file=script.sql postgres pg_upgrade creates various working files, such as schema dumps, stored within pg_upgrade_output.d in - the directory of the new cluster. Each run creates a new subdirectory named - with a timestamp formatted as per ISO 8601 + the directory of the new cluster, or in the current working directory when + is used. + Each run creates a new subdirectory named with a timestamp formatted as per + ISO 8601 (%Y%m%dT%H%M%S), where all its generated files are stored. pg_upgrade_output.d and its contained files will be diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 150c4fc34b9b2..098eeab34ea27 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -30,7 +30,6 @@ static void check_for_incompatible_polymorphics(ClusterInfo *cluster); static void check_for_tables_with_oids(ClusterInfo *cluster); static void check_for_not_null_inheritance(ClusterInfo *cluster); static void check_for_gist_inet_ops(ClusterInfo *cluster); -static void check_for_new_tablespace_dir(void); static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster); static void check_for_unicode_update(ClusterInfo *cluster); static void check_new_cluster_replication_slots(void); @@ -741,7 +740,7 @@ check_new_cluster(void) check_for_prepared_transactions(&new_cluster); - check_for_new_tablespace_dir(); + check_new_cluster_tablespace_dirs(); check_new_cluster_replication_slots(); @@ -923,8 +922,8 @@ check_new_cluster_is_empty(void) * This allows the failure to be detected at check time, rather than * during schema restore. */ -static void -check_for_new_tablespace_dir(void) +void +check_new_cluster_tablespace_dirs(void) { int tblnum; char new_tablespace_dir[MAXPGPATH]; diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c index e83f2d963cdfb..6cdd7fa570d91 100644 --- a/src/bin/pg_upgrade/controldata.c +++ b/src/bin/pg_upgrade/controldata.c @@ -607,6 +607,26 @@ get_control_data(ClusterInfo *cluster) } +/* + * check_old_control_data() + * + * check source control-data states that are unsafe for any upgrade + */ +void +check_old_control_data(ControlData *oldctrl) +{ + /* + * If data checksums are in any in-progress state then disallow the + * upgrade. The user should either let the process finish, or turn off + * data checksums, before retrying. + */ + if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_ON) + pg_fatal("data checksums are being enabled in the old cluster"); + if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_OFF) + pg_fatal("data checksums are being disabled in the old cluster"); +} + + /* * check_control_data() * @@ -653,15 +673,7 @@ check_control_data(ControlData *oldctrl, * check_for_isn_and_int8_passing_mismatch(). */ - /* - * If data checksums are in any in-progress state then disallow the - * upgrade. The user should either let the process finish, or turn off - * data checksums, before retrying. - */ - if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_ON) - pg_fatal("data checksums are being enabled in the old cluster"); - if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_OFF) - pg_fatal("data checksums are being disabled in the old cluster"); + check_old_control_data(oldctrl); /* * We might eventually allow upgrades from checksum to no-checksum diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index e9d7f8a5789b0..347dd682347a6 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -16,8 +16,7 @@ #include "pg_upgrade.h" static void check_data_dir(ClusterInfo *cluster); -static void check_bin_dir(ClusterInfo *cluster, bool check_versions); -void get_bin_version(ClusterInfo *cluster); +static void get_bin_version(ClusterInfo *cluster); static void check_exec(const char *dir, const char *program, bool check_version); #ifdef WIN32 @@ -30,7 +29,7 @@ static int win32_check_directory_write_permissions(void); * * Fetch major version of binaries for cluster. */ -void +static void get_bin_version(ClusterInfo *cluster) { char cmd[MAXPGPATH], @@ -252,7 +251,7 @@ pid_lock_file_exists(const char *datadir) * NOTE: May update the values of all parameters */ void -verify_directories(void) +verify_directories(bool check_new_data_dir) { #ifndef WIN32 if (access(".", R_OK | W_OK | X_OK) != 0) @@ -263,8 +262,22 @@ verify_directories(void) check_bin_dir(&old_cluster, false); check_data_dir(&old_cluster); - check_bin_dir(&new_cluster, true); - check_data_dir(&new_cluster); + /* --initdb validates the new binaries before creating the cluster. */ + if (!user_opts.initdb_new_cluster) + check_bin_dir(&new_cluster, true); + if (check_new_data_dir) + check_data_dir(&new_cluster); + else + { + /* + * --check --initdb does not initialize the new cluster. Fill the + * target fields needed by the source-cluster compatibility checks. + */ + new_cluster.major_version = new_cluster.bin_version; + new_cluster.major_version_str = pg_strdup(PG_MAJORVERSION); + new_cluster.controldata.cat_ver = CATALOG_VERSION_NO; + new_cluster.controldata.float8_pass_by_value = FLOAT8PASSBYVAL; + } } @@ -361,7 +374,7 @@ check_data_dir(ClusterInfo *cluster) * against the version of this pg_upgrade. This is for checking the target * bindir. */ -static void +void check_bin_dir(ClusterInfo *cluster, bool check_versions) { struct stat statBuf; diff --git a/src/bin/pg_upgrade/meson.build b/src/bin/pg_upgrade/meson.build index ffbf6ae8d759b..6288e0798643a 100644 --- a/src/bin/pg_upgrade/meson.build +++ b/src/bin/pg_upgrade/meson.build @@ -69,6 +69,7 @@ tests += { 't/006_transfer_modes.pl', 't/007_multixact_conversion.pl', 't/008_extension_control_path.pl', + 't/009_initdb_option.pl', ], 'deps': [test_ext], 'test_kwargs': {'priority': 40}, # pg_upgrade tests are slow diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index d05b2d913f410..daaf48d47bc9c 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -249,17 +249,6 @@ parseCommandLine(int argc, char *argv[]) if (optind < argc) pg_fatal("too many command-line arguments (first is \"%s\")", argv[optind]); - /* - * -O passes options to the new cluster's postmaster, but with --initdb - * the new cluster is created by initdb, which accepts a different option - * set. Rather than guess which -O options initdb also understands, reject - * the combination and let the user create the cluster manually (without - * --initdb) if they need postmaster-only options. - */ - if (new_cluster.pgopts && user_opts.initdb_new_cluster) - pg_fatal("options %s and %s cannot be used together", - "-O/--new-options", "--initdb"); - if (!user_opts.sync_method) user_opts.sync_method = pg_strdup("fsync"); diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 6588c6b5a3ee6..55a30f0f9fdde 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -41,7 +41,6 @@ #include "postgres_fe.h" -#include #include #include "access/multixact.h" @@ -69,12 +68,15 @@ static void prepare_new_globals(void); static void create_new_objects(void); static void copy_xact_xlog_xid(void); static void set_frozenxids(void); -static void make_outputdirs(char *pgdata); -static void setup(char *argv0); +static void make_outputdirs(const char *output_root); +static void setup(char *argv0, bool check_new_data_dir); static void resolve_new_bindir(const char *argv0); +static void prepare_new_cluster_initdb(const char *argv0); +static void check_new_cluster_initdb_target(void); +static void get_old_cluster_initdb_info(void); static void build_new_cluster_initdb_cmd(PQExpBuffer cmd); static void create_new_cluster_via_initdb(void); -static void check_new_cluster_via_initdb(void); +static void check_new_cluster_via_initdb(char *argv0); static void create_logical_replication_slots(void); static void create_conflict_detection_slot(void); @@ -82,8 +84,9 @@ ClusterInfo old_cluster, new_cluster; OSInfo os_info; -static bool new_cluster_created_by_initdb = false; -static bool initdb_cleanup_registered = false; +static bool new_cluster_cleanup_required = false; +static bool new_cluster_pgdata_existed = false; +static mode_t new_cluster_pgdata_mode; char *output_files[] = { SERVER_LOG_FILE, @@ -120,10 +123,24 @@ main(int argc, char **argv) adjust_data_dir(&old_cluster); adjust_data_dir(&new_cluster); - if (user_opts.check && user_opts.initdb_new_cluster) - check_new_cluster_via_initdb(); /* exits(0), never returns */ - else if (user_opts.initdb_new_cluster) + if (user_opts.initdb_new_cluster) + { + prepare_new_cluster_initdb(argv[0]); + + /* Match output-file permissions to the cluster being inspected. */ + if (!GetDataDirectoryCreatePerm(old_cluster.pgdata)) + pg_fatal("could not read permissions of directory \"%s\": %m", + old_cluster.pgdata); + umask(pg_mode_mask); + + /* The new cluster has not been initialized yet. */ + make_outputdirs("."); + + if (user_opts.check) + check_new_cluster_via_initdb(argv[0]); /* exits, never returns */ + create_new_cluster_via_initdb(); + } /* * Set mask based on PGDATA permissions, needed for the creation of the @@ -139,9 +156,10 @@ main(int argc, char **argv) * This needs to happen after adjusting the data directory of the new * cluster in adjust_data_dir(). */ - make_outputdirs(new_cluster.pgdata); + if (!user_opts.initdb_new_cluster) + make_outputdirs(new_cluster.pgdata); - setup(argv[0]); + setup(argv[0], true); output_check_banner(); @@ -161,8 +179,8 @@ main(int argc, char **argv) check_new_cluster(); report_clusters_compatible(); - /* Disarm orphan cleanup once we reach the point of no easy return. */ - new_cluster_created_by_initdb = false; + /* Keep the new cluster on failure after compatibility checks pass. */ + new_cluster_cleanup_required = false; pg_log(PG_REPORT, "\n" @@ -293,7 +311,7 @@ main(int argc, char **argv) * the process. */ static void -make_outputdirs(char *pgdata) +make_outputdirs(const char *output_root) { FILE *fp; char **filename; @@ -305,7 +323,7 @@ make_outputdirs(char *pgdata) int len; log_opts.rootdir = (char *) pg_malloc0(MAXPGPATH); - len = snprintf(log_opts.rootdir, MAXPGPATH, "%s/%s", pgdata, BASE_OUTPUTDIR); + len = snprintf(log_opts.rootdir, MAXPGPATH, "%s/%s", output_root, BASE_OUTPUTDIR); if (len >= MAXPGPATH) pg_fatal("directory path for new cluster is too long"); @@ -380,7 +398,7 @@ make_outputdirs(char *pgdata) /* * resolve_new_bindir() * - * If new_cluster.bindir was not set by the user via -B, derive it from the + * If new_cluster.bindir was not set by -B or PGBINNEW, derive it from the * path of the currently executing pg_upgrade binary. Safe to call more than * once. */ @@ -393,7 +411,7 @@ resolve_new_bindir(const char *argv0) if (find_my_exec(argv0, exec_path) < 0) pg_fatal("%s: could not find own program executable", argv0); - /* Trim off program name and keep just the directory */ + /* Trim off program name and keep just path */ *last_dir_separator(exec_path) = '\0'; canonicalize_path(exec_path); new_cluster.bindir = pg_strdup(exec_path); @@ -404,51 +422,58 @@ resolve_new_bindir(const char *argv0) /* * new_cluster_cleanup_atexit() * - * atexit() handler: remove the new cluster's data directory if --initdb - * created it but the run failed before reaching the point of no return. - * Does nothing unless new_cluster_created_by_initdb is set. + * Clean up the new cluster if the run fails before compatibility checks + * pass. Preserve a target directory that existed before pg_upgrade. */ static void new_cluster_cleanup_atexit(void) { - if (!new_cluster_created_by_initdb) + int dir_status; + + if (!new_cluster_cleanup_required) + return; + + dir_status = pg_check_dir(new_cluster.pgdata); + if (dir_status == 0) + return; + if (dir_status < 0) + { + pg_log(PG_WARNING, "could not access new cluster data directory \"%s\": %m", + new_cluster.pgdata); return; - (void) rmtree(new_cluster.pgdata, true); + } + + if (new_cluster_pgdata_existed) + { + pg_log(PG_REPORT, "removing contents of new cluster data directory \"%s\"", + new_cluster.pgdata); + (void) rmtree(new_cluster.pgdata, false); + +#if !defined(WIN32) && !defined(__CYGWIN__) + if (chmod(new_cluster.pgdata, new_cluster_pgdata_mode) != 0) + pg_log(PG_WARNING, "could not restore permissions of directory \"%s\": %m", + new_cluster.pgdata); +#endif + } + else + { + pg_log(PG_REPORT, "removing new cluster data directory \"%s\"", + new_cluster.pgdata); + (void) rmtree(new_cluster.pgdata, true); + } } /* - * build_new_cluster_initdb_cmd() - * - * Shared helper for both the real --initdb path and the --check --initdb - * dry-run path. Starts the old cluster (in binary-upgrade mode, which - * disables autovacuum), queries template0 for encoding/locale settings, reads - * old cluster pg_control via get_control_data(), stops the old cluster, and - * populates 'cmd' with the initdb command-string needed to create the new - * cluster with matching settings. - * - * This helper does not execute the command; callers decide whether to - * exec_prog() it (real upgrade) or just report it (dry-run). - * - * Both callers must have already called adjust_data_dir(&new_cluster) and - * resolve_new_bindir() before calling this, to ensure new_cluster.pgdata - * and new_cluster.bindir are set. - * - * On return, log_opts.logdir points at a temporary directory used for the - * old-cluster start/stop and the later initdb run. Callers save and restore - * it around this helper. + * Validate the new binaries and target directory before creating output + * files or starting either cluster. */ static void -build_new_cluster_initdb_cmd(PQExpBuffer cmd) +prepare_new_cluster_initdb(const char *argv0) { - DbLocaleInfo *locale; - const char *encoding_name; char initdb_path[MAXPGPATH]; - char verfile[MAXPGPATH]; - char tmp_logdir[MAXPGPATH]; - struct stat st; - DIR *dir; - struct dirent *de; + + resolve_new_bindir(argv0); /* Verify initdb is present in the new cluster's bin directory. */ snprintf(initdb_path, sizeof(initdb_path), "%s/initdb", new_cluster.bindir); @@ -457,67 +482,85 @@ build_new_cluster_initdb_cmd(PQExpBuffer cmd) "The --initdb option requires initdb to be present in the new cluster's bin directory.", new_cluster.bindir); - /* - * Refuse to run initdb into a directory that already exists and is not - * empty. If a later step fails, the cleanup handler removes the entire - * new data directory. Requiring it to be empty first ensures the cleanup - * never destroys files the user already had there. - */ + check_bin_dir(&new_cluster, true); + check_new_cluster_initdb_target(); +} + + +/* + * Refuse to overwrite an existing target and remember whether an empty target + * directory must be preserved if a later step fails. + */ +static void +check_new_cluster_initdb_target(void) +{ + char verfile[MAXPGPATH]; + char *absolute_pgdata; + char *absolute_outputdir; + struct stat st; + int dir_status; + snprintf(verfile, sizeof(verfile), "%s/PG_VERSION", new_cluster.pgdata); if (stat(verfile, &st) == 0) pg_fatal("new cluster data directory \"%s\" already contains a database system; " "--initdb requires an empty or nonexistent directory", new_cluster.pgdata); - dir = opendir(new_cluster.pgdata); - if (dir) + + dir_status = pg_check_dir(new_cluster.pgdata); + switch (dir_status) { - while (errno = 0, (de = readdir(dir)) != NULL) - { - if (strcmp(de->d_name, ".") != 0 && - strcmp(de->d_name, "..") != 0) - pg_fatal("new cluster data directory \"%s\" is not empty; " - "--initdb requires an empty or nonexistent directory", + case 0: + new_cluster_pgdata_existed = false; + break; + case 1: + new_cluster_pgdata_existed = true; + if (stat(new_cluster.pgdata, &st) != 0) + pg_fatal("could not stat directory \"%s\": %m", new_cluster.pgdata); - } - if (errno) - pg_fatal("could not read directory \"%s\": %m", new_cluster.pgdata); - closedir(dir); + new_cluster_pgdata_mode = st.st_mode; + break; + case 2: + case 3: + case 4: + pg_fatal("new cluster data directory \"%s\" is not empty; " + "--initdb requires an empty or nonexistent directory", + new_cluster.pgdata); + default: + pg_fatal("could not access directory \"%s\": %m", + new_cluster.pgdata); } - else if (errno != ENOENT) - pg_fatal("could not open directory \"%s\": %m", new_cluster.pgdata); - /* - * Validate the new binaries' version before touching disk, so a wrong - * --new-bindir fails before the new cluster is created and there is - * nothing to clean up. - */ - if (new_cluster.bin_version == 0) - get_bin_version(&new_cluster); - if (GET_PG_MAJORVERSION_NUM(new_cluster.bin_version) != - GET_PG_MAJORVERSION_NUM(PG_VERSION_NUM)) - pg_fatal("new cluster binaries are version %d, but pg_upgrade is version %d", - GET_PG_MAJORVERSION_NUM(new_cluster.bin_version), - GET_PG_MAJORVERSION_NUM(PG_VERSION_NUM)); + absolute_pgdata = make_absolute_path(new_cluster.pgdata); + absolute_outputdir = make_absolute_path(BASE_OUTPUTDIR); + if (!absolute_pgdata || !absolute_outputdir) + exit(1); + if (path_is_prefix_of_path(absolute_pgdata, absolute_outputdir) || + path_is_prefix_of_path(absolute_outputdir, absolute_pgdata)) + pg_fatal("new cluster data directory \"%s\" overlaps output directory \"%s\"; " + "--initdb requires separate data and output directories", + absolute_pgdata, absolute_outputdir); + free(absolute_pgdata); + free(absolute_outputdir); +} + +/* + * Collect the old control and template0 settings needed by initdb. This is + * used only by a real upgrade. The dry-run path obtains the same information + * while running the normal old-cluster checks. + */ +static void +get_old_cluster_initdb_info(void) +{ old_cluster.major_version = get_pg_version(old_cluster.pgdata, &old_cluster.major_version_str); - /* - * The normal output directory does not exist yet, so use a temporary log - * directory next to the new data directory (writable, unlike the new bin - * directory) for initdb and the brief old-server start. - */ - snprintf(tmp_logdir, sizeof(tmp_logdir), "%s.initdb_log", new_cluster.pgdata); - if (mkdir(tmp_logdir, pg_dir_create_mode) < 0 && errno != EEXIST) - pg_fatal("could not create log directory \"%s\": %m", tmp_logdir); - log_opts.logdir = pg_strdup(tmp_logdir); - if (!old_cluster.sockdir) old_cluster.sockdir = user_opts.socketdir ? user_opts.socketdir : "."; /* * The old server must be shut down. The template0 read below starts a - * postmaster on the old cluster, and get_control_data() runs pg_resetwal, + * postmaster on the old cluster, and get_control_data() runs pg_resetwal -n, * both of which need exclusive access to the old data directory. A stale * lock file is tolerated as setup() does. */ @@ -537,9 +580,21 @@ build_new_cluster_initdb_cmd(PQExpBuffer cmd) get_template0_info(&old_cluster); stop_postmaster(false); check_ok(); +} + + +/* + * Format the initdb command from information already collected about the old + * cluster. Callers either execute it or report it for --check. + */ +static void +build_new_cluster_initdb_cmd(PQExpBuffer cmd) +{ + DbLocaleInfo *locale = old_cluster.template0; + const char *encoding_name = pg_encoding_to_char(locale->db_encoding); + char initdb_path[MAXPGPATH]; - locale = old_cluster.template0; - encoding_name = pg_encoding_to_char(locale->db_encoding); + snprintf(initdb_path, sizeof(initdb_path), "%s/initdb", new_cluster.bindir); prep_status("Constructing new cluster initdb command"); @@ -558,6 +613,8 @@ build_new_cluster_initdb_cmd(PQExpBuffer cmd) appendShellString(cmd, new_cluster.pgdata); appendPQExpBufferStr(cmd, " -U "); appendShellString(cmd, os_info.user); + if (pg_dir_create_mode == PG_DIR_MODE_GROUP) + appendPQExpBufferStr(cmd, " --allow-group-access"); appendPQExpBuffer(cmd, " --wal-segsize=%u", old_cluster.controldata.walseg / (1024 * 1024)); @@ -602,32 +659,26 @@ build_new_cluster_initdb_cmd(PQExpBuffer cmd) /* * create_new_cluster_via_initdb() * - * Create the new cluster for --initdb: build the initdb command with - * build_new_cluster_initdb_cmd() and execute it. The atexit cleanup is - * enabled just before execution, so a failure after initdb runs (but before - * the point of no return) removes the new directory. + * Create the new cluster with initdb. Register cleanup before the first + * start_postmaster() call so the server's exit handler runs before directory + * cleanup. Enable cleanup immediately before running initdb. */ static void create_new_cluster_via_initdb(void) { PQExpBufferData cmd; - char *saved_logdir = log_opts.logdir; - resolve_new_bindir(os_info.progname); + atexit(new_cluster_cleanup_atexit); + + get_old_cluster_initdb_info(); build_new_cluster_initdb_cmd(&cmd); prep_status("Creating new cluster with initdb"); - if (!initdb_cleanup_registered) - { - atexit(new_cluster_cleanup_atexit); - initdb_cleanup_registered = true; - } - new_cluster_created_by_initdb = true; + new_cluster_cleanup_required = true; exec_prog(UTILITY_LOG_FILE, NULL, true, true, "%s", cmd.data); termPQExpBuffer(&cmd); - log_opts.logdir = saved_logdir; check_ok(); } @@ -636,27 +687,37 @@ create_new_cluster_via_initdb(void) * check_new_cluster_via_initdb() * * Dry-run --check --initdb path: build the initdb command with - * build_new_cluster_initdb_cmd() but do not execute it. Report the command - * and the checks it performed, then exit. Lighter than plain --check, which - * queries an already-created new cluster. + * build_new_cluster_initdb_cmd() but do not execute it. Run source-cluster + * compatibility checks, report the command, and exit. */ static void -check_new_cluster_via_initdb(void) +check_new_cluster_via_initdb(char *argv0) { PQExpBufferData cmd; - resolve_new_bindir(os_info.progname); + setup(argv0, false); + + output_check_banner(); + check_cluster_versions(); + get_sock_dir(&old_cluster); + + get_control_data(&old_cluster); + check_old_control_data(&old_cluster.controldata); + + check_and_dump_old_cluster(); + check_new_cluster_tablespace_dirs(); build_new_cluster_initdb_cmd(&cmd); pg_log(PG_REPORT, _("The following initdb command would be run to create the new cluster:\n %s"), cmd.data); - pg_log(PG_REPORT, _("The new cluster would be created with settings matching the old cluster. Run pg_upgrade --check afterward for the full compatibility check.")); + pg_log(PG_REPORT, _("Source cluster compatibility checks passed.")); termPQExpBuffer(&cmd); + cleanup_output_dirs(); exit(0); } static void -setup(char *argv0) +setup(char *argv0, bool check_new_data_dir) { /* * make sure the user has a clean environment, otherwise, we may confuse @@ -671,7 +732,7 @@ setup(char *argv0) */ resolve_new_bindir(argv0); - verify_directories(); + verify_directories(check_new_data_dir); /* no postmasters should be running, except for a live check */ if (pid_lock_file_exists(old_cluster.pgdata)) diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index ebd3aac080576..3651796886426 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -32,7 +32,7 @@ /* * Base directories that include all the files generated internally, from the - * root path of the new cluster. The paths are dynamically built as of + * selected output root. The paths are dynamically built as of * BASE_OUTPUTDIR/$timestamp/{LOG_OUTPUTDIR,DUMP_OUTPUTDIR} to ensure their * uniqueness in each run. */ @@ -368,6 +368,7 @@ extern OSInfo os_info; void output_check_banner(void); void check_and_dump_old_cluster(void); void check_new_cluster(void); +void check_new_cluster_tablespace_dirs(void); void report_clusters_compatible(void); void issue_warnings_and_set_wal_level(void); void output_completion_banner(char *deletion_script_file_name); @@ -380,6 +381,7 @@ void create_script_for_old_cluster_deletion(char **deletion_script_file_name); void get_control_data(ClusterInfo *cluster); void check_control_data(ControlData *oldctrl, ControlData *newctrl); +void check_old_control_data(ControlData *oldctrl); void disable_old_cluster(transferMode transfer_mode); @@ -394,8 +396,8 @@ void generate_old_dump(void); bool exec_prog(const char *log_filename, const char *opt_log_file, bool report_error, bool exit_on_error, const char *fmt, ...) pg_attribute_printf(5, 6); -void get_bin_version(ClusterInfo *cluster); -void verify_directories(void); +void check_bin_dir(ClusterInfo *cluster, bool check_versions); +void verify_directories(bool check_new_data_dir); bool pid_lock_file_exists(const char *datadir); diff --git a/src/bin/pg_upgrade/t/009_initdb_option.pl b/src/bin/pg_upgrade/t/009_initdb_option.pl index 3f4243946643d..d989dfb0c9d9f 100644 --- a/src/bin/pg_upgrade/t/009_initdb_option.pl +++ b/src/bin/pg_upgrade/t/009_initdb_option.pl @@ -6,38 +6,43 @@ use strict; use warnings FATAL => 'all'; -use File::Path qw(rmtree); +use Config; +use Cwd qw(abs_path); +use File::Basename qw(basename); +use File::Copy qw(copy); +use File::Path qw(rmtree); use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; # Initialize and populate the old cluster. # -# Use non-default settings that --initdb must carry over to the new cluster -# (derived from the old cluster's pg_control): disabled data checksums (initdb -# enables them by default since PG18), a non-default WAL segment size, and the -# C locale. We check below that the new cluster inherits them. +# Use settings that --initdb must carry over to the new cluster: group access, +# disabled data checksums (initdb enables them by default since PG18), a +# non-default WAL segment size, and the C locale. my $oldnode = PostgreSQL::Test::Cluster->new('old_node'); $oldnode->init( extra => [ + '--allow-group-access', '--no-data-checksums', '--wal-segsize' => '2', '--locale' => 'C', ]); $oldnode->start; $oldnode->safe_psql('postgres', - "CREATE TABLE t (id int primary key, note text); " + "CREATE TABLE t (id int primary key, note text); " . "INSERT INTO t SELECT g, 'row ' || g FROM generate_series(1, 100) g; " . "CREATE DATABASE extra_db;"); -my $rows_before = - $oldnode->safe_psql('postgres', 'SELECT count(*) FROM t'); +my $rows_before = $oldnode->safe_psql('postgres', 'SELECT count(*) FROM t'); is($rows_before, '100', 'old cluster has expected rows before upgrade'); # Record the old cluster's settings so we can compare them after the upgrade. my $old_checksums = $oldnode->safe_psql('postgres', 'SHOW data_checksums'); -my $old_wal_segsize = $oldnode->safe_psql('postgres', 'SHOW wal_segment_size'); +my $old_wal_segsize = + $oldnode->safe_psql('postgres', 'SHOW wal_segment_size'); my $old_encoding = $oldnode->safe_psql('postgres', - "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'"); + "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'" +); my $old_collate = $oldnode->safe_psql('postgres', "SELECT datcollate FROM pg_database WHERE datname = 'template0'"); my $old_ctype = $oldnode->safe_psql('postgres', @@ -58,8 +63,10 @@ ok(!-d $newnode->data_dir, 'new cluster data directory does not exist before --initdb'); -# Run pg_upgrade with --initdb. We must run in a writable directory because -# pg_upgrade writes output files relative to the current directory. +# Run pg_upgrade --initdb from a writable directory for its +# pg_upgrade_output.d logs. +# Pass the server-only option -F through -O to verify that it reaches +# the new server without being passed to initdb. chdir ${PostgreSQL::Test::Utils::tmp_check}; command_ok( @@ -73,17 +80,41 @@ '--old-port' => $oldnode->port, '--new-port' => $newnode->port, '--initdb', + '--new-options' => '-F', ], - 'run of pg_upgrade --initdb creates and upgrades the new cluster'); + 'run of pg_upgrade --initdb with -O creates and upgrades the new cluster' +); -# The new data directory should now exist and be a v18+ cluster. +# Check before the test harness starts the target and overwrites this file. +like(slurp_file($newnode->data_dir . '/postmaster.opts'), + qr/(?:^|\s)"-F"(?:\s|$)/, '-O option reached the target postmaster'); + +# pg_upgrade --initdb should have initialized the new data directory. +# Check that PG_VERSION was created there. ok(-f $newnode->data_dir . '/PG_VERSION', 'new cluster data directory created by --initdb'); -# The framework's init() would normally write port/socket settings into -# postgresql.conf; since we skipped it, append them now so we can start the -# upgraded cluster through the test harness. Mirror init()'s own TCP vs Unix -# socket handling so this works on Windows (where TCP is used) as well. +# Group access requested for the old cluster must be carried over to the new +# cluster. Otherwise programs that read PGDATA as a group member lose access +# after the upgrade. +SKIP: +{ + skip "unix-style permissions not supported on Windows", 1 + if ($windows_os || $Config::Config{osname} eq 'cygwin'); + + my $old_mode = (stat($oldnode->data_dir))[2] & 0777; + my $new_mode = (stat($newnode->data_dir))[2] & 0777; + is($new_mode, $old_mode, 'new cluster preserves PGDATA group access'); +} + +# --initdb logs use pg_upgrade_output.d with the normal cleanup and --retain +# handling. Check that no separate .initdb_log directory remains +# beside the new data directory. +ok(!-d $newnode->data_dir . '.initdb_log', + '--initdb does not leave a sibling log directory'); + +# Configure the upgraded node to use its assigned port and host. +# Follow PostgreSQL::Test::Cluster's TCP or Unix-domain socket settings. my $host = $newnode->host; $newnode->append_conf('postgresql.conf', "port = " . $newnode->port); if ($PostgreSQL::Test::Cluster::use_tcp) @@ -109,23 +140,21 @@ "SELECT count(*) FROM pg_database WHERE datname = 'extra_db'"); is($has_extra, '1', 'user database carried over by --initdb upgrade'); -# Verify the new cluster is a newer major version than the old one. -my $newver = $newnode->safe_psql('postgres', - "SELECT current_setting('server_version_num')::int / 10000"); -ok($newver >= 18, "new cluster reports target major version ($newver)"); - -# --initdb must reproduce these settings from the old cluster; otherwise -# check_control_data() would reject the new cluster. Verify each carried over. +# Check the checksum and WAL segment size settings required by +# check_control_data(). my $new_checksums = $newnode->safe_psql('postgres', 'SHOW data_checksums'); is($new_checksums, $old_checksums, "data_checksums propagated by --initdb ($new_checksums)"); -my $new_wal_segsize = $newnode->safe_psql('postgres', 'SHOW wal_segment_size'); +my $new_wal_segsize = + $newnode->safe_psql('postgres', 'SHOW wal_segment_size'); is($new_wal_segsize, $old_wal_segsize, "wal_segment_size propagated by --initdb ($new_wal_segsize)"); +# Check template0's encoding and locale settings. my $new_encoding = $newnode->safe_psql('postgres', - "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'"); + "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'" +); is($new_encoding, $old_encoding, "template0 encoding propagated by --initdb ($new_encoding)"); @@ -146,10 +175,9 @@ $newnode->stop; -# --initdb must refuse to clobber an already-populated data directory, and the -# failure must come from pg_upgrade's own PG_VERSION check (not initdb's -# "directory not empty" error), so confirm the specific message. pg_upgrade -# prints its fatal message to stdout, so match there. +# Check that pg_upgrade --initdb rejects an existing target cluster. +# Match its PG_VERSION diagnostic on stdout to distinguish this failure +# from initdb's nonempty-directory error. command_checks_all( [ 'pg_upgrade', '--no-sync', @@ -167,6 +195,36 @@ [qr/^$/], '--initdb refuses to overwrite an existing cluster (PG_VERSION check)'); +# Reject overlap with the output directory before creating logs or starting +# either server, for both a real upgrade and --check. +my $original_cwd = abs_path('.'); +for my $check (0, 1) +{ + for my $target ('.', 'pg_upgrade_output.d', 'pg_upgrade_output.d/new') + { + my $overlap_cwd = PostgreSQL::Test::Utils::tempdir; + chdir $overlap_cwd or die "could not change to $overlap_cwd: $!"; + my $mode = $check ? '--check --initdb' : '--initdb'; + command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $target, + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--initdb', + $check ? '--check' : (), + ], + 1, + [qr/overlaps output directory/], + [qr/^$/], + "$mode rejects target $target overlapping its output directory"); + is_deeply([ grep { $_ ne '.' && $_ ne '..' } slurp_dir('.') ], + [], "$mode overlap failure leaves the working directory empty"); + chdir $original_cwd or die "could not change to $original_cwd: $!"; + } +} + # --initdb must fail early with a clear message if initdb is not present in the # new cluster's bin directory. Point --new-bindir at an empty directory and use # a fresh (nonexistent) new data directory so we reach the initdb-present check. @@ -188,7 +246,8 @@ [qr/^$/], '--initdb fails early when initdb is missing from the new bindir'); -# --check --initdb performs validation without creating anything. +# --check --initdb performs all source-side validation without creating a +# target cluster. command_checks_all( [ 'pg_upgrade', '--no-sync', @@ -203,32 +262,188 @@ '--check', ], 0, - [qr/created with settings matching the old cluster/], + [qr/Source cluster compatibility checks passed/], [qr/^$/], - '--check --initdb validates without creating the cluster'); + '--check --initdb runs source-side checks without creating the cluster'); # Verify that --check --initdb didn't create anything. ok(!-d $newnode->data_dir . '_dry_run', '--check --initdb does not create the new cluster directory'); +ok( !-d $newnode->data_dir . '_dry_run.initdb_log', + '--check --initdb does not leave a sibling log directory'); + +# Without -B, --initdb must derive the new bindir from the original argv[0], +# even when that directory is not in PATH. +SKIP: +{ + skip "restricted PATH test is not portable to Windows", 3 if $windows_os; + + local %ENV = %ENV; + $ENV{PATH} = '/usr/bin:/bin'; + command_checks_all( + [ + abs_path("$newbindir/pg_upgrade"), '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir . '_no_new_bindir', + '--old-bindir' => $oldbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + '--check', + ], + 0, + [qr/Source cluster compatibility checks passed/], + [qr/^$/], + '--initdb derives the new bindir from an absolute argv[0]'); +} + +# A live source is valid for --check --initdb, just as it is for ordinary +# --check. No target server is started, so the ports can be equal. +# pg_upgrade must neither stop nor modify the source server. +SKIP: +{ + skip "Timing issues with live server detection on Windows", 4 + if $windows_os; + + $oldnode->start; + command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir . '_live_check', + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $oldnode->port, + '--initdb', + '--check', + ], + 0, + [qr/Source cluster compatibility checks passed/], + [qr/^$/], + '--check --initdb accepts a live source with equal old and new ports' + ); + is($oldnode->safe_psql('postgres', 'SELECT 1'), + '1', 'live source remains running after --check --initdb'); + $oldnode->stop; +} + +# Force a failure while the target postmaster is running. This verifies that +# the exit handlers stop the postmaster before cleaning its data directory. +my $test_library = $ENV{TEST_EXT_LIB} + or die "could not get the test extension library path"; +my $missing_library = + PostgreSQL::Test::Utils::tempdir() . '/' . basename($test_library); +copy($test_library, $missing_library) + or die "could not copy $test_library to $missing_library: $!"; +my $sql_library = $missing_library =~ s/'/''/gr; + +$oldnode->start; +$oldnode->safe_psql('postgres', + "CREATE FUNCTION missing_upgrade_library() RETURNS void " + . "AS '$sql_library', 'test_ext' LANGUAGE C"); +$oldnode->stop; +unlink($missing_library) or die "could not remove $missing_library: $!"; + +# On failure after initdb, preserve an empty directory supplied by the +# operator, including its original mode, but remove everything initdb added. +my $existing_target = $newnode->data_dir . '_existing_empty'; +mkdir($existing_target, 0711) or die "could not create $existing_target: $!"; +chmod(0711, $existing_target) + or die "could not set mode on $existing_target: $!"; +my $existing_target_mode = (stat($existing_target))[2] & 0777; -# -O passes postmaster-only options, which initdb does not accept, so the -# combination is rejected during option parsing rather than forwarded. command_checks_all( [ 'pg_upgrade', '--no-sync', '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $newnode->data_dir . '_nonexistent', + '--new-datadir' => $existing_target, + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + ], + 1, + [qr/references loadable libraries that are missing/], + [qr/^$/], + 'failed --initdb stops the target and preserves an existing directory'); + +ok(-d $existing_target, 'operator-created target directory still exists'); +opendir(my $target_dir, $existing_target) + or die "could not open $existing_target: $!"; +my @target_entries = grep { $_ ne '.' && $_ ne '..' } readdir($target_dir); +closedir($target_dir); +is_deeply(\@target_entries, [], + 'operator-created target directory is empty after cleanup'); + +SKIP: +{ + skip "unix-style permissions not supported on Windows", 1 + if ($windows_os || $Config::Config{osname} eq 'cygwin'); + + my $mode_after_failure = (stat($existing_target))[2] & 0777; + is($mode_after_failure, $existing_target_mode, + 'operator-created target directory mode is restored'); +} + +# Starting another server on the target port confirms that the failed +# pg_upgrade did not leave its temporary target postmaster running. +$newnode->start; +is($newnode->safe_psql('postgres', 'SELECT 1'), + '1', 'target port is free after failed --initdb cleanup'); +$newnode->stop; + +# Verify that the dry run executes compatibility checks rather than merely +# printing an initdb command. +$oldnode->start; +$oldnode->safe_psql('postgres', + 'DROP FUNCTION missing_upgrade_library(); CREATE TABLE bad (c regproc)'); +$oldnode->stop; + +my $incompatible_target = $newnode->data_dir . '_incompatible_dry_run'; +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $incompatible_target, + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + '--initdb', + '--check', + ], + 1, + [qr/failed check: Checking for reg\* data types in user tables/], + [qr/^$/], + '--check --initdb rejects an incompatible source cluster'); +ok(!-d $incompatible_target, + 'failed --check --initdb does not create the target directory'); + +# Conversely, pg_upgrade owns a target directory that did not exist before +# initdb and removes it completely after a later failure. +my $created_target = $newnode->data_dir . '_created_then_failed'; +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $created_target, '--old-bindir' => $oldbindir, '--new-bindir' => $newbindir, '--socketdir' => $newnode->host, '--old-port' => $oldnode->port, '--new-port' => $newnode->port, '--initdb', - '--new-options' => '-c work_mem=1MB', ], 1, - [qr/options -O\/--new-options and --initdb cannot be used together/], + [qr/failed check: Checking for reg\* data types in user tables/], [qr/^$/], - '--initdb and -O cannot be used together'); + 'failed --initdb removes a target it created'); +ok(!-d $created_target, 'pg_upgrade-created target directory was removed'); done_testing(); From 09cbfb1eafb94848bebe37a26728c5f228745e3a Mon Sep 17 00:00:00 2001 From: Bohyun Lee Date: Wed, 7 Oct 2026 19:28:10 +0200 Subject: [PATCH] pg_upgrade: add --initdb-options and run full compatibility checks Forward safely parsed and quoted initialization options while protecting settings managed by pg_upgrade and link/swap permissions. Initialize and retain the target for --check --initdb, remove custom rollback handling, and align the documentation and TAP coverage with the resulting behavior. --- doc/src/sgml/ref/pgupgrade.sgml | 107 +++- src/bin/pg_upgrade/Makefile | 2 +- src/bin/pg_upgrade/check.c | 7 +- src/bin/pg_upgrade/controldata.c | 30 +- src/bin/pg_upgrade/exec.c | 16 +- src/bin/pg_upgrade/option.c | 223 +++++++- src/bin/pg_upgrade/pg_upgrade.c | 204 ++------ src/bin/pg_upgrade/pg_upgrade.h | 16 +- src/bin/pg_upgrade/t/009_initdb_option.pl | 608 ++++++++++++---------- 9 files changed, 721 insertions(+), 492 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 21f883886dd30..5bc543717de54 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -106,7 +106,9 @@ PostgreSQL documentation - check clusters only, don't change any data + check clusters without performing the upgrade. + If is also specified, create the new cluster + before checking it. @@ -284,31 +286,98 @@ PostgreSQL documentation It must not overlap the pg_upgrade_output.d directory in the current working directory: neither directory may be equal to or contain the other. + The new data directory may be inside the old data directory; in that + case, the script to delete the old cluster is not generated. When combined with /, - runs source-cluster compatibility checks, - validates the new binaries and the empty or nonexistent target - directory, and reports the initdb command it would - run without creating the new cluster. - Checks requiring an initialized target cluster or a writable target - data directory, including loading required libraries and testing the - selected file transfer mode, are deferred until the real upgrade. + creates the new cluster and runs the normal + compatibility checks on both clusters without performing the upgrade. + The initialized new cluster is retained whether the checks succeed or + fail. After a successful check, run pg_upgrade + without , , or + to upgrade using that cluster. / continues to pass options to the new cluster's server process; those options are not - passed to initdb. To use additional - initdb options, create the new cluster manually and - omit . + passed to initdb. Use + for additional initialization options. - If initdb or a compatibility check fails, - pg_upgrade removes the newly created files. - A directory created by pg_upgrade is also - removed. - A pre-existing empty directory is preserved, and its original - permissions are restored on supported Unix systems. + If initialization fails, initdb performs its normal + cleanup. Once initialization succeeds, pg_upgrade + leaves the new data directory and any separate WAL directory in place + if a later step fails, just as it does without . + See for recovery instructions. + To retry with , the new data and WAL directories + must again be empty or nonexistent. + + + + + + options + + + Pass additional arguments to initdb. Requires + . This option can be repeated; arguments are + appended in command-line order. For example: + +pg_upgrade --initdb --initdb-options='-c huge_pages=off --waldir=/wal/new' ... + + Settings supplied with initdb's + are saved in the new postgresql.conf and can also + affect initialization itself. In particular, a library specified in + shared_preload_libraries must support being loaded + during initialization, as well as by the new server during upgrade. + + + Within options, spaces and tabs separate + arguments. Single or double quotes group text into one argument and + are removed; adjacent quoted and unquoted text forms one argument. + Single quotes preserve their contents literally. Elsewhere, a + backslash escapes a following quote, backslash, space, or tab, and is + preserved before other characters. Unmatched quotes, a trailing + unquoted backslash, and newline or carriage-return characters are + rejected. No shell expansion is performed; for example, + $libdir is passed literally. The caller's shell + quoting rules still apply to the outer command line. Arguments can + appear in command output, log files, and process listings, so use + file-based inputs such as for passwords. + + + Options that replace the data directory, bootstrap superuser, + checksum setting, WAL segment size, or inherited encoding and locale + settings are not allowed. Configuration settings that redirect the data or + configuration files (data_directory, + config_file, hba_file, and + ident_file) are also rejected, as are options + that do not initialize a cluster, such as . + Run initdb separately for setups requiring these + options. + + + An explicit enables group access + even if the old cluster does not allow it; otherwise the old cluster's + access mode is used. With or , group + access cannot be enabled unless the old cluster already allows it, + because these transfer modes preserve the old file and directory + permissions. + + + Authentication options, including and + , are passed unchanged. Provide credentials + accepted by both clusters, for example through a protected + password file. + pg_upgrade does not weaken authentication + rules to obtain access. + + + pg_upgrade checks argument quoting and + conflicts with its managed settings. Other options, their values, + and paths such as are validated by + initdb, including with . @@ -674,9 +743,7 @@ pg_upgrade.exe You can use the same port number for both clusters when doing an upgrade because the old and new clusters will not be running at the same time. However, when checking an old running server, the old - and new port numbers must be different unless - is combined with , which does not start a new - server. + and new port numbers must be different. diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile index 771addb675a6d..9d8602a9a7284 100644 --- a/src/bin/pg_upgrade/Makefile +++ b/src/bin/pg_upgrade/Makefile @@ -3,7 +3,7 @@ PGFILEDESC = "pg_upgrade - an in-place binary upgrade utility" PGAPPICON = win32 -EXTRA_INSTALL=contrib/test_decoding src/test/modules/dummy_seclabel src/test/modules/test_extensions +EXTRA_INSTALL=contrib/test_decoding src/test/modules/dummy_seclabel src/test/modules/test_extensions src/test/modules/test_slru subdir = src/bin/pg_upgrade top_builddir = ../../.. diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 098eeab34ea27..150c4fc34b9b2 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -30,6 +30,7 @@ static void check_for_incompatible_polymorphics(ClusterInfo *cluster); static void check_for_tables_with_oids(ClusterInfo *cluster); static void check_for_not_null_inheritance(ClusterInfo *cluster); static void check_for_gist_inet_ops(ClusterInfo *cluster); +static void check_for_new_tablespace_dir(void); static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster); static void check_for_unicode_update(ClusterInfo *cluster); static void check_new_cluster_replication_slots(void); @@ -740,7 +741,7 @@ check_new_cluster(void) check_for_prepared_transactions(&new_cluster); - check_new_cluster_tablespace_dirs(); + check_for_new_tablespace_dir(); check_new_cluster_replication_slots(); @@ -922,8 +923,8 @@ check_new_cluster_is_empty(void) * This allows the failure to be detected at check time, rather than * during schema restore. */ -void -check_new_cluster_tablespace_dirs(void) +static void +check_for_new_tablespace_dir(void) { int tblnum; char new_tablespace_dir[MAXPGPATH]; diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c index 6cdd7fa570d91..e83f2d963cdfb 100644 --- a/src/bin/pg_upgrade/controldata.c +++ b/src/bin/pg_upgrade/controldata.c @@ -607,26 +607,6 @@ get_control_data(ClusterInfo *cluster) } -/* - * check_old_control_data() - * - * check source control-data states that are unsafe for any upgrade - */ -void -check_old_control_data(ControlData *oldctrl) -{ - /* - * If data checksums are in any in-progress state then disallow the - * upgrade. The user should either let the process finish, or turn off - * data checksums, before retrying. - */ - if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_ON) - pg_fatal("data checksums are being enabled in the old cluster"); - if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_OFF) - pg_fatal("data checksums are being disabled in the old cluster"); -} - - /* * check_control_data() * @@ -673,7 +653,15 @@ check_control_data(ControlData *oldctrl, * check_for_isn_and_int8_passing_mismatch(). */ - check_old_control_data(oldctrl); + /* + * If data checksums are in any in-progress state then disallow the + * upgrade. The user should either let the process finish, or turn off + * data checksums, before retrying. + */ + if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_ON) + pg_fatal("data checksums are being enabled in the old cluster"); + if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_OFF) + pg_fatal("data checksums are being disabled in the old cluster"); /* * We might eventually allow upgrades from checksum to no-checksum diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index 347dd682347a6..66afb03360ad8 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -251,7 +251,7 @@ pid_lock_file_exists(const char *datadir) * NOTE: May update the values of all parameters */ void -verify_directories(bool check_new_data_dir) +verify_directories(void) { #ifndef WIN32 if (access(".", R_OK | W_OK | X_OK) != 0) @@ -265,19 +265,7 @@ verify_directories(bool check_new_data_dir) /* --initdb validates the new binaries before creating the cluster. */ if (!user_opts.initdb_new_cluster) check_bin_dir(&new_cluster, true); - if (check_new_data_dir) - check_data_dir(&new_cluster); - else - { - /* - * --check --initdb does not initialize the new cluster. Fill the - * target fields needed by the source-cluster compatibility checks. - */ - new_cluster.major_version = new_cluster.bin_version; - new_cluster.major_version_str = pg_strdup(PG_MAJORVERSION); - new_cluster.controldata.cat_ver = CATALOG_VERSION_NO; - new_cluster.controldata.float8_pass_by_value = FLOAT8PASSBYVAL; - } + check_data_dir(&new_cluster); } diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index daaf48d47bc9c..d24ac95caea2c 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -9,6 +9,8 @@ #include "postgres_fe.h" +#include + #ifdef WIN32 #include #endif @@ -20,6 +22,9 @@ #include "utils/pidfile.h" static void usage(void); +static void parse_initdb_options(const char *options); +static void validate_initdb_options(void); +static void check_initdb_setting(const char *setting); static void check_required_directory(char **dirpath, const char *envVarName, bool useCwd, const char *cmdLineOption, const char *description, @@ -64,6 +69,7 @@ parseCommandLine(int argc, char *argv[]) {"set-char-signedness", required_argument, NULL, 6}, {"swap", no_argument, NULL, 7}, {"initdb", no_argument, NULL, 8}, + {"initdb-options", required_argument, NULL, 9}, {NULL, 0, NULL, 0} }; @@ -239,6 +245,11 @@ parseCommandLine(int argc, char *argv[]) user_opts.initdb_new_cluster = true; break; + case 9: + user_opts.initdb_options_given = true; + parse_initdb_options(optarg); + break; + default: fprintf(stderr, _("Try \"%s --help\" for more information.\n"), os_info.progname); @@ -249,6 +260,13 @@ parseCommandLine(int argc, char *argv[]) if (optind < argc) pg_fatal("too many command-line arguments (first is \"%s\")", argv[optind]); + if (user_opts.initdb_options_given) + { + if (!user_opts.initdb_new_cluster) + pg_fatal("--initdb-options requires --initdb"); + validate_initdb_options(); + } + if (!user_opts.sync_method) user_opts.sync_method = pg_strdup("fsync"); @@ -305,6 +323,208 @@ parseCommandLine(int argc, char *argv[]) } +/* + * Split --initdb-options without invoking a shell. Single quotes preserve + * every character. Outside single quotes, backslashes escape only quotes, + * backslashes, spaces, and tabs, so ordinary Windows path separators survive. + */ +static void +parse_initdb_options(const char *options) +{ + const char *p = options; + char *argument; + + if (strpbrk(options, "\r\n") != NULL) + pg_fatal("--initdb-options must not contain a newline or carriage return"); + + argument = pg_malloc(strlen(options) + 1); + while (*p) + { + char *out = argument; + char quote = '\0'; + + while (*p == ' ' || *p == '\t') + p++; + if (*p == '\0') + break; + + while (*p) + { + if (quote == '\0' && (*p == ' ' || *p == '\t')) + break; + if (*p == quote) + { + quote = '\0'; + p++; + } + else if (quote != '\'' && *p == '\\') + { + p++; + if (*p == '\0') + pg_fatal("trailing backslash in --initdb-options"); + if (*p == '\'' || *p == '"' || *p == '\\' || + *p == ' ' || *p == '\t') + *out++ = *p++; + else + *out++ = '\\'; + } + else if (quote == '\0' && (*p == '\'' || *p == '"')) + quote = *p++; + else + *out++ = *p++; + } + + if (quote != '\0') + pg_fatal("unterminated quote in --initdb-options"); + *out = '\0'; + + if (user_opts.num_initdb_options == INT_MAX - 1) + pg_fatal("too many arguments in --initdb-options"); + user_opts.initdb_options = + pg_realloc_array(user_opts.initdb_options, char *, + (size_t) user_opts.num_initdb_options + 1); + user_opts.initdb_options[user_opts.num_initdb_options++] = + pg_strdup(argument); + } + pg_free(argument); +} + + +/* + * Reject settings that redirect the new cluster's data or configuration + * files, which pg_upgrade expects to find in the new data directory. + */ +static void +check_initdb_setting(const char *setting) +{ + char *name = pg_strdup(setting); + char *equals = strchr(name, '='); + + if (equals) + *equals = '\0'; + + /* Treat hyphens as underscores, as ParseLongOption() does. */ + for (char *p = name; *p; p++) + if (*p == '-') + *p = '_'; + + if (pg_strcasecmp(name, "data_directory") == 0 || + pg_strcasecmp(name, "config_file") == 0 || + pg_strcasecmp(name, "hba_file") == 0 || + pg_strcasecmp(name, "ident_file") == 0) + pg_fatal("initdb setting \"%s\" cannot be used with --initdb", name); + + pg_free(name); +} + + +/* + * Check for options that conflict with pg_upgrade, leaving other validation + * to initdb. Parse after the pg_upgrade options, since getopt_long() uses + * global state. Keep the option table consistent with initdb so getopt_long() + * knows which options take arguments. + */ +static void +validate_initdb_options(void) +{ + static struct option long_options[] = { + {"pgdata", required_argument, NULL, 'D'}, + {"encoding", required_argument, NULL, 'E'}, + {"locale", required_argument, NULL, 1}, + {"lc-collate", required_argument, NULL, 2}, + {"lc-ctype", required_argument, NULL, 3}, + {"lc-monetary", required_argument, NULL, 4}, + {"lc-numeric", required_argument, NULL, 5}, + {"lc-time", required_argument, NULL, 6}, + {"lc-messages", required_argument, NULL, 7}, + {"no-locale", no_argument, NULL, 8}, + {"text-search-config", required_argument, NULL, 'T'}, + {"auth", required_argument, NULL, 'A'}, + {"auth-local", required_argument, NULL, 10}, + {"auth-host", required_argument, NULL, 11}, + {"pwprompt", no_argument, NULL, 'W'}, + {"pwfile", required_argument, NULL, 9}, + {"username", required_argument, NULL, 'U'}, + {"help", no_argument, NULL, 22}, + {"version", no_argument, NULL, 'V'}, + {"debug", no_argument, NULL, 'd'}, + {"show", no_argument, NULL, 's'}, + {"noclean", no_argument, NULL, 'n'}, + {"no-clean", no_argument, NULL, 'n'}, + {"nosync", no_argument, NULL, 'N'}, + {"no-sync", no_argument, NULL, 'N'}, + {"no-instructions", no_argument, NULL, 13}, + {"set", required_argument, NULL, 'c'}, + {"sync-only", no_argument, NULL, 'S'}, + {"waldir", required_argument, NULL, 'X'}, + {"wal-segsize", required_argument, NULL, 12}, + {"data-checksums", no_argument, NULL, 'k'}, + {"allow-group-access", no_argument, NULL, 'g'}, + {"discard-caches", no_argument, NULL, 14}, + {"locale-provider", required_argument, NULL, 15}, + {"builtin-locale", required_argument, NULL, 16}, + {"icu-locale", required_argument, NULL, 17}, + {"icu-rules", required_argument, NULL, 18}, + {"sync-method", required_argument, NULL, 19}, + {"no-data-checksums", no_argument, NULL, 20}, + {"no-sync-data-files", no_argument, NULL, 21}, + {NULL, 0, NULL, 0} + }; + int argc = user_opts.num_initdb_options + 1; + char **argv = pg_malloc_array(char *, (size_t) argc + 1); + int option; + int optindex; + int save_opterr = opterr; + + argv[0] = pg_strdup(os_info.progname); + for (int i = 1; i < argc; i++) + argv[i] = user_opts.initdb_options[i - 1]; + argv[argc] = NULL; + optind = 1; + opterr = 0; +#ifdef HAVE_INT_OPTRESET + optreset = 1; +#endif + while ((option = getopt_long(argc, argv, "A:c:dD:E:gkL:nNsST:U:VWX:", + long_options, &optindex)) != -1) + { + switch (option) + { + case 1: + case 2: + case 3: + case 8: + case 12: + case 15: + case 16: + case 17: + case 18: + case 20: + case 22: + pg_fatal("initdb option \"--%s\" cannot be used with --initdb", + long_options[optindex].name); + case 'D': + case 'E': + case 'k': + case 's': + case 'S': + case 'U': + case 'V': + pg_fatal("initdb option \"-%c\" cannot be used with --initdb", option); + case 'g': + user_opts.initdb_allow_group_access = true; + break; + case 'c': + check_initdb_setting(optarg); + break; + } + } + opterr = save_opterr; + pg_free(argv[0]); + pg_free(argv); +} + + static void usage(void) { @@ -334,7 +554,8 @@ usage(void) printf(_(" --copy copy files to new cluster (default)\n")); printf(_(" --copy-file-range copy files to new cluster with copy_file_range\n")); printf(_(" --initdb create the new cluster with initdb before\n" - " upgrading (settings derived from old cluster)\n")); + " checking or upgrading, using old cluster settings\n")); + printf(_(" --initdb-options=OPTIONS options to pass to initdb (requires --initdb)\n")); printf(_(" --no-statistics do not import statistics from old cluster\n")); printf(_(" --set-char-signedness=OPTION set new cluster char signedness to \"signed\" or\n" " \"unsigned\"\n")); diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 55a30f0f9fdde..5ced9a1f7af12 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -69,14 +69,13 @@ static void create_new_objects(void); static void copy_xact_xlog_xid(void); static void set_frozenxids(void); static void make_outputdirs(const char *output_root); -static void setup(char *argv0, bool check_new_data_dir); +static void setup(char *argv0); static void resolve_new_bindir(const char *argv0); static void prepare_new_cluster_initdb(const char *argv0); static void check_new_cluster_initdb_target(void); static void get_old_cluster_initdb_info(void); static void build_new_cluster_initdb_cmd(PQExpBuffer cmd); static void create_new_cluster_via_initdb(void); -static void check_new_cluster_via_initdb(char *argv0); static void create_logical_replication_slots(void); static void create_conflict_detection_slot(void); @@ -84,10 +83,6 @@ ClusterInfo old_cluster, new_cluster; OSInfo os_info; -static bool new_cluster_cleanup_required = false; -static bool new_cluster_pgdata_existed = false; -static mode_t new_cluster_pgdata_mode; - char *output_files[] = { SERVER_LOG_FILE, #ifdef WIN32 @@ -133,12 +128,25 @@ main(int argc, char **argv) old_cluster.pgdata); umask(pg_mode_mask); +#if !defined(WIN32) && !defined(__CYGWIN__) + + /* + * Link and swap preserve the old file permissions, so enabling group + * access would leave transferred files unreadable by the group. + * Changing permissions on hard links would also change the old + * cluster. + */ + if (user_opts.initdb_allow_group_access && + pg_dir_create_mode != PG_DIR_MODE_GROUP && + (user_opts.transfer_mode == TRANSFER_MODE_LINK || + user_opts.transfer_mode == TRANSFER_MODE_SWAP)) + pg_fatal("cannot enable group access with %s when the old cluster does not allow group access", + user_opts.transfer_mode == TRANSFER_MODE_LINK ? "--link" : "--swap"); +#endif + /* The new cluster has not been initialized yet. */ make_outputdirs("."); - if (user_opts.check) - check_new_cluster_via_initdb(argv[0]); /* exits, never returns */ - create_new_cluster_via_initdb(); } @@ -159,7 +167,7 @@ main(int argc, char **argv) if (!user_opts.initdb_new_cluster) make_outputdirs(new_cluster.pgdata); - setup(argv[0], true); + setup(argv[0]); output_check_banner(); @@ -179,9 +187,6 @@ main(int argc, char **argv) check_new_cluster(); report_clusters_compatible(); - /* Keep the new cluster on failure after compatibility checks pass. */ - new_cluster_cleanup_required = false; - pg_log(PG_REPORT, "\n" "Performing Upgrade\n" @@ -419,51 +424,6 @@ resolve_new_bindir(const char *argv0) } -/* - * new_cluster_cleanup_atexit() - * - * Clean up the new cluster if the run fails before compatibility checks - * pass. Preserve a target directory that existed before pg_upgrade. - */ -static void -new_cluster_cleanup_atexit(void) -{ - int dir_status; - - if (!new_cluster_cleanup_required) - return; - - dir_status = pg_check_dir(new_cluster.pgdata); - if (dir_status == 0) - return; - if (dir_status < 0) - { - pg_log(PG_WARNING, "could not access new cluster data directory \"%s\": %m", - new_cluster.pgdata); - return; - } - - if (new_cluster_pgdata_existed) - { - pg_log(PG_REPORT, "removing contents of new cluster data directory \"%s\"", - new_cluster.pgdata); - (void) rmtree(new_cluster.pgdata, false); - -#if !defined(WIN32) && !defined(__CYGWIN__) - if (chmod(new_cluster.pgdata, new_cluster_pgdata_mode) != 0) - pg_log(PG_WARNING, "could not restore permissions of directory \"%s\": %m", - new_cluster.pgdata); -#endif - } - else - { - pg_log(PG_REPORT, "removing new cluster data directory \"%s\"", - new_cluster.pgdata); - (void) rmtree(new_cluster.pgdata, true); - } -} - - /* * Validate the new binaries and target directory before creating output * files or starting either cluster. @@ -471,64 +431,30 @@ new_cluster_cleanup_atexit(void) static void prepare_new_cluster_initdb(const char *argv0) { - char initdb_path[MAXPGPATH]; - + check_pghost_envvar(); resolve_new_bindir(argv0); - - /* Verify initdb is present in the new cluster's bin directory. */ - snprintf(initdb_path, sizeof(initdb_path), "%s/initdb", new_cluster.bindir); - if (validate_exec(initdb_path) != 0) - pg_fatal("could not find \"initdb\" in \"%s\": %m\n" - "The --initdb option requires initdb to be present in the new cluster's bin directory.", - new_cluster.bindir); - check_bin_dir(&new_cluster, true); check_new_cluster_initdb_target(); } /* - * Refuse to overwrite an existing target and remember whether an empty target - * directory must be preserved if a later step fails. + * Require an empty or nonexistent new data directory and reject overlap + * with pg_upgrade_output.d. */ static void check_new_cluster_initdb_target(void) { - char verfile[MAXPGPATH]; char *absolute_pgdata; char *absolute_outputdir; - struct stat st; - int dir_status; + int dir_status = pg_check_dir(new_cluster.pgdata); - snprintf(verfile, sizeof(verfile), "%s/PG_VERSION", new_cluster.pgdata); - if (stat(verfile, &st) == 0) - pg_fatal("new cluster data directory \"%s\" already contains a database system; " + if (dir_status > 1) + pg_fatal("new cluster data directory \"%s\" is not empty; " "--initdb requires an empty or nonexistent directory", new_cluster.pgdata); - - dir_status = pg_check_dir(new_cluster.pgdata); - switch (dir_status) - { - case 0: - new_cluster_pgdata_existed = false; - break; - case 1: - new_cluster_pgdata_existed = true; - if (stat(new_cluster.pgdata, &st) != 0) - pg_fatal("could not stat directory \"%s\": %m", - new_cluster.pgdata); - new_cluster_pgdata_mode = st.st_mode; - break; - case 2: - case 3: - case 4: - pg_fatal("new cluster data directory \"%s\" is not empty; " - "--initdb requires an empty or nonexistent directory", - new_cluster.pgdata); - default: - pg_fatal("could not access directory \"%s\": %m", - new_cluster.pgdata); - } + else if (dir_status < 0) + pg_fatal("could not access directory \"%s\": %m", new_cluster.pgdata); absolute_pgdata = make_absolute_path(new_cluster.pgdata); absolute_outputdir = make_absolute_path(BASE_OUTPUTDIR); @@ -545,9 +471,7 @@ check_new_cluster_initdb_target(void) /* - * Collect the old control and template0 settings needed by initdb. This is - * used only by a real upgrade. The dry-run path obtains the same information - * while running the normal old-cluster checks. + * Read the old cluster's control data and template0 settings for initdb. */ static void get_old_cluster_initdb_info(void) @@ -558,34 +482,34 @@ get_old_cluster_initdb_info(void) if (!old_cluster.sockdir) old_cluster.sockdir = user_opts.socketdir ? user_opts.socketdir : "."; - /* - * The old server must be shut down. The template0 read below starts a - * postmaster on the old cluster, and get_control_data() runs pg_resetwal -n, - * both of which need exclusive access to the old data directory. A stale - * lock file is tolerated as setup() does. - */ + /* Allow a running old server for --check, as setup() does. */ if (pid_lock_file_exists(old_cluster.pgdata)) { if (start_postmaster(&old_cluster, false)) stop_postmaster(false); - else + else if (!user_opts.check) pg_fatal("There seems to be a postmaster servicing the old cluster.\n" "Please shutdown that postmaster and try again."); + else + user_opts.live_check = true; } + get_sock_dir(&old_cluster); get_control_data(&old_cluster); prep_status("Examining old cluster settings"); - start_postmaster(&old_cluster, true); + if (!user_opts.live_check) + start_postmaster(&old_cluster, true); get_template0_info(&old_cluster); - stop_postmaster(false); + if (!user_opts.live_check) + stop_postmaster(false); check_ok(); } /* - * Format the initdb command from information already collected about the old - * cluster. Callers either execute it or report it for --check. + * Build the initdb command using the old cluster's settings and any + * arguments supplied with --initdb-options. */ static void build_new_cluster_initdb_cmd(PQExpBuffer cmd) @@ -652,30 +576,27 @@ build_new_cluster_initdb_cmd(PQExpBuffer cmd) } } + for (int i = 0; i < user_opts.num_initdb_options; i++) + { + appendPQExpBufferChar(cmd, ' '); + appendShellString(cmd, user_opts.initdb_options[i]); + } + check_ok(); } -/* - * create_new_cluster_via_initdb() - * - * Create the new cluster with initdb. Register cleanup before the first - * start_postmaster() call so the server's exit handler runs before directory - * cleanup. Enable cleanup immediately before running initdb. - */ +/* Initialize the new cluster before checking compatibility with the old cluster. */ static void create_new_cluster_via_initdb(void) { PQExpBufferData cmd; - atexit(new_cluster_cleanup_atexit); - get_old_cluster_initdb_info(); build_new_cluster_initdb_cmd(&cmd); prep_status("Creating new cluster with initdb"); - new_cluster_cleanup_required = true; exec_prog(UTILITY_LOG_FILE, NULL, true, true, "%s", cmd.data); termPQExpBuffer(&cmd); @@ -683,41 +604,10 @@ create_new_cluster_via_initdb(void) } -/* - * check_new_cluster_via_initdb() - * - * Dry-run --check --initdb path: build the initdb command with - * build_new_cluster_initdb_cmd() but do not execute it. Run source-cluster - * compatibility checks, report the command, and exit. - */ -static void -check_new_cluster_via_initdb(char *argv0) -{ - PQExpBufferData cmd; - - setup(argv0, false); - - output_check_banner(); - check_cluster_versions(); - get_sock_dir(&old_cluster); - - get_control_data(&old_cluster); - check_old_control_data(&old_cluster.controldata); - - check_and_dump_old_cluster(); - check_new_cluster_tablespace_dirs(); - build_new_cluster_initdb_cmd(&cmd); - - pg_log(PG_REPORT, _("The following initdb command would be run to create the new cluster:\n %s"), cmd.data); - pg_log(PG_REPORT, _("Source cluster compatibility checks passed.")); - termPQExpBuffer(&cmd); - cleanup_output_dirs(); - exit(0); -} static void -setup(char *argv0, bool check_new_data_dir) +setup(char *argv0) { /* * make sure the user has a clean environment, otherwise, we may confuse @@ -732,7 +622,7 @@ setup(char *argv0, bool check_new_data_dir) */ resolve_new_bindir(argv0); - verify_directories(check_new_data_dir); + verify_directories(); /* no postmasters should be running, except for a live check */ if (pid_lock_file_exists(old_cluster.pgdata)) diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 3651796886426..698873d879416 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -314,7 +314,8 @@ typedef struct */ typedef struct { - bool check; /* check clusters only, don't change any data */ + bool check; /* check clusters without performing the + * upgrade */ bool live_check; /* check clusters only, old server is running */ bool do_sync; /* flush changes to disk */ transferMode transfer_mode; /* copy files or link them? */ @@ -325,9 +326,12 @@ typedef struct int char_signedness; /* default char signedness: -1 for initial * value, 1 for "signed" and 0 for * "unsigned" */ - bool initdb_new_cluster; /* run initdb to create the new cluster - * before upgrading, instead of requiring - * the user to have created it manually */ + bool initdb_new_cluster; /* run initdb before compatibility checks */ + bool initdb_options_given; /* --initdb-options given, even if + * empty */ + bool initdb_allow_group_access; /* --allow-group-access */ + char **initdb_options; /* additional arguments to initdb */ + int num_initdb_options; } UserOpts; typedef struct @@ -368,7 +372,6 @@ extern OSInfo os_info; void output_check_banner(void); void check_and_dump_old_cluster(void); void check_new_cluster(void); -void check_new_cluster_tablespace_dirs(void); void report_clusters_compatible(void); void issue_warnings_and_set_wal_level(void); void output_completion_banner(char *deletion_script_file_name); @@ -381,7 +384,6 @@ void create_script_for_old_cluster_deletion(char **deletion_script_file_name); void get_control_data(ClusterInfo *cluster); void check_control_data(ControlData *oldctrl, ControlData *newctrl); -void check_old_control_data(ControlData *oldctrl); void disable_old_cluster(transferMode transfer_mode); @@ -397,7 +399,7 @@ void generate_old_dump(void); bool exec_prog(const char *log_filename, const char *opt_log_file, bool report_error, bool exit_on_error, const char *fmt, ...) pg_attribute_printf(5, 6); void check_bin_dir(ClusterInfo *cluster, bool check_versions); -void verify_directories(bool check_new_data_dir); +void verify_directories(void); bool pid_lock_file_exists(const char *datadir); diff --git a/src/bin/pg_upgrade/t/009_initdb_option.pl b/src/bin/pg_upgrade/t/009_initdb_option.pl index d989dfb0c9d9f..395c33e48da2c 100644 --- a/src/bin/pg_upgrade/t/009_initdb_option.pl +++ b/src/bin/pg_upgrade/t/009_initdb_option.pl @@ -1,25 +1,18 @@ # Copyright (c) 2026, PostgreSQL Global Development Group -# Test the --initdb option of pg_upgrade: pg_upgrade creates the new cluster -# itself via initdb, instead of requiring the user to have run initdb first. +# Test pg_upgrade's --initdb and --initdb-options options. use strict; use warnings FATAL => 'all'; use Config; -use Cwd qw(abs_path); -use File::Basename qw(basename); -use File::Copy qw(copy); -use File::Path qw(rmtree); +use Cwd qw(abs_path); use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; -# Initialize and populate the old cluster. -# -# Use settings that --initdb must carry over to the new cluster: group access, -# disabled data checksums (initdb enables them by default since PG18), a -# non-default WAL segment size, and the C locale. +# Use nondefault settings to verify that --initdb carries them over to the +# new cluster. my $oldnode = PostgreSQL::Test::Cluster->new('old_node'); $oldnode->init( extra => [ @@ -36,30 +29,62 @@ my $rows_before = $oldnode->safe_psql('postgres', 'SELECT count(*) FROM t'); is($rows_before, '100', 'old cluster has expected rows before upgrade'); -# Record the old cluster's settings so we can compare them after the upgrade. -my $old_checksums = $oldnode->safe_psql('postgres', 'SHOW data_checksums'); -my $old_wal_segsize = - $oldnode->safe_psql('postgres', 'SHOW wal_segment_size'); -my $old_encoding = $oldnode->safe_psql('postgres', - "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'" -); -my $old_collate = $oldnode->safe_psql('postgres', - "SELECT datcollate FROM pg_database WHERE datname = 'template0'"); -my $old_ctype = $oldnode->safe_psql('postgres', - "SELECT datctype FROM pg_database WHERE datname = 'template0'"); -my $old_provider = $oldnode->safe_psql('postgres', - "SELECT datlocprovider FROM pg_database WHERE datname = 'template0'"); +# Capture the old settings for the post-upgrade comparison. +my %setting_queries = ( + data_checksums => 'SHOW data_checksums', + wal_segment_size => 'SHOW wal_segment_size', + encoding => + "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'", + collation => + "SELECT datcollate FROM pg_database WHERE datname = 'template0'", + ctype => "SELECT datctype FROM pg_database WHERE datname = 'template0'", + provider => + "SELECT datlocprovider FROM pg_database WHERE datname = 'template0'",); +my %old_settings; +for my $setting (sort keys %setting_queries) +{ + $old_settings{$setting} = + $oldnode->safe_psql('postgres', $setting_queries{$setting}); +} $oldnode->stop; -# Create the new node object but do NOT init() it: pg_upgrade --initdb is -# responsible for creating the data directory. Only new() runs, which -# allocates the port/host/basedir the framework needs. +# Leave the new cluster uninitialized so pg_upgrade --initdb creates it. my $newnode = PostgreSQL::Test::Cluster->new('new_node'); my $oldbindir = $oldnode->config_data('--bindir'); my $newbindir = $newnode->config_data('--bindir'); -# Sanity: the new data directory must not exist yet. +sub upgrade_command +{ + my ($old, $new, @extra) = @_; + return [ + 'pg_upgrade', '--no-sync', '--initdb', + '--old-datadir' => $old->data_dir, + '--new-datadir' => $new->data_dir, + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $new->host, + '--old-port' => $old->port, + '--new-port' => $new->port, + @extra, + ]; +} + +# Configure the connection settings normally written by init(), without +# changing initdb's pg_hba.conf. +sub start_new_cluster +{ + my ($node) = @_; + my $listen = $PostgreSQL::Test::Cluster::use_tcp ? $node->host : ''; + my $socketdir = $PostgreSQL::Test::Cluster::use_tcp ? '' : $node->host; + $node->append_conf('postgresql.conf', + 'port = ' + . $node->port + . "\nlisten_addresses = '$listen'\nunix_socket_directories = '$socketdir'" + ); + $node->start; +} + ok(!-d $newnode->data_dir, 'new cluster data directory does not exist before --initdb'); @@ -67,21 +92,21 @@ # pg_upgrade_output.d logs. # Pass the server-only option -F through -O to verify that it reaches # the new server without being passed to initdb. +# Repeated --initdb-options must preserve argument order and quoted values in +# the configuration used after the upgrade. chdir ${PostgreSQL::Test::Utils::tmp_check}; command_ok( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $newnode->data_dir, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - '--initdb', + upgrade_command( + $oldnode, $newnode, '--new-options' => '-F', - ], + '--initdb-options' => + '-c huge_pages=try -c custom.initdb_first=first', + '--initdb-options' => + q{-c huge_pages=off --set=custom.initdb_first=last -c "custom.initdb_text=space, apostrophe's $libdir" -c custom.initdb_path=C:\new\path -c custom.initdb_empty=""}, + '--initdb-options' => + q{-c 'custom.initdb_single=single quoted $libdir' -c "custom.initdb_escaped=quote\" slash\\\\" -c custom.initdb_join='joined 'pieces -c custom.initdb_space=escaped\ space}, + ), 'run of pg_upgrade --initdb with -O creates and upgrades the new cluster' ); @@ -89,8 +114,6 @@ like(slurp_file($newnode->data_dir . '/postmaster.opts'), qr/(?:^|\s)"-F"(?:\s|$)/, '-O option reached the target postmaster'); -# pg_upgrade --initdb should have initialized the new data directory. -# Check that PG_VERSION was created there. ok(-f $newnode->data_dir . '/PG_VERSION', 'new cluster data directory created by --initdb'); @@ -113,87 +136,47 @@ ok(!-d $newnode->data_dir . '.initdb_log', '--initdb does not leave a sibling log directory'); -# Configure the upgraded node to use its assigned port and host. -# Follow PostgreSQL::Test::Cluster's TCP or Unix-domain socket settings. -my $host = $newnode->host; -$newnode->append_conf('postgresql.conf', "port = " . $newnode->port); -if ($PostgreSQL::Test::Cluster::use_tcp) -{ - $newnode->append_conf('postgresql.conf', "unix_socket_directories = ''"); - $newnode->append_conf('postgresql.conf', "listen_addresses = '$host'"); -} -else +start_new_cluster($newnode); + +my %expected_settings = ( + huge_pages => 'off', + 'custom.initdb_first' => 'last', + 'custom.initdb_text' => q{space, apostrophe's $libdir}, + 'custom.initdb_path' => q{C:\new\path}, + 'custom.initdb_empty' => '', + 'custom.initdb_single' => q{single quoted $libdir}, + 'custom.initdb_escaped' => "quote\" slash\\", + 'custom.initdb_join' => 'joined pieces', + 'custom.initdb_space' => 'escaped space',); +for my $setting (sort keys %expected_settings) { - $newnode->append_conf('postgresql.conf', - "unix_socket_directories = '$host'"); - $newnode->append_conf('postgresql.conf', "listen_addresses = ''"); + is( $newnode->safe_psql('postgres', "SHOW $setting"), + $expected_settings{$setting}, + "$setting survives initialization and upgrade"); } -$newnode->start; - -# Verify the user data survived the upgrade. my $rows_after = $newnode->safe_psql('postgres', 'SELECT count(*) FROM t'); is($rows_after, '100', 'user data survived --initdb upgrade'); -# Verify the extra database carried over too. my $has_extra = $newnode->safe_psql('postgres', "SELECT count(*) FROM pg_database WHERE datname = 'extra_db'"); is($has_extra, '1', 'user database carried over by --initdb upgrade'); -# Check the checksum and WAL segment size settings required by -# check_control_data(). -my $new_checksums = $newnode->safe_psql('postgres', 'SHOW data_checksums'); -is($new_checksums, $old_checksums, - "data_checksums propagated by --initdb ($new_checksums)"); - -my $new_wal_segsize = - $newnode->safe_psql('postgres', 'SHOW wal_segment_size'); -is($new_wal_segsize, $old_wal_segsize, - "wal_segment_size propagated by --initdb ($new_wal_segsize)"); - -# Check template0's encoding and locale settings. -my $new_encoding = $newnode->safe_psql('postgres', - "SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = 'template0'" -); -is($new_encoding, $old_encoding, - "template0 encoding propagated by --initdb ($new_encoding)"); - -my $new_collate = $newnode->safe_psql('postgres', - "SELECT datcollate FROM pg_database WHERE datname = 'template0'"); -is($new_collate, $old_collate, - "template0 collation propagated by --initdb ($new_collate)"); - -my $new_ctype = $newnode->safe_psql('postgres', - "SELECT datctype FROM pg_database WHERE datname = 'template0'"); -is($new_ctype, $old_ctype, - "template0 ctype propagated by --initdb ($new_ctype)"); - -my $new_provider = $newnode->safe_psql('postgres', - "SELECT datlocprovider FROM pg_database WHERE datname = 'template0'"); -is($new_provider, $old_provider, - "template0 locale provider propagated by --initdb ($new_provider)"); +# The new cluster must match the old checksum, WAL segment, encoding, and +# locale settings. +for my $setting (sort keys %setting_queries) +{ + is($newnode->safe_psql('postgres', $setting_queries{$setting}), + $old_settings{$setting}, "$setting propagated by --initdb"); +} $newnode->stop; -# Check that pg_upgrade --initdb rejects an existing target cluster. -# Match its PG_VERSION diagnostic on stdout to distinguish this failure -# from initdb's nonempty-directory error. -command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $newnode->data_dir, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - '--initdb', - ], - 1, - [qr/already contains a database system/], - [qr/^$/], - '--initdb refuses to overwrite an existing cluster (PG_VERSION check)'); +# Check that pg_upgrade rejects an existing cluster before invoking initdb. +# The error is reported on stdout. +command_checks_all(upgrade_command($oldnode, $newnode), + 1, [qr/is not empty/], [qr/^$/], + '--initdb refuses to overwrite an existing cluster'); # Reject overlap with the output directory before creating logs or starting # either server, for both a real upgrade and --check. @@ -206,15 +189,10 @@ chdir $overlap_cwd or die "could not change to $overlap_cwd: $!"; my $mode = $check ? '--check --initdb' : '--initdb'; command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, + upgrade_command( + $oldnode, $newnode, '--new-datadir' => $target, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--initdb', - $check ? '--check' : (), - ], + $check ? '--check' : ()), 1, [qr/overlaps output directory/], [qr/^$/], @@ -225,52 +203,35 @@ } } -# --initdb must fail early with a clear message if initdb is not present in the -# new cluster's bin directory. Point --new-bindir at an empty directory and use -# a fresh (nonexistent) new data directory so we reach the initdb-present check. +# Validate the new binaries before initialization. my $empty_bindir = PostgreSQL::Test::Utils::tempdir; command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, + upgrade_command( + $oldnode, $newnode, '--new-datadir' => $newnode->data_dir . '_nonexistent', - '--old-bindir' => $oldbindir, - '--new-bindir' => $empty_bindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - '--initdb', - ], + '--new-bindir' => $empty_bindir), 1, - [qr/could not find "initdb"/], + [qr/check for .*postgres.* failed/], [qr/^$/], - '--initdb fails early when initdb is missing from the new bindir'); + '--initdb fails early when the new binaries are missing'); -# --check --initdb performs all source-side validation without creating a -# target cluster. +# --check --initdb initializes the new cluster and runs compatibility checks +# on both clusters. +my $checked_target = $newnode->data_dir . '_check'; command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $newnode->data_dir . '_dry_run', - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - '--initdb', - '--check', - ], + upgrade_command( + $oldnode, $newnode, + '--new-datadir' => $checked_target, + '--check'), 0, - [qr/Source cluster compatibility checks passed/], + [qr/Clusters are compatible/], [qr/^$/], - '--check --initdb runs source-side checks without creating the cluster'); + '--check --initdb initializes the target and checks both clusters'); -# Verify that --check --initdb didn't create anything. -ok(!-d $newnode->data_dir . '_dry_run', - '--check --initdb does not create the new cluster directory'); -ok( !-d $newnode->data_dir . '_dry_run.initdb_log', - '--check --initdb does not leave a sibling log directory'); +ok(-f "$checked_target/PG_VERSION", + 'successful check retains the new cluster'); +ok( !-f "$checked_target/postmaster.pid", + 'successful check stops the new server'); # Without -B, --initdb must derive the new bindir from the original argv[0], # even when that directory is not in PATH. @@ -293,14 +254,13 @@ '--check', ], 0, - [qr/Source cluster compatibility checks passed/], + [qr/Clusters are compatible/], [qr/^$/], '--initdb derives the new bindir from an absolute argv[0]'); } -# A live source is valid for --check --initdb, just as it is for ordinary -# --check. No target server is started, so the ports can be equal. -# pg_upgrade must neither stop nor modify the source server. +# --check --initdb must accept a running old server and leave it running. +# Use a different port for the new server. SKIP: { skip "Timing issues with live server detection on Windows", 4 @@ -308,142 +268,254 @@ $oldnode->start; command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, + upgrade_command( + $oldnode, $newnode, '--new-datadir' => $newnode->data_dir . '_live_check', - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $oldnode->port, - '--initdb', - '--check', - ], + '--check'), 0, - [qr/Source cluster compatibility checks passed/], + [qr/Clusters are compatible/], [qr/^$/], - '--check --initdb accepts a live source with equal old and new ports' - ); + '--check --initdb accepts a live source'); is($oldnode->safe_psql('postgres', 'SELECT 1'), '1', 'live source remains running after --check --initdb'); $oldnode->stop; } -# Force a failure while the target postmaster is running. This verifies that -# the exit handlers stop the postmaster before cleaning its data directory. -my $test_library = $ENV{TEST_EXT_LIB} - or die "could not get the test extension library path"; -my $missing_library = - PostgreSQL::Test::Utils::tempdir() . '/' . basename($test_library); -copy($test_library, $missing_library) - or die "could not copy $test_library to $missing_library: $!"; -my $sql_library = $missing_library =~ s/'/''/gr; - +# Both upgrade and --check must reject regproc columns in user tables and +# retain the initialized new cluster. $oldnode->start; -$oldnode->safe_psql('postgres', - "CREATE FUNCTION missing_upgrade_library() RETURNS void " - . "AS '$sql_library', 'test_ext' LANGUAGE C"); +$oldnode->safe_psql('postgres', 'CREATE TABLE bad (c regproc)'); $oldnode->stop; -unlink($missing_library) or die "could not remove $missing_library: $!"; -# On failure after initdb, preserve an empty directory supplied by the -# operator, including its original mode, but remove everything initdb added. -my $existing_target = $newnode->data_dir . '_existing_empty'; -mkdir($existing_target, 0711) or die "could not create $existing_target: $!"; -chmod(0711, $existing_target) - or die "could not set mode on $existing_target: $!"; -my $existing_target_mode = (stat($existing_target))[2] & 0777; +for my $check (0, 1) +{ + my $target = $newnode->data_dir . "_incompatible_$check"; + my $mode = $check ? '--check --initdb' : '--initdb'; + command_checks_all( + upgrade_command( + $oldnode, $newnode, + '--new-datadir' => $target, + $check ? '--check' : ()), + 1, + [qr/failed check: Checking for reg\* data types in user tables/], + [qr/^$/], + "$mode rejects an incompatible old cluster"); + ok(-f "$target/PG_VERSION", "failed $mode retains the new cluster"); +} +# Require --initdb even when --initdb-options is empty. +# Reject invalid quoting and line breaks before examining either cluster. command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $existing_target, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - '--initdb', - ], + [ 'pg_upgrade', '--initdb-options=' ], 1, - [qr/references loadable libraries that are missing/], + [qr/--initdb-options requires --initdb/], [qr/^$/], - 'failed --initdb stops the target and preserves an existing directory'); + '--initdb-options requires --initdb even for an empty argument'); -ok(-d $existing_target, 'operator-created target directory still exists'); -opendir(my $target_dir, $existing_target) - or die "could not open $existing_target: $!"; -my @target_entries = grep { $_ ne '.' && $_ ne '..' } readdir($target_dir); -closedir($target_dir); -is_deeply(\@target_entries, [], - 'operator-created target directory is empty after cleanup'); - -SKIP: +for my $case ( + [ + q{-c 'huge_pages=off}, qr/unterminated quote/, + 'unclosed single quote' + ], + [ + q{-c "huge_pages=off}, qr/unterminated quote/, + 'unclosed double quote' + ], + [ "-c huge_pages=off\\", qr/trailing backslash/, 'trailing escape' ], + [ "-c huge_pages=off\n", qr/newline or carriage return/, 'newline' ]) { - skip "unix-style permissions not supported on Windows", 1 - if ($windows_os || $Config::Config{osname} eq 'cygwin'); - - my $mode_after_failure = (stat($existing_target))[2] & 0777; - is($mode_after_failure, $existing_target_mode, - 'operator-created target directory mode is restored'); + command_checks_all( + [ 'pg_upgrade', '--initdb', "--initdb-options=$case->[0]" ], + 1, [ $case->[1] ], + [qr/^$/], "reject $case->[2]"); } -# Starting another server on the target port confirms that the failed -# pg_upgrade did not leave its temporary target postmaster running. -$newnode->start; -is($newnode->safe_psql('postgres', 'SELECT 1'), - '1', 'target port is free after failed --initdb cleanup'); -$newnode->stop; +# Reject options that override pg_upgrade's settings or skip initialization. +for my $options ( + '-D/another/datadir', + '--username=another_user', + '--wal-segsize=32', + '--no-data-checksums', + '--encoding=UTF8', + '--locale=C', + '--locale-provider=builtin', + '--sync-only', + '--help', + '-c data_directory=/another/datadir', + '--set=CONFIG_FILE=/another/config', + '-c Hba-File=/another/hba', + '-cident_file=/another/ident') +{ + command_checks_all( + [ 'pg_upgrade', '--initdb', "--initdb-options=$options" ], 1, + [qr/cannot be used with --initdb/], [qr/^$/], + "reject managed or incompatible option: $options"); +} -# Verify that the dry run executes compatibility checks rather than merely -# printing an initdb command. -$oldnode->start; -$oldnode->safe_psql('postgres', - 'DROP FUNCTION missing_upgrade_library(); CREATE TABLE bad (c regproc)'); -$oldnode->stop; +sub option_quote +{ + my ($value) = @_; + $value =~ s/'/'\\''/g; + return "'$value'"; +} -my $incompatible_target = $newnode->data_dir . '_incompatible_dry_run'; +# test_slru rejects LOAD unless preloaded. The new cluster must preload it +# for pg_upgrade's loadable-library check to pass. +my $preload_old = PostgreSQL::Test::Cluster->new('preload_old'); +$preload_old->init; +$preload_old->append_conf('postgresql.conf', + "shared_preload_libraries = 'test_slru'"); +$preload_old->start; +$preload_old->safe_psql('postgres', 'CREATE EXTENSION test_slru'); +my $quoted_superuser = + $preload_old->safe_psql('postgres', 'SELECT quote_ident(current_user)'); +# This password is used only by the temporary test clusters. +my $password = 'initdb-options-test-password'; +$preload_old->safe_psql('postgres', + "ALTER ROLE $quoted_superuser PASSWORD '$password'"); +$preload_old->stop; + +my $preload_missing = PostgreSQL::Test::Cluster->new('preload_missing'); +my $preload_missing_wal = $preload_missing->basedir . '/wal'; command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $incompatible_target, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - '--initdb', + upgrade_command( + $preload_old, + $preload_missing, '--check', - ], + '--initdb-options' => '--waldir=' . option_quote($preload_missing_wal) + ), 1, - [qr/failed check: Checking for reg\* data types in user tables/], + [qr/references loadable libraries that are missing/], [qr/^$/], - '--check --initdb rejects an incompatible source cluster'); -ok(!-d $incompatible_target, - 'failed --check --initdb does not create the target directory'); + '--check --initdb checks required libraries in the new cluster'); +ok( -f $preload_missing->data_dir . '/PG_VERSION', + 'failed preload check retains the new cluster'); +ok(-d $preload_missing_wal, 'failed preload check retains the WAL directory'); +ok( !-f $preload_missing->data_dir . '/postmaster.pid', + 'failed preload check stops the new server'); + +my $credential_dir = PostgreSQL::Test::Utils::tempdir; +my $pwfile = "$credential_dir/initdb.pw"; +append_to_file($pwfile, "$password\n"); +chmod(0600, $pwfile) or die "could not protect $pwfile: $!"; +my $pgpass = "$credential_dir/pgpass"; +append_to_file($pgpass, "*:*:*:*:$password\n"); +chmod(0600, $pgpass) or die "could not protect $pgpass: $!"; + +my $auth_options = + q{-c shared_preload_libraries=$libdir/test_slru --auth-local=scram-sha-256 --auth-host=scram-sha-256 --pwfile="} + . $pwfile . '"'; -# Conversely, pg_upgrade owns a target directory that did not exist before -# initdb and removes it completely after a later failure. -my $created_target = $newnode->data_dir . '_created_then_failed'; -command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $created_target, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - '--initdb', - ], - 1, - [qr/failed check: Checking for reg\* data types in user tables/], - [qr/^$/], - 'failed --initdb removes a target it created'); -ok(!-d $created_target, 'pg_upgrade-created target directory was removed'); +{ + local $ENV{PGPASSFILE} = "$credential_dir/no-password-file"; + delete local $ENV{PGPASSWORD}; + my $auth_missing = PostgreSQL::Test::Cluster->new('auth_missing'); + command_checks_all( + upgrade_command( + $preload_old, $auth_missing, + '--initdb-options' => $auth_options), + 1, + [qr/fe_sendauth: no password supplied/], + [qr/^$/], + 'password authentication is not weakened when credentials are missing' + ); + ok( -f $auth_missing->data_dir . '/PG_VERSION', + 'authentication failure retains the new cluster'); + ok( !-f $auth_missing->data_dir . '/postmaster.pid', + 'authentication failure stops the new server'); +} + +{ + local $ENV{PGPASSFILE} = $pgpass; + my $preload_new = PostgreSQL::Test::Cluster->new('preload_new'); + my $wal = $preload_new->basedir . '/wal with spaces'; + command_ok( + upgrade_command( + $preload_old, + $preload_new, + '--initdb-options' => $auth_options + . ' --waldir=' + . option_quote($wal)), + 'initdb options support a preload-only extension and password authentication' + ); + ok(-d $wal, 'initdb uses the requested WAL directory'); + + my $hba = slurp_file($preload_new->data_dir . '/pg_hba.conf'); + my @rules = grep { /\S/ && !/^\s*#/ } split(/\n/, $hba); + ok(!(grep { !/\bscram-sha-256\s*$/ } @rules), + 'all generated authentication rules keep the requested method'); + SKIP: + { + skip 'unix-style permissions not supported on Windows', 1 + if ($windows_os || $Config::Config{osname} eq 'cygwin'); + is((stat($preload_new->data_dir))[2] & 0777, + 0700, 'owner-only data directory mode is inherited'); + } + + start_new_cluster($preload_new); + is( $preload_new->safe_psql('postgres', 'SHOW shared_preload_libraries'), + q{$libdir/test_slru}, + 'preload setting preserves literal $libdir'); + $preload_new->safe_psql('postgres', + "SELECT test_slru_page_write(0, 'upgraded')"); + is( $preload_new->safe_psql('postgres', 'SELECT test_slru_page_read(0)'), + 'upgraded', + 'restored preload-only extension is usable'); + $preload_new->stop; +} + +# Link and swap retain old file modes, so a group-access request must not +# turn an owner-only cluster into a partly group-readable cluster. +SKIP: +{ + skip 'Unix permissions are not supported on this platform', 1 + if $windows_os || $Config::Config{osname} eq 'cygwin'; + + subtest 'group access with link and swap' => sub { + for my $mode ('--link', '--swap') + { + for my $check (0, 1) + { + my $new = PostgreSQL::Test::Cluster->new( + substr($mode, 2) . "_group_rejected_$check"); + command_checks_all( + upgrade_command( + $preload_old, $new, $mode, + '--initdb-options' => $check + ? '-dg' + : '--allow-group-access', + $check ? '--check' : ()), + 1, + [qr/cannot enable group access with \Q$mode\E/], + [qr/^$/], + "$mode rejects group access with check=$check"); + ok(!-d $new->data_dir, + 'permission mismatch leaves no new cluster'); + } + + my $name = substr($mode, 2); + my $old = PostgreSQL::Test::Cluster->new("old_$name"); + my $new = PostgreSQL::Test::Cluster->new("new_$name"); + $old->init(extra => ['--allow-group-access']); + $old->start; + $old->safe_psql('postgres', + 'CREATE TABLE permissions (id integer); INSERT INTO permissions VALUES (1)' + ); + my $relpath = $old->safe_psql('postgres', + "SELECT pg_relation_filepath('permissions')"); + $old->stop; + command_ok( + upgrade_command( + $old, $new, + $mode, '--initdb-options' => '--allow-group-access'), + "$mode accepts matching group access settings"); + is((stat($new->data_dir))[2] & 0777, + 0750, "$mode preserves data directory permissions"); + is((stat($new->data_dir . "/$relpath"))[2] & 0777, + 0640, "$mode preserves relation permissions"); + } + done_testing(); + }; +} done_testing(); From 170df44c1253dd4a601dacbfc5bfa924e316b8a5 Mon Sep 17 00:00:00 2001 From: Bohyun Lee Date: Tue, 13 Oct 2026 16:50:15 +0200 Subject: [PATCH] pg_upgrade: simplify --initdb option handling and retries Let initdb validate forwarded options, and leave group access opt-in as in the manual workflow. Reuse an initialized target for repeated --check --initdb runs, and sync retained clusters when requested. Use temporary logs during initialization, then resume logging in the new data directory. Keep early logs on failure or with --retain. Update the documentation and existing TAP expectations. --- doc/src/sgml/ref/pgupgrade.sgml | 83 +++++-------- src/bin/pg_upgrade/check.c | 7 ++ src/bin/pg_upgrade/option.c | 145 +--------------------- src/bin/pg_upgrade/pg_upgrade.c | 97 ++++++++------- src/bin/pg_upgrade/pg_upgrade.h | 5 +- src/bin/pg_upgrade/t/009_initdb_option.pl | 57 ++------- src/bin/pg_upgrade/util.c | 3 +- 7 files changed, 107 insertions(+), 290 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 5bc543717de54..bd239af7eb83c 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -107,8 +107,7 @@ PostgreSQL documentation check clusters without performing the upgrade. - If is also specified, create the new cluster - before checking it. + If is also specified, initialize the new cluster if needed before checking it. @@ -274,29 +273,18 @@ PostgreSQL documentation The new cluster uses the old cluster's WAL segment size and data-checksum setting, and the encoding and locale of its template0 database. - On Unix systems, if the old cluster allows group read/execute access - to its data directory, is passed - to initdb for the new cluster. - The new cluster data directory specified with - / must be empty or - not yet exist; otherwise pg_upgrade exits - with an error. - It must not overlap the pg_upgrade_output.d - directory in the current working directory: neither directory may - be equal to or contain the other. - The new data directory may be inside the old data directory; in that - case, the script to delete the old cluster is not generated. + To initialize the new cluster, the directory specified by / must be empty or must not exist. + During initialization, pg_upgrade uses pg_upgrade_output.d in the current working directory for temporary logs. + The new data directory must not be the same as that log directory, be inside it, or contain it. + If the new data directory is inside the old data directory, pg_upgrade does not generate the script for deleting the old cluster, because the script would also delete the new cluster. - When combined with /, - creates the new cluster and runs the normal - compatibility checks on both clusters without performing the upgrade. - The initialized new cluster is retained whether the checks succeed or - fail. After a successful check, run pg_upgrade - without , , or - to upgrade using that cluster. + When combined with /, initializes the new cluster if the target directory is empty or nonexistent, then runs the compatibility checks on both clusters without performing the upgrade. + If the target directory is nonempty, initialization is skipped and the existing cluster is checked instead. + The new cluster is retained whether the checks succeed or fail, so the same check command can be run again. + After a successful check, run pg_upgrade without , , or to upgrade using that cluster. / continues to pass @@ -305,13 +293,10 @@ PostgreSQL documentation for additional initialization options. - If initialization fails, initdb performs its normal - cleanup. Once initialization succeeds, pg_upgrade - leaves the new data directory and any separate WAL directory in place - if a later step fails, just as it does without . + If initialization fails, initdb performs its normal cleanup. + Once initialization succeeds, pg_upgrade leaves the new data directory and any separate WAL directory in place if a later step fails, just as it does without . See for recovery instructions. - To retry with , the new data and WAL directories - must again be empty or nonexistent. + To rerun initialization with , the new data and WAL directories must again be empty or nonexistent. @@ -332,6 +317,11 @@ pg_upgrade --initdb --initdb-options='-c huge_pages=off --waldir=/wal/new' ... shared_preload_libraries must support being loaded during initialization, as well as by the new server during upgrade. + + These options apply only when pg_upgrade initializes the new cluster. + When you retry with an already initialized cluster, pg_upgrade prints a notice and skips initdb. + Changes to on the retry are not applied to that cluster. + Within options, spaces and tabs separate arguments. Single or double quotes group text into one argument and @@ -347,23 +337,17 @@ pg_upgrade --initdb --initdb-options='-c huge_pages=off --waldir=/wal/new' ... file-based inputs such as for passwords. - Options that replace the data directory, bootstrap superuser, - checksum setting, WAL segment size, or inherited encoding and locale - settings are not allowed. Configuration settings that redirect the data or - configuration files (data_directory, - config_file, hba_file, and - ident_file) are also rejected, as are options - that do not initialize a cluster, such as . - Run initdb separately for setups requiring these - options. + These arguments are passed to initdb after the + arguments generated by pg_upgrade and are + validated by initdb. + pg_upgrade then performs its normal + cluster compatibility checks. - An explicit enables group access - even if the old cluster does not allow it; otherwise the old cluster's - access mode is used. With or , group - access cannot be enabled unless the old cluster already allows it, - because these transfer modes preserve the old file and directory - permissions. + Group access to the new cluster is disabled by default. + To enable it, use . + With or , transferred relation files retain their existing permissions, even when group access is requested for the new cluster. + With , moved database directories also retain their existing permissions. Authentication options, including and @@ -373,12 +357,6 @@ pg_upgrade --initdb --initdb-options='-c huge_pages=off --waldir=/wal/new' ... pg_upgrade does not weaken authentication rules to obtain access. - - pg_upgrade checks argument quoting and - conflicts with its managed settings. Other options, their values, - and paths such as are validated by - initdb, including with . - @@ -1196,10 +1174,11 @@ psql --username=postgres --file=script.sql postgres Notes - pg_upgrade creates various working files, such - as schema dumps, stored within pg_upgrade_output.d in - the directory of the new cluster, or in the current working directory when - is used. + pg_upgrade creates various working files, such as schema dumps, stored within pg_upgrade_output.d in the directory of the new cluster. + When initializes the new cluster, early server and initialization logs are written under pg_upgrade_output.d in the current working directory, because initdb requires an empty target directory. + After initialization succeeds, these temporary logs are removed unless is specified. + Subsequent output is written to the new cluster's pg_upgrade_output.d directory. + If an early step or initdb fails, the logs remain in the current working directory. Each run creates a new subdirectory named with a timestamp formatted as per ISO 8601 (%Y%m%dT%H%M%S), where all its generated files are diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 150c4fc34b9b2..3265569ae39ca 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -758,6 +758,13 @@ report_clusters_compatible(void) /* stops new cluster */ stop_postmaster(false); + /* + * --initdb defers syncing until pg_upgrade finishes. Sync the + * retained cluster before returning from --check. + */ + if (user_opts.initdb_new_cluster) + sync_new_cluster(true); + cleanup_output_dirs(); exit(0); } diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index d24ac95caea2c..0497677cfde92 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -23,8 +23,6 @@ static void usage(void); static void parse_initdb_options(const char *options); -static void validate_initdb_options(void); -static void check_initdb_setting(const char *setting); static void check_required_directory(char **dirpath, const char *envVarName, bool useCwd, const char *cmdLineOption, const char *description, @@ -260,12 +258,8 @@ parseCommandLine(int argc, char *argv[]) if (optind < argc) pg_fatal("too many command-line arguments (first is \"%s\")", argv[optind]); - if (user_opts.initdb_options_given) - { - if (!user_opts.initdb_new_cluster) - pg_fatal("--initdb-options requires --initdb"); - validate_initdb_options(); - } + if (user_opts.initdb_options_given && !user_opts.initdb_new_cluster) + pg_fatal("--initdb-options requires --initdb"); if (!user_opts.sync_method) user_opts.sync_method = pg_strdup("fsync"); @@ -390,141 +384,6 @@ parse_initdb_options(const char *options) } -/* - * Reject settings that redirect the new cluster's data or configuration - * files, which pg_upgrade expects to find in the new data directory. - */ -static void -check_initdb_setting(const char *setting) -{ - char *name = pg_strdup(setting); - char *equals = strchr(name, '='); - - if (equals) - *equals = '\0'; - - /* Treat hyphens as underscores, as ParseLongOption() does. */ - for (char *p = name; *p; p++) - if (*p == '-') - *p = '_'; - - if (pg_strcasecmp(name, "data_directory") == 0 || - pg_strcasecmp(name, "config_file") == 0 || - pg_strcasecmp(name, "hba_file") == 0 || - pg_strcasecmp(name, "ident_file") == 0) - pg_fatal("initdb setting \"%s\" cannot be used with --initdb", name); - - pg_free(name); -} - - -/* - * Check for options that conflict with pg_upgrade, leaving other validation - * to initdb. Parse after the pg_upgrade options, since getopt_long() uses - * global state. Keep the option table consistent with initdb so getopt_long() - * knows which options take arguments. - */ -static void -validate_initdb_options(void) -{ - static struct option long_options[] = { - {"pgdata", required_argument, NULL, 'D'}, - {"encoding", required_argument, NULL, 'E'}, - {"locale", required_argument, NULL, 1}, - {"lc-collate", required_argument, NULL, 2}, - {"lc-ctype", required_argument, NULL, 3}, - {"lc-monetary", required_argument, NULL, 4}, - {"lc-numeric", required_argument, NULL, 5}, - {"lc-time", required_argument, NULL, 6}, - {"lc-messages", required_argument, NULL, 7}, - {"no-locale", no_argument, NULL, 8}, - {"text-search-config", required_argument, NULL, 'T'}, - {"auth", required_argument, NULL, 'A'}, - {"auth-local", required_argument, NULL, 10}, - {"auth-host", required_argument, NULL, 11}, - {"pwprompt", no_argument, NULL, 'W'}, - {"pwfile", required_argument, NULL, 9}, - {"username", required_argument, NULL, 'U'}, - {"help", no_argument, NULL, 22}, - {"version", no_argument, NULL, 'V'}, - {"debug", no_argument, NULL, 'd'}, - {"show", no_argument, NULL, 's'}, - {"noclean", no_argument, NULL, 'n'}, - {"no-clean", no_argument, NULL, 'n'}, - {"nosync", no_argument, NULL, 'N'}, - {"no-sync", no_argument, NULL, 'N'}, - {"no-instructions", no_argument, NULL, 13}, - {"set", required_argument, NULL, 'c'}, - {"sync-only", no_argument, NULL, 'S'}, - {"waldir", required_argument, NULL, 'X'}, - {"wal-segsize", required_argument, NULL, 12}, - {"data-checksums", no_argument, NULL, 'k'}, - {"allow-group-access", no_argument, NULL, 'g'}, - {"discard-caches", no_argument, NULL, 14}, - {"locale-provider", required_argument, NULL, 15}, - {"builtin-locale", required_argument, NULL, 16}, - {"icu-locale", required_argument, NULL, 17}, - {"icu-rules", required_argument, NULL, 18}, - {"sync-method", required_argument, NULL, 19}, - {"no-data-checksums", no_argument, NULL, 20}, - {"no-sync-data-files", no_argument, NULL, 21}, - {NULL, 0, NULL, 0} - }; - int argc = user_opts.num_initdb_options + 1; - char **argv = pg_malloc_array(char *, (size_t) argc + 1); - int option; - int optindex; - int save_opterr = opterr; - - argv[0] = pg_strdup(os_info.progname); - for (int i = 1; i < argc; i++) - argv[i] = user_opts.initdb_options[i - 1]; - argv[argc] = NULL; - optind = 1; - opterr = 0; -#ifdef HAVE_INT_OPTRESET - optreset = 1; -#endif - while ((option = getopt_long(argc, argv, "A:c:dD:E:gkL:nNsST:U:VWX:", - long_options, &optindex)) != -1) - { - switch (option) - { - case 1: - case 2: - case 3: - case 8: - case 12: - case 15: - case 16: - case 17: - case 18: - case 20: - case 22: - pg_fatal("initdb option \"--%s\" cannot be used with --initdb", - long_options[optindex].name); - case 'D': - case 'E': - case 'k': - case 's': - case 'S': - case 'U': - case 'V': - pg_fatal("initdb option \"-%c\" cannot be used with --initdb", option); - case 'g': - user_opts.initdb_allow_group_access = true; - break; - case 'c': - check_initdb_setting(optarg); - break; - } - } - opterr = save_opterr; - pg_free(argv[0]); - pg_free(argv); -} - - static void usage(void) { diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 5ced9a1f7af12..56e039665eefd 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -71,8 +71,8 @@ static void set_frozenxids(void); static void make_outputdirs(const char *output_root); static void setup(char *argv0); static void resolve_new_bindir(const char *argv0); -static void prepare_new_cluster_initdb(const char *argv0); -static void check_new_cluster_initdb_target(void); +static bool prepare_new_cluster_initdb(const char *argv0); +static bool check_new_cluster_initdb_target(void); static void get_old_cluster_initdb_info(void); static void build_new_cluster_initdb_cmd(PQExpBuffer cmd); static void create_new_cluster_via_initdb(void); @@ -118,36 +118,23 @@ main(int argc, char **argv) adjust_data_dir(&old_cluster); adjust_data_dir(&new_cluster); - if (user_opts.initdb_new_cluster) + if (user_opts.initdb_new_cluster && + prepare_new_cluster_initdb(argv[0])) { - prepare_new_cluster_initdb(argv[0]); - /* Match output-file permissions to the cluster being inspected. */ if (!GetDataDirectoryCreatePerm(old_cluster.pgdata)) pg_fatal("could not read permissions of directory \"%s\": %m", old_cluster.pgdata); umask(pg_mode_mask); -#if !defined(WIN32) && !defined(__CYGWIN__) - /* - * Link and swap preserve the old file permissions, so enabling group - * access would leave transferred files unreadable by the group. - * Changing permissions on hard links would also change the old - * cluster. + * Keep early logs in a temporary directory because initdb requires an + * empty target directory. */ - if (user_opts.initdb_allow_group_access && - pg_dir_create_mode != PG_DIR_MODE_GROUP && - (user_opts.transfer_mode == TRANSFER_MODE_LINK || - user_opts.transfer_mode == TRANSFER_MODE_SWAP)) - pg_fatal("cannot enable group access with %s when the old cluster does not allow group access", - user_opts.transfer_mode == TRANSFER_MODE_LINK ? "--link" : "--swap"); -#endif - - /* The new cluster has not been initialized yet. */ make_outputdirs("."); create_new_cluster_via_initdb(); + cleanup_output_dirs(); } /* @@ -164,8 +151,7 @@ main(int argc, char **argv) * This needs to happen after adjusting the data directory of the new * cluster in adjust_data_dir(). */ - if (!user_opts.initdb_new_cluster) - make_outputdirs(new_cluster.pgdata); + make_outputdirs(new_cluster.pgdata); setup(argv[0]); @@ -279,18 +265,7 @@ main(int argc, char **argv) stop_postmaster(false); } - if (user_opts.do_sync) - { - prep_status("Sync data directory to disk"); - exec_prog(UTILITY_LOG_FILE, NULL, true, true, - "\"%s/initdb\" --sync-only %s \"%s\" --sync-method %s", - new_cluster.bindir, - (user_opts.transfer_mode == TRANSFER_MODE_SWAP) ? - "--no-sync-data-files" : "", - new_cluster.pgdata, - user_opts.sync_method); - check_ok(); - } + sync_new_cluster(user_opts.transfer_mode != TRANSFER_MODE_SWAP); create_script_for_old_cluster_deletion(&deletion_script_file_name); @@ -310,6 +285,28 @@ main(int argc, char **argv) return 0; } +/* + * Sync the new cluster after shutdown, unless --no-sync is specified. + * --check --initdb needs a full sync. A completed --swap upgrade can skip + * syncing data files here because they have already been synced. + */ +void +sync_new_cluster(bool sync_data_files) +{ + if (!user_opts.do_sync) + return; + + prep_status("Sync data directory to disk"); + exec_prog(UTILITY_LOG_FILE, NULL, true, true, + "\"%s/initdb\" --sync-only %s \"%s\" --sync-method %s", + new_cluster.bindir, + sync_data_files ? "" : "--no-sync-data-files", + new_cluster.pgdata, + user_opts.sync_method); + check_ok(); +} + + /* * Create and assign proper permissions to the set of output directories * used to store any data generated internally, filling in log_opts in @@ -327,6 +324,7 @@ make_outputdirs(const char *output_root) time_t tt; int len; + pg_free(log_opts.rootdir); log_opts.rootdir = (char *) pg_malloc0(MAXPGPATH); len = snprintf(log_opts.rootdir, MAXPGPATH, "%s/%s", output_root, BASE_OUTPUTDIR); if (len >= MAXPGPATH) @@ -339,6 +337,7 @@ make_outputdirs(const char *output_root) /* append milliseconds */ snprintf(timebuf + strlen(timebuf), sizeof(timebuf) - strlen(timebuf), ".%03d", (int) (time.tv_usec / 1000)); + pg_free(log_opts.basedir); log_opts.basedir = (char *) pg_malloc0(MAXPGPATH); len = snprintf(log_opts.basedir, MAXPGPATH, "%s/%s", log_opts.rootdir, timebuf); @@ -346,6 +345,7 @@ make_outputdirs(const char *output_root) pg_fatal("directory path for new cluster is too long"); /* BASE_OUTPUTDIR/$timestamp/dump/ */ + pg_free(log_opts.dumpdir); log_opts.dumpdir = (char *) pg_malloc0(MAXPGPATH); len = snprintf(log_opts.dumpdir, MAXPGPATH, "%s/%s/%s", log_opts.rootdir, timebuf, DUMP_OUTPUTDIR); @@ -353,6 +353,7 @@ make_outputdirs(const char *output_root) pg_fatal("directory path for new cluster is too long"); /* BASE_OUTPUTDIR/$timestamp/log/ */ + pg_free(log_opts.logdir); log_opts.logdir = (char *) pg_malloc0(MAXPGPATH); len = snprintf(log_opts.logdir, MAXPGPATH, "%s/%s/%s", log_opts.rootdir, timebuf, LOG_OUTPUTDIR); @@ -425,24 +426,25 @@ resolve_new_bindir(const char *argv0) /* - * Validate the new binaries and target directory before creating output - * files or starting either cluster. + * Validate the new binaries and decide whether initdb is needed before + * creating output files or starting either cluster. */ -static void +static bool prepare_new_cluster_initdb(const char *argv0) { check_pghost_envvar(); resolve_new_bindir(argv0); check_bin_dir(&new_cluster, true); - check_new_cluster_initdb_target(); + return check_new_cluster_initdb_target(); } /* - * Require an empty or nonexistent new data directory and reject overlap - * with pg_upgrade_output.d. + * For --check, reuse a nonempty target and let the normal cluster checks + * validate it. Otherwise, require an empty or nonexistent directory that + * does not overlap pg_upgrade_output.d. */ -static void +static bool check_new_cluster_initdb_target(void) { char *absolute_pgdata; @@ -450,9 +452,19 @@ check_new_cluster_initdb_target(void) int dir_status = pg_check_dir(new_cluster.pgdata); if (dir_status > 1) + { + if (user_opts.check) + { + pg_log(PG_REPORT, + "Using existing new cluster data directory \"%s\"; " + "skipping initdb and --initdb-options.", + new_cluster.pgdata); + return false; + } pg_fatal("new cluster data directory \"%s\" is not empty; " "--initdb requires an empty or nonexistent directory", new_cluster.pgdata); + } else if (dir_status < 0) pg_fatal("could not access directory \"%s\": %m", new_cluster.pgdata); @@ -467,6 +479,7 @@ check_new_cluster_initdb_target(void) absolute_pgdata, absolute_outputdir); free(absolute_pgdata); free(absolute_outputdir); + return true; } @@ -537,8 +550,6 @@ build_new_cluster_initdb_cmd(PQExpBuffer cmd) appendShellString(cmd, new_cluster.pgdata); appendPQExpBufferStr(cmd, " -U "); appendShellString(cmd, os_info.user); - if (pg_dir_create_mode == PG_DIR_MODE_GROUP) - appendPQExpBufferStr(cmd, " --allow-group-access"); appendPQExpBuffer(cmd, " --wal-segsize=%u", old_cluster.controldata.walseg / (1024 * 1024)); diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 698873d879416..317bc401a3e19 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -329,7 +329,6 @@ typedef struct bool initdb_new_cluster; /* run initdb before compatibility checks */ bool initdb_options_given; /* --initdb-options given, even if * empty */ - bool initdb_allow_group_access; /* --allow-group-access */ char **initdb_options; /* additional arguments to initdb */ int num_initdb_options; } UserOpts; @@ -441,6 +440,10 @@ void parseCommandLine(int argc, char *argv[]); void adjust_data_dir(ClusterInfo *cluster); void get_sock_dir(ClusterInfo *cluster); +/* pg_upgrade.c */ + +void sync_new_cluster(bool sync_data_files); + /* relfilenumber.c */ void transfer_all_new_tablespaces(DbInfoArr *old_db_arr, diff --git a/src/bin/pg_upgrade/t/009_initdb_option.pl b/src/bin/pg_upgrade/t/009_initdb_option.pl index 395c33e48da2c..7c76c61f4154c 100644 --- a/src/bin/pg_upgrade/t/009_initdb_option.pl +++ b/src/bin/pg_upgrade/t/009_initdb_option.pl @@ -11,7 +11,7 @@ use PostgreSQL::Test::Utils; use Test::More; -# Use nondefault settings to verify that --initdb carries them over to the +# Use nondefault settings to verify which settings --initdb carries over to the # new cluster. my $oldnode = PostgreSQL::Test::Cluster->new('old_node'); $oldnode->init( @@ -117,17 +117,15 @@ sub start_new_cluster ok(-f $newnode->data_dir . '/PG_VERSION', 'new cluster data directory created by --initdb'); -# Group access requested for the old cluster must be carried over to the new -# cluster. Otherwise programs that read PGDATA as a group member lose access -# after the upgrade. +# Group access requires an explicit initdb option, even if the old cluster +# allows it. SKIP: { skip "unix-style permissions not supported on Windows", 1 if ($windows_os || $Config::Config{osname} eq 'cygwin'); - my $old_mode = (stat($oldnode->data_dir))[2] & 0777; my $new_mode = (stat($newnode->data_dir))[2] & 0777; - is($new_mode, $old_mode, 'new cluster preserves PGDATA group access'); + is($new_mode, 0700, 'new cluster does not inherit PGDATA group access'); } # --initdb logs use pg_upgrade_output.d with the normal cleanup and --retain @@ -330,28 +328,6 @@ sub start_new_cluster [qr/^$/], "reject $case->[2]"); } -# Reject options that override pg_upgrade's settings or skip initialization. -for my $options ( - '-D/another/datadir', - '--username=another_user', - '--wal-segsize=32', - '--no-data-checksums', - '--encoding=UTF8', - '--locale=C', - '--locale-provider=builtin', - '--sync-only', - '--help', - '-c data_directory=/another/datadir', - '--set=CONFIG_FILE=/another/config', - '-c Hba-File=/another/hba', - '-cident_file=/another/ident') -{ - command_checks_all( - [ 'pg_upgrade', '--initdb', "--initdb-options=$options" ], 1, - [qr/cannot be used with --initdb/], [qr/^$/], - "reject managed or incompatible option: $options"); -} - sub option_quote { my ($value) = @_; @@ -449,7 +425,8 @@ sub option_quote skip 'unix-style permissions not supported on Windows', 1 if ($windows_os || $Config::Config{osname} eq 'cygwin'); is((stat($preload_new->data_dir))[2] & 0777, - 0700, 'owner-only data directory mode is inherited'); + 0700, + 'new cluster uses owner-only data directory mode by default'); } start_new_cluster($preload_new); @@ -464,8 +441,7 @@ sub option_quote $preload_new->stop; } -# Link and swap retain old file modes, so a group-access request must not -# turn an owner-only cluster into a partly group-readable cluster. +# Link and swap retain old file modes. SKIP: { skip 'Unix permissions are not supported on this platform', 1 @@ -474,25 +450,6 @@ sub option_quote subtest 'group access with link and swap' => sub { for my $mode ('--link', '--swap') { - for my $check (0, 1) - { - my $new = PostgreSQL::Test::Cluster->new( - substr($mode, 2) . "_group_rejected_$check"); - command_checks_all( - upgrade_command( - $preload_old, $new, $mode, - '--initdb-options' => $check - ? '-dg' - : '--allow-group-access', - $check ? '--check' : ()), - 1, - [qr/cannot enable group access with \Q$mode\E/], - [qr/^$/], - "$mode rejects group access with check=$check"); - ok(!-d $new->data_dir, - 'permission mismatch leaves no new cluster'); - } - my $name = substr($mode, 2); my $old = PostgreSQL::Test::Cluster->new("old_$name"); my $new = PostgreSQL::Test::Cluster->new("new_$name"); diff --git a/src/bin/pg_upgrade/util.c b/src/bin/pg_upgrade/util.c index e7d7ab5644522..47705b6853cfd 100644 --- a/src/bin/pg_upgrade/util.c +++ b/src/bin/pg_upgrade/util.c @@ -57,12 +57,13 @@ end_progress_output(void) } /* - * Remove any logs generated internally. To be used once when exiting. + * Close the internal log and remove output directories unless --retain is set. */ void cleanup_output_dirs(void) { fclose(log_opts.internal); + log_opts.internal = NULL; /* Remove dump and log files? */ if (log_opts.retain) From dbdb99ded49113cc55dbecd0c1b19642d2af4f08 Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Tue, 13 Oct 2026 21:11:32 -0500 Subject: [PATCH] pg_upgrade: preserve failed-test WAL junctions during cleanup Detach the two test-owned junctions only after the test passes. Keep the stopped-node check and preserve PG_TEST_NOCLEAN, including a defined value of zero. --- src/bin/pg_upgrade/t/009_initdb_option.pl | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/bin/pg_upgrade/t/009_initdb_option.pl b/src/bin/pg_upgrade/t/009_initdb_option.pl index 7c76c61f..8438e92c 100644 --- a/src/bin/pg_upgrade/t/009_initdb_option.pl +++ b/src/bin/pg_upgrade/t/009_initdb_option.pl @@ -335,6 +335,22 @@ sub option_quote return "'$value'"; } +# Detach test-owned WAL junctions before native Perl recursively removes +# the node. rmdir removes the junction itself, leaving its target intact. +my @test_wal_junctions; + +sub detach_test_wal_junction +{ + my ($node, $wal) = @_; + return unless $windows_os; + return if defined $ENV{PG_TEST_NOCLEAN}; + return unless PostgreSQL::Test::Utils::all_tests_passing(); + BAIL_OUT 'WAL junction cleanup requires a stopped node' + if -f $node->data_dir . '/postmaster.pid'; + ok(rmdir($node->data_dir . '/pg_wal'), 'detach test-owned WAL junction'); + ok(-d $wal, 'detaching WAL junction preserves its target'); +} + # test_slru rejects LOAD unless preloaded. The new cluster must preload it # for pg_upgrade's loadable-library check to pass. my $preload_old = PostgreSQL::Test::Cluster->new('preload_old'); @@ -369,6 +385,7 @@ ok( -f $preload_missing->data_dir . '/PG_VERSION', ok(-d $preload_missing_wal, 'failed preload check retains the WAL directory'); ok( !-f $preload_missing->data_dir . '/postmaster.pid', 'failed preload check stops the new server'); +push @test_wal_junctions, [ $preload_missing, $preload_missing_wal ]; my $credential_dir = PostgreSQL::Test::Utils::tempdir; my $pwfile = "$credential_dir/initdb.pw"; @@ -439,6 +456,7 @@ my $auth_options = 'upgraded', 'restored preload-only extension is usable'); $preload_new->stop; + push @test_wal_junctions, [ $preload_new, $wal ]; } # Link and swap retain old file modes. @@ -475,4 +493,10 @@ SKIP: }; } +# Match Cluster.pm: preserve the original cluster layout on a failed test. +foreach my $junction (@test_wal_junctions) +{ + detach_test_wal_junction(@{$junction}); +} + done_testing();