From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Frédéric Yhuel <frederic(dot)yhuel(at)dalibo(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] minor bug fix for pg_dump --clean |
Date: | 2022-10-24 01:01:40 |
Message-ID: | 3650376.1666573300@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
=?UTF-8?Q?Fr=c3=a9d=c3=a9ric_Yhuel?= <frederic(dot)yhuel(at)dalibo(dot)com> writes:
> When using pg_dump (or pg_restore) with option "--clean", there is some
> SQL code to drop every objects at the beginning.
Yup ...
> The DROP statement for a view involving circular dependencies is :
> CREATE OR REPLACE VIEW [...]
> (see commit message of d8c05aff for a much better explanation)
> If the view is not in the "public" schema, and the target database is
> empty, this statement fails, because the schema hasn't been created yet.
> The attached patches are a TAP test which can be used to reproduce the
> bug, and a proposed fix. They apply to the master branch.
I am disinclined to accept this as a valid bug, because there's never
been any guarantee that a --clean script would execute error-free in
a database that doesn't match what the source database contains.
(The pg_dump documentation used to say that in so many words.
I see that whoever added the --if-exists option was under the
fond illusion that that fixes all cases, which it surely does not.
We need to back off the promises a bit there.)
An example of a case that won't execute error-free is if the view
having a circular dependency includes a column of a non-built-in
data type. If you try to run that in an empty database, you'll
get an error from the CREATE OR REPLACE VIEW's reference to that
data type. For instance, if I adjust your test case to make
the "payload" column be of type hstore, I get something like
psql:dumpresult.sql:22: ERROR: type "public.hstore" does not exist
LINE 4: NULL::public.hstore AS payload;
^
The same type of failure occurs for user-defined functions and
operators that use a non-built-in type, and I'm sure there are
more cases in the same vein. But it gets *really* messy if
the target database isn't completely empty, but contains objects
with different properties than the dump script expects; for example,
if the view being discussed here exists with a different column set
than the script thinks, or if the dependency chains aren't all the
same.
If this fix were cleaner I might be inclined to accept it anyway,
but it's not very clean at all --- for example, it's far from
obvious to me what are the side-effects of changing the filter
in RestoreArchive like that. Nor am I sure that the schema
you want to create is guaranteed to get dropped again later in
every use-case.
So I think mainly what we ought to do here is to adjust the
documentation to make it clearer that --clean is not guaranteed
to work without errors unless the target database has the same
set of objects as the source. --if-exists can reduce the set
of error cases, but not eliminate it. Possibly we should be
more enthusiastic about recommending --create --clean (ie,
drop and recreate the whole database) instead.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Steve Chavez | 2022-10-24 01:50:11 | csv_populate_recordset and csv_agg |
Previous Message | Виктория Шепард | 2022-10-24 00:12:46 | Re: [PATCH] minor bug fix for pg_dump --clean |