public inbox for [email protected]help / color / mirror / Atom feed
Re: Should we document how column DEFAULT expressions work? 2+ messages / 2 participants [nested] [flat]
* Re: Should we document how column DEFAULT expressions work? @ 2024-07-05 21:04 David G. Johnston <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: David G. Johnston @ 2024-07-05 21:04 UTC (permalink / raw) To: Bruce Momjian <[email protected]>; +Cc: Tom Lane <[email protected]>; David Rowley <[email protected]>; Peter Eisentraut <[email protected]>; James Coleman <[email protected]>; pgsql-hackers On Fri, Jul 5, 2024 at 1:55 PM Bruce Momjian <[email protected]> wrote: > On Fri, Jul 5, 2024 at 04:50:32PM -0400, Tom Lane wrote: > > Bruce Momjian <[email protected]> writes: > > > Also interestingly, "now" without quotes requires parentheses to make > it > > > a function call: > > > > I'm not sure why you find that surprising, or why you think that > > 'now()'::timestamptz is a function call. I suspect mostly because SQL has a habit of adding functions that don't require parentheses and it isn't obvious that "now" is not one of them. select current_timestamp; current_timestamp ------------------------------- 2024-07-05 13:55:12.521334-07 (1 row) > (Well, it is a call of > > timestamptz_in, but not of the SQL function now().) Documentation > > that is equally confused won't help any. > > Well, 'now()' certainly _looks_ like a function call, though it isn't. > The fact that 'now()'::timestamptz and 'now'::timestamptz generate > volatile results via a function call was my point. > > They generate volatile results during typed value construction. That such things are implemented via functions are best left unreferenced here, reserving mention of function calls to those things users explicitly add to their query that are, and only are, function calls. Whether we change going forward or not I'd be content to simply add a warning that writing 'now()' in a default expression is invalid syntax that fails-to-fails on backward compatibility grounds. If you want the function don't quote it, if you want the literal, remove the parentheses. David J. ^ permalink raw reply [nested|flat] 2+ messages in thread
* [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically @ 2026-07-10 10:41 Bohyun Lee <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Bohyun Lee @ 2026-07-10 10:41 UTC (permalink / raw) To: [email protected] Hi, This patch adds an --initdb option to pg_upgrade that automates the initdb step currently required before running pg_upgrade. Problem ————— Before running pg_upgrade, users must manually run initdb with options that exactly match the old cluster: WAL segment size, data checksum setting, encoding, and locale. Getting these right is error-prone. A mismatch results in a confusing check_control_data() failure after the user has already invested time in setting up the new cluster. A related question was raised before [1], where Jeff Davis discussed whether pg_upgrade should perform initdb itself rather than requiring a pre-initialized cluster. Solution ———— With --initdb, pg_upgrade handles this automatically. It derives the WAL segment size and checksum setting from pg_control and invokes initdb with the correct flags. The option refuses to proceed if the new cluster already exists. Testing ———— All existing pg_upgrade TAP tests pass. A new test, t/007_initdb_option.pl, verifies the happy path end-to-end and checks that --initdb refuses to overwrite an existing cluster. Branch: https://github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb <https://github.com/LeeBohyun/postgres/treepg_upgrade_initdb; Patch attached. [1] https://www.postgresql.org/message-id/2a7feb71dbdcea31478b3974b8075982ac2326d2.camel%40j-davis.com Regards, Bohyun Lee Attachments: [application/octet-stream] v1-pg_upgrade-initdb.patch (18.2K, ../../CAMPh8Mr8FbMnPct2Zom88UkpkddT48mZoY+m4rRxG9PdW6Fo+A@mail.gmail.com/3-v1-pg_upgrade-initdb.patch) download | inline diff: From 5710d523900c4091ab760e142d999411435cb164 Mon Sep 17 00:00:00 2001 From: Bohyun Lee <[email protected]> Date: Fri, 10 Jul 2026 09:12:41 +0000 Subject: [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically Historically, pg_upgrade requires the user to manually run initdb before invoking pg_upgrade, passing options that exactly match the old cluster's WAL segment size, data checksum setting, encoding, and locale. Getting these right is error-prone: a mismatch causes pg_upgrade to fail with an opaque check_control_data() error after the user has already gone through the trouble of running initdb. This patch adds a --initdb option that automates the initdb step. When given, pg_upgrade starts the old server briefly, reads template0's locale and encoding, derives the WAL segment size and checksum setting from the old cluster's pg_control, and runs initdb with matching options. The new cluster data directory must not already exist; pg_upgrade exits with an error if it does, to avoid clobbering an existing installation. The locale inspection requires a brief start of the old postmaster, which is already done later in the normal pg_upgrade flow; here it is done earlier, before the new cluster exists, using a temporary log directory. Extra initdb options can be passed via the existing -O flag and will be forwarded to the initdb invocation. --- doc/src/sgml/ref/pgupgrade.sgml | 25 +++ src/bin/pg_upgrade/info.c | 3 +- src/bin/pg_upgrade/option.c | 11 +- src/bin/pg_upgrade/pg_upgrade.c | 178 ++++++++++++++++++++-- src/bin/pg_upgrade/pg_upgrade.h | 4 + src/bin/pg_upgrade/t/007_initdb_option.pl | 105 +++++++++++++ 6 files changed, 312 insertions(+), 14 deletions(-) create mode 100644 src/bin/pg_upgrade/t/007_initdb_option.pl diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index e4e8c02e6d6..902b0233805 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -262,6 +262,25 @@ PostgreSQL documentation </listitem> </varlistentry> + <varlistentry> + <term><option>--initdb</option></term> + <listitem> + <para> + Create the new cluster automatically by running + <command>initdb</command> before upgrading, instead of requiring the + user to have created it manually. The WAL segment size, data checksum + setting, encoding, and locale are derived from the old cluster so that + <application>pg_upgrade</application> can verify compatibility. + </para> + <para> + The new cluster data directory specified with + <option>-D</option>/<option>--new-datadir</option> must not already + exist when this option is given; if it does, + <application>pg_upgrade</application> will exit with an error. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><option>--no-statistics</option></term> <listitem> @@ -462,6 +481,12 @@ make prefix=/usr/local/pgsql.new install prebuilt installers do this step automatically. There is no need to start the new cluster. </para> + <para> + Alternatively, pass <option>--initdb</option> to + <application>pg_upgrade</application> to have it run + <command>initdb</command> automatically, deriving the required settings + from the old cluster. In that case this manual step can be skipped. + </para> </step> <step> diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index 37fff93892f..65ae97cdc1f 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 f01d2f92d95..daaf48d47bc 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); @@ -328,6 +333,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 +343,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 7366fd4627c..b108d3beb0d 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -45,10 +45,13 @@ #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 +70,8 @@ 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 create_new_cluster_via_initdb(void); static void create_logical_replication_slots(void); static void create_conflict_detection_slot(void); @@ -107,6 +112,10 @@ main(int argc, char **argv) get_restricted_token(); adjust_data_dir(&old_cluster); + + if (user_opts.initdb_new_cluster) + create_new_cluster_via_initdb(); + adjust_data_dir(&new_cluster); /* @@ -358,6 +367,163 @@ make_outputdirs(char *pgdata) } +/* + * resolve_new_bindir() + * + * Idempotent helper: if new_cluster.bindir has not been set by the user via + * -B, derive it from the path of the currently executing pg_upgrade binary. + * Called early by create_new_cluster_via_initdb() so that the initdb path + * is available before verify_directories() runs. + */ +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); + } +} + + +/* + * create_new_cluster_via_initdb() + * + * Implements --initdb: run initdb to create the new cluster before upgrading, + * deriving WAL segment size, data checksums, encoding, and locale settings + * from the old cluster so that check_control_data() passes. + * + * This runs before the normal verify_directories() / setup() path, so we + * use a temporary log directory under the new bindir for the early server + * start; make_outputdirs() will replace log_opts.logdir later. + */ +static void +create_new_cluster_via_initdb(void) +{ + DbLocaleInfo *locale; + PQExpBufferData cmd; + char tmp_logdir[MAXPGPATH]; + char *saved_logdir = log_opts.logdir; + const char *encoding_name; + + resolve_new_bindir(os_info.progname); + + /* + * Verify that initdb is present and executable before doing any work. + * The normal path checks this later inside verify_directories(), but we + * run before that, so fail early with a useful message. + */ + { + char initdb_path[MAXPGPATH]; + + 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); + } + + old_cluster.major_version = get_pg_version(old_cluster.pgdata, + &old_cluster.major_version_str); + + /* + * get_control_data() selects pg_resetwal vs. pg_resetxlog via + * bin_version, which check_bindir() normally fills in later. Seed it + * now so the right binary name is used in this early call. + */ + if (old_cluster.bin_version == 0) + old_cluster.bin_version = old_cluster.major_version; + + /* Refuse to overwrite an existing cluster. */ + { + char verfile[MAXPGPATH]; + struct stat st; + + 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); + } + + get_control_data(&old_cluster); + + /* Set up a temporary log directory for the early server start. */ + snprintf(tmp_logdir, sizeof(tmp_logdir), "%s/pg_upgrade_initdb.log.d", + new_cluster.bindir); + if (mkdir(tmp_logdir, pg_dir_create_mode) < 0 && errno != EEXIST) + pg_fatal("could not create temporary log directory \"%s\": %m", + tmp_logdir); + log_opts.logdir = tmp_logdir; + + if (!old_cluster.sockdir) + old_cluster.sockdir = user_opts.socketdir ? user_opts.socketdir : "."; + + prep_status("Inspecting old cluster locale for new cluster creation"); + 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("Creating new cluster with initdb"); + + initPQExpBuffer(&cmd); + appendPQExpBuffer(&cmd, "\"%s/initdb\" -D \"%s\" -N", + new_cluster.bindir, new_cluster.pgdata); + appendPQExpBuffer(&cmd, " -U \"%s\"", 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"); + + appendPQExpBuffer(&cmd, " --encoding=%s", encoding_name); + appendPQExpBuffer(&cmd, " --locale-provider=%s", + collprovider_name(locale->db_collprovider)); + appendPQExpBuffer(&cmd, " --lc-collate=\"%s\" --lc-ctype=\"%s\"", + locale->db_collate, locale->db_ctype); + + if (locale->db_locale) + { + if (locale->db_collprovider == COLLPROVIDER_ICU) + appendPQExpBuffer(&cmd, " --icu-locale=\"%s\"", + locale->db_locale); + else if (locale->db_collprovider == COLLPROVIDER_BUILTIN) + appendPQExpBuffer(&cmd, " --builtin-locale=\"%s\"", + locale->db_locale); + } + + if (new_cluster.pgopts) + appendPQExpBuffer(&cmd, " %s", new_cluster.pgopts); + + exec_prog(UTILITY_LOG_FILE, NULL, true, true, "%s", cmd.data); + + termPQExpBuffer(&cmd); + log_opts.logdir = saved_logdir; + + check_ok(); +} + + static void setup(char *argv0) { @@ -372,17 +538,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 d6e5bca5792..199998e0ab1 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 @@ -423,6 +426,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/007_initdb_option.pl b/src/bin/pg_upgrade/t/007_initdb_option.pl new file mode 100644 index 00000000000..577ce7340a4 --- /dev/null +++ b/src/bin/pg_upgrade/t/007_initdb_option.pl @@ -0,0 +1,105 @@ +# Copyright (c) 2022-2025, 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. +my $oldnode = PostgreSQL::Test::Cluster->new('old_node'); +$oldnode->init; +$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'); +$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. +my $conf = $newnode->data_dir . '/postgresql.conf'; +open(my $fh, '>>', $conf) or die "could not open $conf: $!"; +print $fh "\n# added by test to start the --initdb-created cluster\n"; +print $fh "port = " . $newnode->port . "\n"; +print $fh "listen_addresses = ''\n"; +print $fh "unix_socket_directories = '" . $newnode->host . "'\n"; +close($fh); + +$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)"); + +$newnode->stop; + +# --initdb must refuse to clobber an already-populated data directory. +command_fails( + [ + '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', + ], + '--initdb refuses to overwrite an existing cluster'); + +done_testing(); -- 2.54.0 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-07-10 10:41 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-07-05 21:04 Re: Should we document how column DEFAULT expressions work? David G. Johnston <[email protected]> 2026-07-10 10:41 [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically Bohyun Lee <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox