Re: There should be a way to use the force flag when restoring databases

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Ahmed Ibrahim <ahmed(dot)ibr(dot)hashim(at)gmail(dot)com>, Gurjeet Singh <gurjeet(at)singh(dot)im>, Joan <aseques(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: There should be a way to use the force flag when restoring databases
Date: 2023-09-20 11:57:28
Message-ID: D2DDFAA1-8ECA-4B6C-B531-B8689D587EB7@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 20 Sep 2023, at 11:24, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 06.08.23 21:39, Ahmed Ibrahim wrote:
>> I have addressed the pg version compatibility with the FORCE option in drop. Here is the last version of the patch
>
> The patch is pretty small, but I think there is some disagreement whether we want this option at all? Maybe some more people can make their opinions more explicit?

My my concern is that a --force parameter conveys to the user that it's a big
hammer to override things and get them done, when in reality this doesn't do
that. Taking the example from the pg_restore documentation which currently has
a dropdb step:

====
:~ $ ./bin/createdb foo
:~ $ ./bin/psql -c "create table t(a int);" foo
CREATE TABLE
:~ $ ./bin/pg_dump --format=custom -f foo.dump foo
:~ $ ./bin/pg_restore -d foo -C -c --force foo.dump
pg_restore: error: could not execute query: ERROR: cannot drop the currently open database
Command was: DROP DATABASE foo WITH(FORCE);
pg_restore: error: could not execute query: ERROR: database "foo" already exists
Command was: CREATE DATABASE foo WITH TEMPLATE = template0 ENCODING = 'UTF8' LOCALE_PROVIDER = libc LOCALE = 'en_US.UTF-8';

pg_restore: error: could not execute query: ERROR: relation "t" already exists
Command was: CREATE TABLE public.t (
a integer
);

pg_restore: warning: errors ignored on restore: 3
====

Without knowing internals, I would expect an option named --force to make that
just work, especially given the documentation provided in this patch. I think
the risk for user confusion outweighs the benefits, or maybe I'm just not smart
enough to see all the benefits? If so, I would argue that more documentation
is required.

Skimming the patch itself, it updates the --help output with --force for
pg_dump and not for pg_restore. Additionally it produces a compilerwarning:

pg_restore.c:127:26: warning: incompatible pointer types initializing 'int *' with an expression of type 'bool *' [-Wincompatible-pointer-types]
{"force", no_argument, &force, 1},
^~~~~~
1 warning generated.

--
Daniel Gustafsson

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message a.rybakina 2023-09-20 12:06:35 Re: POC, WIP: OR-clause support for indexes
Previous Message Amit Langote 2023-09-20 11:54:24 Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning