Re: There should be a way to use the force flag when restoring databases - Mailing list pgsql-hackers

From vignesh C
Subject Re: There should be a way to use the force flag when restoring databases
Date
Msg-id CALDaNm2dpNBHaxPOGF+Y8jSJ+k1vDGz-tMrsqARZ2YR7uUd2OA@mail.gmail.com
Whole thread Raw
In response to Re: There should be a way to use the force flag when restoring databases  (Daniel Gustafsson <daniel@yesql.se>)
List pgsql-hackers
On Wed, 20 Sept 2023 at 17:27, Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 20 Sep 2023, at 11:24, Peter Eisentraut <peter@eisentraut.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
morepeople 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.

I have changed the status of the patch to "Returned with Feedback" as
the comments have not been addressed for some time. Please feel free
to address these issues and update commitfest accordingly.

Regards,
Vignesh



pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: ALTER ROLE documentation improvement
Next
From: vignesh C
Date:
Subject: Re: Add last_commit_lsn to pg_stat_database