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
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 |