Hi everyone, I have been working on this. This is a proposed patch for it so we have a force option for DROPping the database.
I'd appreciate it if anyone can review. On Thu, Jul 20, 2023 at 9:36 PM Gurjeet Singh <gurj...@singh.im> wrote: > On Thu, Jul 20, 2023 at 2:10 AM Daniel Gustafsson <dan...@yesql.se> wrote: > > > > > On 19 Jul 2023, at 19:28, Gurjeet Singh <gurj...@singh.im> wrote: > > > > > > The docs for 'pg_restore --create` say "Create the database before > > > restoring into it. If --clean is also specified, drop and recreate the > > > target database before connecting to it." > > > > > > If we provided a force option, it may then additionally say: "If the > > > --clean and --force options are specified, DROP DATABASE ... WITH > > > FORCE command will be used to drop the database." > > > > pg_restore --clean refers to dropping any pre-existing database objects > and not > > just databases, but --force would only apply to databases. > > > > I wonder if it's worth complicating pg_restore with that when running > dropdb > > --force before pg_restore is an option for those wanting to use WITH > FORCE. > > Fair point. But the same argument could've been applied to --clean > option, as well; why overload the meaning of --clean and make it drop > database, when a dropdb before pg_restore was an option. > > IMHO, if pg_restore offers to drop database, providing an option to > the user to do it forcibly is not that much of a stretch, and within > reason for the user to expect it to be there, like Joan did. > > Best regards, > Gurjeet > http://Gurje.et > > >
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index aba780ef4b..aeec3c8dcb 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -154,6 +154,8 @@ typedef struct _restoreOptions int enable_row_security; int sequence_data; /* dump sequence data even in schema-only mode */ int binary_upgrade; + + int force; } RestoreOptions; typedef struct _dumpOptions diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 39ebcfec32..cc13be841a 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -532,6 +532,16 @@ RestoreArchive(Archive *AHX) */ if (*te->dropStmt != '\0') { + if (ropt->force){ + char *dropStmt = pg_strdup(te->dropStmt); + PQExpBuffer ftStmt = createPQExpBuffer(); + dropStmt[strlen(dropStmt) - 2] = ' '; + dropStmt[strlen(dropStmt) - 1] = '\0'; + appendPQExpBufferStr(ftStmt, dropStmt); + appendPQExpBufferStr(ftStmt, "WITH(FORCE);"); + te->dropStmt = ftStmt->data; + } + if (!ropt->if_exists || strncmp(te->dropStmt, "--", 2) == 0) { diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c index 049a100634..02347e86fb 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -74,6 +74,7 @@ main(int argc, char **argv) static int no_security_labels = 0; static int no_subscriptions = 0; static int strict_names = 0; + static int force = 0; struct option cmdopts[] = { {"clean", 0, NULL, 'c'}, @@ -123,6 +124,7 @@ main(int argc, char **argv) {"no-publications", no_argument, &no_publications, 1}, {"no-security-labels", no_argument, &no_security_labels, 1}, {"no-subscriptions", no_argument, &no_subscriptions, 1}, + {"force", no_argument, &force, 1}, {NULL, 0, NULL, 0} }; @@ -351,6 +353,7 @@ main(int argc, char **argv) opts->no_publications = no_publications; opts->no_security_labels = no_security_labels; opts->no_subscriptions = no_subscriptions; + opts->force = force; if (if_exists && !opts->dropSchema) pg_fatal("option --if-exists requires option -c/--clean");