BUG #17753: pg_dump --if-exists bug

On Thu, 19 Jan 2023 at 22:31, PG Bug reporting form
<noreply@postgresql.org> wrote:

Running pg_dump with the options "--section=pre-data --no-owner
--no-privileges --no-subscriptions --no-publications --clean --if-exists
--file=/Users/ju5tinz/Dev/pg_pass_test/pre_data.sql" prints the warning:
pg_dump: warning: could not find where to insert IF EXISTS in statement "--
*not* dropping schema, since initdb creates it
"

Are you able to share the full warning message including the statement?

David

David Rowley <dgrowleyml@gmail.com> writes:

On Thu, 19 Jan 2023 at 22:31, PG Bug reporting form
<noreply@postgresql.org> wrote:

Running pg_dump with the options "--section=pre-data --no-owner
--no-privileges --no-subscriptions --no-publications --clean --if-exists
--file=/Users/ju5tinz/Dev/pg_pass_test/pre_data.sql" prints the warning:
pg_dump: warning: could not find where to insert IF EXISTS in statement "--
*not* dropping schema, since initdb creates it
"

Are you able to share the full warning message including the statement?

Noting that that string is only associated with the public schema,
I tried dumping a DB in which the public schema had been dropped
--- no dice --- and then recreated --- and then it reproduces!
Apparently, a schema that's named "public" but isn't the original
one is confusing something somewhere, but I'm not immediately
seeing where.

regards, tom lane

I wrote:

Noting that that string is only associated with the public schema,
I tried dumping a DB in which the public schema had been dropped
--- no dice --- and then recreated --- and then it reproduces!
Apparently, a schema that's named "public" but isn't the original
one is confusing something somewhere, but I'm not immediately
seeing where.

Ah, here's the difference: a schema named "public" will be marked
dumpable if its owner is not pg_database_owner:

else if (strcmp(nsinfo->dobj.name, "public") == 0)
{
/*
* The public schema is a strange beast that sits in a sort of
* no-mans-land between being a system object and a user object.
* CREATE SCHEMA would fail, so its DUMP_COMPONENT_DEFINITION is just
* a comment and an indication of ownership. If the owner is the
* default, omit that superfluous DUMP_COMPONENT_DEFINITION. Before
* v15, the default owner was BOOTSTRAP_SUPERUSERID.
*/
nsinfo->create = false;
nsinfo->dobj.dump = DUMP_COMPONENT_ALL;
if (nsinfo->nspowner == ROLE_PG_DATABASE_OWNER)
nsinfo->dobj.dump &= ~DUMP_COMPONENT_DEFINITION;

Having done that, RestoreArchive's kluge to inject IF EXISTS
clauses fails, because (unlike other places in pg_backup_archiver.c)
it is unaware that the dropStmt might be just a comment.

I think we need to do the attached, more or less. Also, although the
given case seems to be new, we'd probably better back-patch, because
I'm not convinced that there aren't other ways to reach this. The
warning message is just cosmetic (the dump output is the same either
way), so possibly this has been seen in the field but not reported.

regards, tom lane

I wrote:

I think we need to do the attached, more or less. Also, although the
given case seems to be new, we'd probably better back-patch, because
I'm not convinced that there aren't other ways to reach this.

Ah --- we only need to back-patch to v15, because this business of
possibly having a comment in the dropStmt is new in a7a7be1f2.
That commit also added the existing exceptions for comments in
pg_backup_archiver.c ... but it missed the need to have special
cases for dropStmts too.

regards, tom lane

On Thu, Jan 19, 2023 at 07:09:50PM -0500, Tom Lane wrote:

I wrote:

I think we need to do the attached, more or less. Also, although the
given case seems to be new, we'd probably better back-patch, because
I'm not convinced that there aren't other ways to reach this.

Ah --- we only need to back-patch to v15, because this business of
possibly having a comment in the dropStmt is new in a7a7be1f2.
That commit also added the existing exceptions for comments in
pg_backup_archiver.c ... but it missed the need to have special
cases for dropStmts too.

Your commit 74739d1 looks good. Thanks.