> On 20 Sep 2023, at 11:24, Peter Eisentraut <[email protected]> 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
